fix(rich): Phase-D3 persisted-read trust and fail-closed editable adoption (#669) - #690
Conversation
classifyPersistedRichAnswer is the single §7 read authority for persisted candidate answers (empty / plain / legacy_plain / rich_valid / rich_noncanonical / unsupported_version / corrupt). A string in a rich slot is corrupt unless explicit legacyPlainProvenance evidence is supplied — runtime shape never establishes legacy_plain (PC-F03). rich_valid requires the write seam's canonicalization to reproduce the persisted value (RC-03); schema-valid noncanonical values (incl. the PC-F01 closure class) are returned unrepaired. resolveRichAnswerDocument becomes a binary projection of the classifier; isContentDocumentV1's contract comment now states it is a shape hint, never trust proof.
RichTextAnswerInput now adopts through the typed read authority: only rich_valid / rich_noncanonical / empty (and plain-mode strings via the authorized kernel upgrade) mount the editor; unsupported_version and corrupt render an integrity notice with NO editor — no initial onChange/autosave can ever replace the persisted source with synthetic empty content (PC-F04, CORRUPT_PERSISTED_RICH != EMPTY_DOCUMENT). The frozen snapshot answerMode is threaded from QuestionRenderer so the classification has slot context. Initial load, reload, EXAM-519 reconcile, REC-I3 restore (all through applySnapshot) and STALE_VERSION serverAnswer adoption share these trust semantics at the single component seam. Read-only consumers (ResultPage / GradingDetailPage / AttemptDetailPage) route the same authority: an uninterpretable rich-mode answer keeps the integrity notice instead of the raw payload or the shallow-gate branch (PC-F05 read side).
inlineToTiptap / blockToTiptap gain explicit default branches that throw on nodes outside the closed grammar. Previously an unknown block produced a silent undefined entry in the Tiptap content array and the editor mounted with the remaining fragments (PC-F07); conversion now fails explicitly. Classifier-gated schema-valid documents make the throw unreachable in practice — it is the defense-in-depth boundary, matching the reverse direction's existing unmappable-node throws.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
jnhu76
left a comment
jnhu76
left a comment
There was a problem hiding this comment.
Focused D3 review: CHANGE_REQUIRED — one blocker remains.
F1 — rich_noncanonical is currently treated as editable exactly like rich_valid, which violates the frozen §7 rule that read classification must not become a silent repair write.
Evidence:
classifyPersistedRichAnswerdeliberately returns the persisted, unrepaired document asrich_noncanonical.RichTextAnswerInputmountsrich_noncanonicaldirectly intoRichContentEditorLazywith the normalonChangecallback.RichContentEditormaps everyonUpdatethroughtiptapToContentDocument(), which normalizes beforeonChange(next).- The new read-trust test itself notes that the normal editor mount may emit transactions/callbacks (
callsAtValidMount).
Therefore a schema-valid but noncanonical persisted value can be mounted, normalized by the editor representation, and emitted into autosave without a user semantic edit. For ordinary noncanonical values this is an implicit repair write; for the PC-F01 closure-class fixture, the editor can hold/emit the merged >textRun representation that the D1 server boundary rejects, leaving an apparently editable answer that cannot be saved. Both contradict the D3 requirement: persisted content must earn editability, and read must not silently repair persisted truth.
Required focused correction (do not broaden D3):
- Define an explicit editable policy for
rich_noncanonicalthat cannot silently mutate persisted truth. The simplest safe policy is fail-closed/read-integrity state for allrich_noncanonicaluntil an explicit migration/repair path exists. A more permissive policy is acceptable only if it proves mount emits zero repair write and handles canonicalization-failing PC-F01-class values safely. - Add regressions for both:
- canonicalizable noncanonical fixture (split/unsorted marks): mount/adoption must not silently emit a repaired value;
- PC-F01 closure-class noncanonical fixture: must not expose an editor that immediately/inevitably produces an unsaveable canonical value.
- Keep
rich_valid, explicit empty, plain-slot mapping, corrupt/unsupported, STALE_VERSION, and PC-F07 behavior unchanged.
Everything else I reviewed is good: provenance rule closes PC-F03; unsupported/corrupt fail closed and block the destructive empty-editor path; resolveRichAnswerDocument is a projection of the typed classifier rather than a second oracle; adapter unknown block/inline now throws; exact-head CI is green.
Verdict:
PC-F03 = PASS
PC-F04 = PASS
PC-F05 = PASS
PC-F07 = PASS
F1_NONCANONICAL_EDIT_ADOPTION = BLOCKER
PHASE_D3 = CHANGE_REQUIRED
BROAD_RERUN_REQUIRED = NO
Only a focused correction + focused re-review is needed.
Focused-review repair (#669 Phase D3 F1): RichTextAnswerInput mounted rich_noncanonical with the ordinary editor, whose onUpdate path re-serializes through canonicalization — turning read classification into a silent repair write of never-edited content, and, for the PC-F01 closure class, producing a canonical form the write seam always rejects. Noncanonical persisted values now join the integrity state; repair of such values remains a future explicit policy, never read-time behavior.
F1 处置（
|
jnhu76
left a comment
jnhu76
left a comment
There was a problem hiding this comment.
Focused re-review — PASS.
F1_NONCANONICAL_EDIT_ADOPTION = CLOSED.
Verified at head e47c110:
RichTextAnswerInputgrants editability only torich_valid, explicitempty, and authorizedplain/legacy_plainmappings.rich_noncanonical,unsupported_version, andcorruptall fail closed to the same integrity state; no editor surface means no mount/update/autosave repair path.- F1-A guards a canonicalizable noncanonical value (split same-mark runs + unsorted marks) with zero
onChangeemission. - F1-B guards the PC-F01 closure-class noncanonical value from exposing any editable surface.
resolveRichAnswerDocumentremains explicitly read-only DISPLAY projection forrich_valid/rich_noncanonical; this does not confer editability.- Classifier/provenance/STALE_VERSION/adapter semantics from the prior review are unchanged.
- exact-head CI run 37058097355 completed SUCCESS (10 jobs).
Final D3 verdict:
PC-F03 = PASS
PC-F04 = PASS
PC-F05 = PASS
PC-F07 = PASS
F1 = CLOSED
PHASE_D3 = PASS
READY_FOR_D2 = YES_AFTER_MERGE
GATE_1 = NOT_EVALUATED
No further D3 code changes requested.
1 participant
变更说明
#669 Phase D3：修复 Phase-C 发现 PC-F03 / PC-F04 / PC-F05 / PC-F07（候选可编辑路径的类型化读信任缺失 + adapter 静默降级）。基线 = live master
1d8bbb6a（PR #687 / Phase D1 merge）。根因与修复
RichTextAnswerInput以typeof value === "string"无出处采用字符串为 legacy 内容plain（冻结 answerMode 即出处）；rich slot 字符串仅在显式legacyPlainProvenance证据下 =legacy_plain，否则corruptdocVersion:2信封掉进plainTextToDocument("")→ 可编辑空编辑器 → 首个 onChange/autosave 覆写原值unsupported_version：渲染完整性提示，不挂编辑器 ⇒ 无任何 onChange/autosave 路径isContentDocumentV1放深度非法值进编辑器blockToTiptap/inlineToTiptap无 default 分支：未知节点 →undefined条目 → 编辑器带残缺文档静默挂载unmappable block/inline node）；分类器使其实际不可达，throw 是纵深防御边界读状态模型（§7 单一权威）
classifyPersistedRichAnswer({ value, answerMode, legacyPlainProvenance? })→empty | plain | legacy_plain | rich_valid | rich_noncanonical | unsupported_version | corrupt（richAnswer.ts，由既有resolveRichAnswerDocument演化而来；后者保留为其二进制投影，无第二读 oracle）。rich_valid= 写缝同一canonicalizeContentDocument精确复现持久值（RC-03）；schema 合法但非 canonical（含 PC-F01 闭包类，fixture 化回归）→rich_noncanonical，按持久化形态返回，读取期不修复、不改写。answerMode来自冻结快照（nullish → plain 合同变换）。legacy_plain当前无生产者——rich 写缝与 answerMode 同期落地（Content/WYSIWYG V1 — Plain/Rich content model, math/table/code, asset-ready, performance-first #301），受支持的写者从未产出 rich-slot 字符串（Phase-C 台账 L1：SUPPORTED_CURRENT_WRITER = NO）；状态为未来可证明出处的迁移保留。采纳策略（所有路径同一信任语义）
initial load / reload retry / EXAM-519 reconcile / REC-I3 restore 全部经
applySnapshot，STALE_VERSION 经setAnswers——都存 raw 值，由RichTextAnswerInput单点解释（§10）：rich_valid→ 按持久化文档挂载编辑器empty→ 显式空编辑器；plain/legacy_plain→ 内核授权升级映射（plainTextToDocument）rich_noncanonical/unsupported_version/corrupt→ 完整性提示（content.unsafeEditableAnswer），无编辑器 ⇒ 无合成 onChange ⇒ 无 autosave 可覆写源真值rich_noncanonical可解释但不可编辑（F1，focused review 修复e47c1107）：编辑器 onUpdate 经 canonicalization 再序列化，挂载 noncanonical 值即把读分类变成对未编辑内容的静默修复写；PC-F01 闭包类 canonical 化后超CONTENT_LIMITS.textRun，会得到一个产出永远无法保存的值的可编辑表面。此类值的修复属未来显式 repair policy，绝不发生在读取期。只读渲染（结果/批改页）仍按持久化形态原样显示。只读消费者（ResultPage / GradingDetailPage / AttemptDetailPage）经同一权威：rich slot 不可解释值显示
content.unsupportedAnswer，不再漏渲染原始字符串、不再以浅门做信任决策（§13：editable 与静态显示无弱差异）。回归证据（R1–R5 + F1，最小化 fixture）
corrupt+ 组件完整性态（原QuestionRenderer的 PC-F03 行为测试改写为 fail-closed 断言）；正控：plain slot 字符串仍升级挂载；分类器级正控证明显式 provenance 下legacy_plain可达docVersion:2→unsupported_version；组件断言无.ProseMirror、onChange 零发射（覆盖 mount → onChange → autosave 全危险边）corrupt；生产代码中isContentDocumentV1调用仅剩分类器内部深门控前置/unmappable block node: script/）；正控：canonical 文档无损往返验证
e47c1107）：richAnswer + contentAdapter + readTrust(9，含 F1-A/F1-B) + QuestionRenderer 66/66 PASS；pnpm verify:staticPASSpnpm verify:static：PASSpnpm verify：PASS @017cf87c（API coverage 显式重跑：210 files / 2783 tests passed | 4 files skipped，含server.shutdown——首跑 EADDRINUSE 为宿主遗留 12htsx watchdev server 占用 :3000 的环境性冲突，清理后复跑全绿，与本 diff 无关）；e47c1107全量等价证据由 exact-head CI 提供bash scripts/e2e/run.sh rich-content）：9/9 passed @017cf87c（含 STALE_VERSION 真浏览器 reconciliation 流），runner exit 0非目标
无 D2（receipt）/ D4（export 契约）/ D5（KaTeX）工作；未改 Rich 文法、
CONTENT_LIMITS、canonicalization 语义、Attempt 生命周期、SaveAnswer 错误优先级；未重开 C12–C15（编辑器→canonical 方向的 C14 冻结 cell/list 映射保持原样）；无 #678 capability 工作。关联 #669 #673 #686；基线 #687 merge
1d8bbb6a。