fix: harden conversational rectification errors
This commit is contained in:
@@ -1,120 +1,110 @@
|
||||
# Task 4 Report — Selection-Only Dynamic Choice Generation
|
||||
# Task 4 report: v3 public contract and domain errors
|
||||
|
||||
## Outcome
|
||||
## Status
|
||||
|
||||
Task 4 now implements the approved hybrid boundary:
|
||||
Implemented and committed as `18234e7 feat: define conversational rectification contract`.
|
||||
|
||||
- the deterministic engine creates opportunities, candidate partitions, selectable answer
|
||||
semantics, localized prompts, and localized labels;
|
||||
- the Agent may only select one exact server opportunity ID or return advisory
|
||||
`no_useful_question`;
|
||||
- the server validates and renders all public copy, attaches private score vectors, creates
|
||||
public UUIDs, and decides retry, fallback, and termination behavior; and
|
||||
- the raw unmatched-answer note remains available to later workflow layers but is completely
|
||||
omitted from the Agent prompt.
|
||||
## Files
|
||||
|
||||
The Agent cannot author a question, option, label, partition ID, birth-time claim, confidence
|
||||
claim, or control instruction. Those fields are unrepresentable in its strict output schema.
|
||||
This supersedes the keyword-filter/substring-grounding design reviewed in
|
||||
`.omo/evidence/task-4-rereview.md` and the earlier interim `CLEAR` narrative.
|
||||
The final acceptance correction has been implemented and independently re-audited by the
|
||||
executor, but the main acceptance reviewer remains authoritative for completion status.
|
||||
- `frontend/src/lib/conversational-rectification/contracts.ts`
|
||||
- `frontend/src/lib/conversational-rectification/errors.ts`
|
||||
- `frontend/tests/conversational-rectification-contracts.test.ts`
|
||||
|
||||
## RED evidence
|
||||
## TDD evidence
|
||||
|
||||
Artifact: `.omo/evidence/task-4-finite-red.log`
|
||||
### RED
|
||||
|
||||
Tests were changed before production code:
|
||||
Command:
|
||||
|
||||
- TypeScript: 14 tests, 9 expected failures. The failures demonstrated that the note still
|
||||
crossed the prompt, selection-only output was rejected, old free-copy output remained
|
||||
possible, selected server copy was not rendered, and duplicate server labels were accepted.
|
||||
- Python: 7 tests, 1 expected failure. Two distinct same-year windows both rendered as the
|
||||
indistinguishable label `2012—2012 年`.
|
||||
```bash
|
||||
node --import ./frontend/node_modules/tsx/dist/loader.mjs --test frontend/tests/conversational-rectification-contracts.test.ts
|
||||
```
|
||||
|
||||
Final-fix RED artifact: `.omo/evidence/task-4-final-red.log`.
|
||||
Result: failed as intended with `ERR_MODULE_NOT_FOUND` for
|
||||
`frontend/src/lib/conversational-rectification/contracts.ts`; 0 passed, 1 failed.
|
||||
|
||||
- TypeScript: 17 tests, 2 expected failures. Exact and NFKC/whitespace-equivalent primary
|
||||
labels matching either reserved choice were accepted by both binder and service instead of
|
||||
failing before ID allocation.
|
||||
- Python: 4/4 passed, including the new same-month/day-precision regression, confirming that
|
||||
the production behavior existed but previously lacked durable coverage.
|
||||
### GREEN
|
||||
|
||||
Standards-axis RED artifact: `.omo/evidence/task-4-axis-red.log`.
|
||||
Command:
|
||||
|
||||
- TypeScript: 19 tests, 2 expected failures. Server prompts of 121 and 240 characters were
|
||||
accepted by binder and service instead of failing before ID allocation and commit.
|
||||
- Python: 4/4 passed after replacing localized precision glyph assertions with numeric-boundary
|
||||
structure and normalized uniqueness checks.
|
||||
```bash
|
||||
node --import ./frontend/node_modules/tsx/dist/loader.mjs --test frontend/tests/conversational-rectification-contracts.test.ts
|
||||
```
|
||||
|
||||
## Implementation
|
||||
Result: 6 passed, 0 failed.
|
||||
|
||||
- `birth-time-dynamic-question-copy.ts` now contains only server-copy structural validation,
|
||||
NFKC/whitespace label normalization, the note-free opportunity-selection projection, and
|
||||
deterministic server-copy fingerprinting. The former note blacklist and substring
|
||||
grounding logic were removed. Shared constants cap server questions at 120 characters and
|
||||
labels at 80 across the API adapter, public schema, internal model, persisted schema, and
|
||||
binding guard.
|
||||
- `birth-time-dynamic-question-validator.ts` accepts only strict selection objects. Binding
|
||||
resolves the selected server opportunity, validates the prompt and normalized uniqueness
|
||||
across every primary and reserved visible label, validates every matching private
|
||||
partition, and only then allocates IDs. Malformed
|
||||
server copy, private bindings, UUIDs, and persisted records raise
|
||||
`BirthTimeDynamicBindingError` and cannot be retried into a false low result.
|
||||
- Fallback sorts opportunities by information gain descending and then opportunity ID,
|
||||
independent of packet order. Repeated fingerprints alone are skipped as recoverable.
|
||||
- `dynamic_rectification_copy.py` owns localized contexts and the least detailed
|
||||
year/month/day range representation needed to distinguish visible windows. Cross-year
|
||||
ranges stay concise; same-year or same-month collisions gain month or day precision.
|
||||
Its precision discriminator is the exhaustive `Literal["year", "month", "day"]` domain;
|
||||
unknown precision cannot silently fall through. `dynamic_rectification_opportunities.py`
|
||||
remains below the 250-pure-LOC boundary.
|
||||
- The Mastra contract describes selection only and forbids prompt/options/labels/partition
|
||||
fields in Agent output.
|
||||
### Full frontend suite
|
||||
|
||||
The real Python-shaped fixture retains structural CJK/no-ASCII copy, normalized label
|
||||
uniqueness, partition count, opportunity ID, fingerprint, and partition-ID seam checks without
|
||||
pinning exact natural-language prose. It is parsed through the Task 3 adapter and exercised
|
||||
through the Task 4 service. Task 5 persistence was not changed.
|
||||
The service-level adversarial-note regression independently parses every captured Agent prompt
|
||||
and requires the exact `task`/`opportunities` projection and exact safe opportunity keys. It
|
||||
does not call the production serializer or search for literal note prose.
|
||||
Command:
|
||||
|
||||
## Verification
|
||||
```bash
|
||||
node --import ./frontend/node_modules/tsx/dist/loader.mjs --test frontend/tests/*.test.ts
|
||||
```
|
||||
|
||||
| Gate | Result | Artifact |
|
||||
| --- | --- | --- |
|
||||
| Standards-axis RED | expected 2 TS failures; Python 4/4 | `.omo/evidence/task-4-axis-red.log` |
|
||||
| Focused dynamic/guide TypeScript | 40/40 pass | `.omo/evidence/task-4-axis-focused-ts.log` |
|
||||
| Public dynamic-choice schema TypeScript | 7/7 pass | `.omo/evidence/task-4-axis-public-schema-ts.log` |
|
||||
| Dynamic adapter boundary TypeScript | 8/8 pass | `.omo/evidence/task-4-axis-adapter-ts.log` |
|
||||
| Focused Task 2 Python | 26/26 pass | `.omo/evidence/task-4-axis-focused-python.log` |
|
||||
| Legacy Python rectification | 22/22 pass | `.omo/evidence/task-4-axis-legacy-python.log` |
|
||||
| All birth-time TypeScript | 229/229 pass | `.omo/evidence/task-4-axis-birth-time.log` |
|
||||
| Full frontend | 304/304 pass | `.omo/evidence/task-4-axis-frontend-full.log` |
|
||||
| Cumulative changed TypeScript ESLint | pass, zero diagnostics | `.omo/evidence/task-4-axis-eslint.log` |
|
||||
| Cumulative changed Python Ruff | pass | `.omo/evidence/task-4-axis-ruff.log` |
|
||||
| Diff check and all changed TS/Python LOC | pass; every audited file <=250 | `.omo/evidence/task-4-axis-quality.log` |
|
||||
| Full TypeScript check | only known unrelated `profile-persistence.test.ts:7` TS1501 | `.omo/evidence/task-4-axis-tsc.log` |
|
||||
| Structural prompt focused TypeScript | 40/40 pass | `.omo/evidence/task-4-structural-focused-ts.log` |
|
||||
| Structural prompt ESLint | pass, zero diagnostics | `.omo/evidence/task-4-structural-eslint.log` |
|
||||
| Structural prompt diff/LOC audit | pass; cumulative files <=250 | `.omo/evidence/task-4-structural-quality.log` |
|
||||
| Structural prompt TypeScript check | only known unrelated TS1501 | `.omo/evidence/task-4-structural-tsc.log` |
|
||||
| Fresh structural-prompt review | CLEAR / APPROVE; no blockers | `.omo/evidence/task-4-structural-prompt-code-review.md` |
|
||||
Result: 482 passed, 0 failed.
|
||||
|
||||
The TypeScript command remains non-zero solely because the pre-existing profile-persistence
|
||||
test uses a regular-expression flag newer than the configured target. No Task 4 file reports
|
||||
a type error.
|
||||
The earlier `.omo/evidence/task-4-selection-boundary-code-review.md` `CLEAR` is explicitly
|
||||
superseded by `.omo/evidence/task-4-final-review.md`; it is not cited as current acceptance.
|
||||
The earlier `.omo/evidence/task-4-final-fix-code-review.md` `CLEAR` is explicitly superseded by
|
||||
the standards-axis review and is not cited as current acceptance. The new tests contain no
|
||||
localized month/day or domain-word assertions; precision is verified through distinct normalized
|
||||
labels and the number of numeric range-boundary tokens.
|
||||
The earlier `.omo/evidence/task-4-axis-fix-code-review.md` `CLEAR` is explicitly superseded by
|
||||
the main acceptance test finding; it is retained only as historical evidence. The shared
|
||||
120/80 boundary remains verified through the public schema, API adapter, internal and persisted
|
||||
schemas, and binding guard.
|
||||
The fresh reviewer independently verified the adversarial structural projection assertion,
|
||||
40/40 focused tests, zero-diagnostic ESLint, the 250-pure-LOC boundary, and both required
|
||||
programming/remove-slops perspectives with no remaining blocker.
|
||||
`git diff --check` over the three task files also exited successfully before the commit.
|
||||
|
||||
## Self-review
|
||||
|
||||
- Commands are a strict discriminated union of exactly `start`, `resume`, `answer`,
|
||||
`pause`, `abandon`, and `confirm`.
|
||||
- Every action-bearing command requires a UUID `actionId`; every command after `start`
|
||||
requires a nonnegative integer `turnVersion`; answers are trimmed, nonblank, and at
|
||||
most 4,000 characters; confirmation time is strict 24-hour `HH:mm`.
|
||||
- Strict command objects reject client candidate scores, technical receipts, and other
|
||||
unknown fields. The response schema matches the prescribed public v3 turn shape and
|
||||
rejects extra nested candidate or technical fields.
|
||||
- Domain errors expose fixed Chinese `error` and `message` values. Unknown browser,
|
||||
SQL, and model errors are converted to the stable `service_unavailable` response;
|
||||
their raw messages are never copied into public output.
|
||||
- Tests use synthetic UUIDs, times, and event text only.
|
||||
|
||||
## Concerns
|
||||
|
||||
- None for this contract boundary. Future route/orchestrator work must return only
|
||||
`toConversationalRectificationPublicError()` output to preserve the non-leakage
|
||||
guarantee.
|
||||
|
||||
## Review remediation: safe route-facing error DTO
|
||||
|
||||
### RED
|
||||
|
||||
After adding the public-boundary regressions, the focused test command failed as
|
||||
expected because `toConversationalRectificationPublicError` was not exported:
|
||||
|
||||
```bash
|
||||
node --import ./frontend/node_modules/tsx/dist/loader.mjs --test frontend/tests/conversational-rectification-contracts.test.ts
|
||||
```
|
||||
|
||||
### GREEN
|
||||
|
||||
`toConversationalRectificationPublicError()` now returns a plain, frozen public DTO
|
||||
with only stable `code`, `status`, `error`, `message`, and `retryable` fields. It
|
||||
does not retain the unknown input, its `cause`, or any browser, SQL, or model-error
|
||||
properties. The compatibility mapper delegates to that same safe route-facing mapper.
|
||||
|
||||
The catch-all copy is now neutral: `服务暂时不可用,请稍后重试。` It makes no data-retention
|
||||
promise when persistence could have failed.
|
||||
|
||||
The new regressions recursively inspect all own keys and value descriptors of the
|
||||
complete mapper result, serialize it with `JSON.stringify`, assert that a synthetic
|
||||
raw browser/SQL/model failure message is unreachable, and prove the returned DTO is
|
||||
immutable.
|
||||
|
||||
Focused result: 7 passed, 0 failed.
|
||||
|
||||
### Full frontend suite
|
||||
|
||||
```bash
|
||||
node --import ./frontend/node_modules/tsx/dist/loader.mjs --test frontend/tests/*.test.ts
|
||||
```
|
||||
|
||||
Result: 483 passed, 0 failed.
|
||||
|
||||
### Pre-work gate
|
||||
|
||||
The required `python3 scripts/pre_work_check.py --remote-timeout 8 --command-timeout 45`
|
||||
was run. It remains blocked by the repository's documented host-wide Python 3.9 /
|
||||
missing-pytest and fragment-scan compatibility failures; this frontend-only change
|
||||
does not alter those checks.
|
||||
|
||||
@@ -3,46 +3,55 @@ const errorDefinitions = {
|
||||
status: 400,
|
||||
error: "校正请求格式不正确",
|
||||
message: "请检查填写内容后再试。",
|
||||
retryable: false,
|
||||
},
|
||||
authentication_required: {
|
||||
status: 401,
|
||||
error: "请先登录",
|
||||
message: "登录后才能继续生时校正。",
|
||||
retryable: false,
|
||||
},
|
||||
case_not_found: {
|
||||
status: 404,
|
||||
error: "校正记录不存在",
|
||||
message: "请重新开始生时校正。",
|
||||
retryable: false,
|
||||
},
|
||||
stale_turn: {
|
||||
status: 409,
|
||||
error: "校正进度已更新",
|
||||
message: "请加载最新进度后再试。",
|
||||
retryable: true,
|
||||
},
|
||||
invalid_transition: {
|
||||
status: 409,
|
||||
error: "当前步骤不可用",
|
||||
message: "请加载最新进度后再试。",
|
||||
retryable: true,
|
||||
},
|
||||
candidate_changed: {
|
||||
status: 409,
|
||||
error: "候选结果已变化",
|
||||
message: "请查看最新候选结果后再确认。",
|
||||
retryable: true,
|
||||
},
|
||||
profile_incomplete: {
|
||||
status: 409,
|
||||
error: "出生资料尚未完成",
|
||||
message: "请先补全出生日期、时间和地点。",
|
||||
retryable: false,
|
||||
},
|
||||
insufficient_credits: {
|
||||
status: 409,
|
||||
error: "校正点数不足",
|
||||
message: "请补充点数后再开始校正。",
|
||||
retryable: false,
|
||||
},
|
||||
service_unavailable: {
|
||||
status: 503,
|
||||
error: "生时校正暂时不可用",
|
||||
message: "当前资料已安全保留,请稍后重试。",
|
||||
message: "服务暂时不可用,请稍后重试。",
|
||||
retryable: true,
|
||||
},
|
||||
} as const;
|
||||
|
||||
@@ -53,11 +62,23 @@ export type ConversationalRectificationPublicError = Readonly<{
|
||||
status: number;
|
||||
error: string;
|
||||
message: string;
|
||||
retryable: boolean;
|
||||
}>;
|
||||
|
||||
function createPublicError(code: ConversationalRectificationErrorCode): ConversationalRectificationPublicError {
|
||||
const definition = errorDefinitions[code];
|
||||
return Object.freeze({
|
||||
code,
|
||||
status: definition.status,
|
||||
error: definition.error,
|
||||
message: definition.message,
|
||||
retryable: definition.retryable,
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* A domain error with a deliberately fixed public representation. The optional cause
|
||||
* is retained only for server-side logging and is never copied into the response.
|
||||
* A domain error with fixed, client-safe copy. Do not attach raw causes to this
|
||||
* browser-importable value; server code must log an unknown cause before mapping it.
|
||||
*/
|
||||
export class ConversationalRectificationError extends Error {
|
||||
readonly name = "ConversationalRectificationError";
|
||||
@@ -65,25 +86,26 @@ export class ConversationalRectificationError extends Error {
|
||||
readonly status: number;
|
||||
readonly public: ConversationalRectificationPublicError;
|
||||
|
||||
constructor(code: ConversationalRectificationErrorCode, options?: ErrorOptions) {
|
||||
constructor(code: ConversationalRectificationErrorCode) {
|
||||
const definition = errorDefinitions[code];
|
||||
super(definition.error, options);
|
||||
super(definition.error);
|
||||
this.code = code;
|
||||
this.status = definition.status;
|
||||
this.public = {
|
||||
code,
|
||||
status: definition.status,
|
||||
error: definition.error,
|
||||
message: definition.message,
|
||||
};
|
||||
this.public = createPublicError(code);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Converts unknown database, browser, and model failures to one safe recovery error.
|
||||
* The only error mapper intended for route responses. It returns a plain, frozen DTO
|
||||
* and never keeps the unknown input or any of its properties reachable.
|
||||
*/
|
||||
export function toConversationalRectificationError(error: unknown): ConversationalRectificationError {
|
||||
export function toConversationalRectificationPublicError(error: unknown): ConversationalRectificationPublicError {
|
||||
return error instanceof ConversationalRectificationError
|
||||
? error
|
||||
: new ConversationalRectificationError("service_unavailable", { cause: error });
|
||||
? error.public
|
||||
: createPublicError("service_unavailable");
|
||||
}
|
||||
|
||||
/** @deprecated Use toConversationalRectificationPublicError for route responses. */
|
||||
export function toConversationalRectificationError(error: unknown): ConversationalRectificationPublicError {
|
||||
return toConversationalRectificationPublicError(error);
|
||||
}
|
||||
|
||||
@@ -7,6 +7,7 @@ import {
|
||||
import {
|
||||
ConversationalRectificationError,
|
||||
toConversationalRectificationError,
|
||||
toConversationalRectificationPublicError,
|
||||
} from "../src/lib/conversational-rectification/errors.ts";
|
||||
|
||||
const actionId = "a9890e09-d535-46f0-9a36-86017515a5a1";
|
||||
@@ -96,21 +97,52 @@ test("accepts only the exact public turn shape", () => {
|
||||
assert.equal(conversationalRectificationTurnSchema.safeParse({ ...turn, technicalReceipt: { ...turn.technicalReceipt, rawModelOutput: "secret" } }).success, false);
|
||||
});
|
||||
|
||||
test("maps known domain failures to stable Chinese recovery copy", () => {
|
||||
const stale = new ConversationalRectificationError("stale_turn");
|
||||
assert.deepEqual(stale.public, {
|
||||
function assertNoReachableText(value: unknown, forbidden: string, seen = new Set<unknown>()) {
|
||||
if (typeof value === "string") {
|
||||
assert.equal(value.includes(forbidden), false, `found raw text in ${value}`);
|
||||
return;
|
||||
}
|
||||
if (value === null || (typeof value !== "object" && typeof value !== "function") || seen.has(value)) return;
|
||||
|
||||
seen.add(value);
|
||||
for (const key of Reflect.ownKeys(value)) {
|
||||
assertNoReachableText(String(key), forbidden, seen);
|
||||
const descriptor = Object.getOwnPropertyDescriptor(value, key);
|
||||
if (descriptor && "value" in descriptor) assertNoReachableText(descriptor.value, forbidden, seen);
|
||||
}
|
||||
}
|
||||
|
||||
test("maps known domain failures to a frozen stable public DTO", () => {
|
||||
const stale = toConversationalRectificationPublicError(new ConversationalRectificationError("stale_turn"));
|
||||
assert.deepEqual(stale, {
|
||||
code: "stale_turn",
|
||||
status: 409,
|
||||
error: "校正进度已更新",
|
||||
message: "请加载最新进度后再试。",
|
||||
retryable: true,
|
||||
});
|
||||
assert.equal(Object.isFrozen(stale), true);
|
||||
assert.equal(Reflect.set(stale, "message", "mutated"), false);
|
||||
});
|
||||
|
||||
const recovered = toConversationalRectificationError(new Error("WebKit SyntaxError: SQL password=model secret"));
|
||||
assert.deepEqual(recovered.public, {
|
||||
test("maps unknown failures to a complete non-leaking public DTO", () => {
|
||||
const rawMessage = "WebKit SyntaxError: SQL password=model secret";
|
||||
const rawFailure = Object.assign(new Error(rawMessage, { cause: new Error(rawMessage) }), {
|
||||
browserError: rawMessage,
|
||||
modelResponse: { message: rawMessage },
|
||||
});
|
||||
const recovered = toConversationalRectificationPublicError(rawFailure);
|
||||
assert.deepEqual(recovered, {
|
||||
code: "service_unavailable",
|
||||
status: 503,
|
||||
error: "生时校正暂时不可用",
|
||||
message: "当前资料已安全保留,请稍后重试。",
|
||||
message: "服务暂时不可用,请稍后重试。",
|
||||
retryable: true,
|
||||
});
|
||||
assert.doesNotMatch(recovered.public.message, /WebKit|SQL|model|secret/i);
|
||||
assert.equal(Object.isFrozen(recovered), true);
|
||||
assertNoReachableText(recovered, rawMessage);
|
||||
assert.equal(JSON.stringify(recovered).includes(rawMessage), false);
|
||||
assert.equal(Reflect.set(recovered, "error", rawMessage), false);
|
||||
|
||||
assert.deepEqual(toConversationalRectificationError(rawFailure), recovered);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user