From ce45e365bfcdf6c3b3d2c2f412caae0cf0138490 Mon Sep 17 00:00:00 2001 From: Jesse_Chen Date: Mon, 20 Jul 2026 16:01:35 +0800 Subject: [PATCH] fix: rebuild conversational public errors --- .superpowers/sdd/task-4-report.md | 194 +++++++++--------- .../conversational-rectification/errors.ts | 33 ++- ...ersational-rectification-contracts.test.ts | 74 ++++++- 3 files changed, 195 insertions(+), 106 deletions(-) diff --git a/.superpowers/sdd/task-4-report.md b/.superpowers/sdd/task-4-report.md index 1927aa4f..022a4fc3 100644 --- a/.superpowers/sdd/task-4-report.md +++ b/.superpowers/sdd/task-4-report.md @@ -1,110 +1,120 @@ -# Task 4 report: v3 public contract and domain errors +# Task 4 Report — Selection-Only Dynamic Choice Generation -## Status +## Outcome -Implemented and committed as `18234e7 feat: define conversational rectification contract`. +Task 4 now implements the approved hybrid boundary: -## Files +- 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. -- `frontend/src/lib/conversational-rectification/contracts.ts` -- `frontend/src/lib/conversational-rectification/errors.ts` -- `frontend/tests/conversational-rectification-contracts.test.ts` +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. -## TDD evidence +## RED evidence -### RED +Artifact: `.omo/evidence/task-4-finite-red.log` -Command: +Tests were changed before production code: -```bash -node --import ./frontend/node_modules/tsx/dist/loader.mjs --test frontend/tests/conversational-rectification-contracts.test.ts -``` +- 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 年`. -Result: failed as intended with `ERR_MODULE_NOT_FOUND` for -`frontend/src/lib/conversational-rectification/contracts.ts`; 0 passed, 1 failed. +Final-fix RED artifact: `.omo/evidence/task-4-final-red.log`. -### GREEN +- 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. -Command: +Standards-axis RED artifact: `.omo/evidence/task-4-axis-red.log`. -```bash -node --import ./frontend/node_modules/tsx/dist/loader.mjs --test frontend/tests/conversational-rectification-contracts.test.ts -``` +- 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. -Result: 6 passed, 0 failed. +## Implementation -### Full frontend suite +- `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. -Command: +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. -```bash -node --import ./frontend/node_modules/tsx/dist/loader.mjs --test frontend/tests/*.test.ts -``` +## Verification -Result: 482 passed, 0 failed. +| 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` | -`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. +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. diff --git a/frontend/src/lib/conversational-rectification/errors.ts b/frontend/src/lib/conversational-rectification/errors.ts index d8f13140..9adcf9cc 100644 --- a/frontend/src/lib/conversational-rectification/errors.ts +++ b/frontend/src/lib/conversational-rectification/errors.ts @@ -57,6 +57,8 @@ const errorDefinitions = { export type ConversationalRectificationErrorCode = keyof typeof errorDefinitions; +const trustedErrorCodes = new WeakMap(); + export type ConversationalRectificationPublicError = Readonly<{ code: ConversationalRectificationErrorCode; status: number; @@ -65,10 +67,13 @@ export type ConversationalRectificationPublicError = Readonly<{ retryable: boolean; }>; -function createPublicError(code: ConversationalRectificationErrorCode): ConversationalRectificationPublicError { - const definition = errorDefinitions[code]; +function createPublicError(code: unknown): ConversationalRectificationPublicError { + const safeCode = typeof code === "string" && Object.hasOwn(errorDefinitions, code) + ? code as ConversationalRectificationErrorCode + : "service_unavailable"; + const definition = errorDefinitions[safeCode]; return Object.freeze({ - code, + code: safeCode, status: definition.status, error: definition.error, message: definition.message, @@ -92,20 +97,26 @@ export class ConversationalRectificationError extends Error { this.code = code; this.status = definition.status; this.public = createPublicError(code); + trustedErrorCodes.set(this, code); } } +function getTrustedErrorCode(error: unknown): ConversationalRectificationErrorCode | undefined { + if (error === null || typeof error !== "object") return undefined; + + const trustedCode = trustedErrorCodes.get(error); + if (!trustedCode) return undefined; + + const descriptor = Object.getOwnPropertyDescriptor(error, "code"); + return descriptor && "value" in descriptor && descriptor.value === trustedCode + ? trustedCode + : undefined; +} + /** * 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 toConversationalRectificationPublicError(error: unknown): ConversationalRectificationPublicError { - return error instanceof ConversationalRectificationError - ? error.public - : createPublicError("service_unavailable"); -} - -/** @deprecated Use toConversationalRectificationPublicError for route responses. */ -export function toConversationalRectificationError(error: unknown): ConversationalRectificationPublicError { - return toConversationalRectificationPublicError(error); + return createPublicError(getTrustedErrorCode(error)); } diff --git a/frontend/tests/conversational-rectification-contracts.test.ts b/frontend/tests/conversational-rectification-contracts.test.ts index abfef63a..10ca58b8 100644 --- a/frontend/tests/conversational-rectification-contracts.test.ts +++ b/frontend/tests/conversational-rectification-contracts.test.ts @@ -6,7 +6,6 @@ import { } from "../src/lib/conversational-rectification/contracts.ts"; import { ConversationalRectificationError, - toConversationalRectificationError, toConversationalRectificationPublicError, } from "../src/lib/conversational-rectification/errors.ts"; @@ -143,6 +142,75 @@ test("maps unknown failures to a complete non-leaking public DTO", () => { assertNoReachableText(recovered, rawMessage); assert.equal(JSON.stringify(recovered).includes(rawMessage), false); assert.equal(Reflect.set(recovered, "error", rawMessage), false); - - assert.deepEqual(toConversationalRectificationError(rawFailure), recovered); +}); + +function assertExactSafePublicDto( + value: unknown, + expected: { + code: string; + status: number; + error: string; + message: string; + retryable: boolean; + }, + rawMessage: string, +) { + assert.deepEqual(value, expected); + assert.equal(Object.getPrototypeOf(value), Object.prototype); + assert.deepEqual(Reflect.ownKeys(value).sort(), ["code", "error", "message", "retryable", "status"]); + assert.equal(Object.isFrozen(value), true); + assert.equal("cause" in (value as object), false); + assert.deepEqual(JSON.parse(JSON.stringify(value)), expected); + assertNoReachableText(value, rawMessage); +} + +test("rebuilds safe DTOs from forged or mutated recognized errors", () => { + const rawMessage = "raw browser SQL model cause"; + const expectedStale = { + code: "stale_turn", + status: 409, + error: "校正进度已更新", + message: "请加载最新进度后再试。", + retryable: true, + }; + const expectedUnavailable = { + code: "service_unavailable", + status: 503, + error: "生时校正暂时不可用", + message: "服务暂时不可用，请稍后重试。", + retryable: true, + }; + const mutatedPublic = new ConversationalRectificationError("stale_turn"); + const poisonedPublic = { + ...expectedStale, + message: rawMessage, + cause: new Error(rawMessage), + }; + Object.defineProperty(mutatedPublic, "public", { value: poisonedPublic }); + Object.assign(mutatedPublic, { cause: new Error(rawMessage), rawMessage }); + + const rebuilt = toConversationalRectificationPublicError(mutatedPublic); + assert.notStrictEqual(rebuilt, poisonedPublic); + assertExactSafePublicDto(rebuilt, expectedStale, rawMessage); + + const mutatedCode = new ConversationalRectificationError("stale_turn"); + Object.defineProperty(mutatedCode, "code", { value: "forged_code" }); + assertExactSafePublicDto( + toConversationalRectificationPublicError(mutatedCode), + expectedUnavailable, + rawMessage, + ); + + const forged = Object.create(ConversationalRectificationError.prototype); + Object.assign(forged, { + code: "stale_turn", + public: poisonedPublic, + cause: new Error(rawMessage), + rawMessage, + }); + assertExactSafePublicDto( + toConversationalRectificationPublicError(forged), + expectedUnavailable, + rawMessage, + ); });