Files
ARCADIA-Studio/harness/text-default-omission-review-20260913.md
T

5.5 KiB
Raw Blame History

Independent default-omission candidate review

Reviewer: /root/default_omission_review, 2026-09-13. Bounded static review of harness/text-default-omission-design-20260913.md against source commit 9fd76aef3ae2810667d3f2b031fc43692fc04c89, branch codex/text-diagnostics-9e8r6t. Parent owns issue 11/comment 911 and coordination. No production edits, builds, runtime experiments, staging, or Gitea operations. The candidate and concurrent issue 12 work were left untouched. Read applicable local instructions and relevant user specification/checkpoint. Local origin/HEAD names the old codex/behavior-draft-lifecycle ref, where AGENTS.md is absent; this is not a fresh remote-instruction check or authority to discard current instructions.

Findings

  1. P2 — Define CompleteForEdit as structurally complete, not the existing serializer with compact mode disabled. Candidate lines 132–135 require complete comparison shapes, but do not explicitly require expansion of fields already suppressed by persistence serialization. Current extraction uses serde_json::to_value (crates/oa-core/src/language.rs:675, 691, 755–760); OccurrenceBody::Exchange.operation, NodePlacement.port_anchors, and TraceBaseline.semantic have skip_serializing_if (model.rs:561, 1556, 1604). Thus retaining today's writer as the complete branch does not satisfy the proposed preservation invariant. Concrete static witnesses: an authored port_anchors []; disappears when the same node's position changes; authored operation none; disappears when another Exchange member changes; authored semantic none; disappears when a prose snapshot inside the same baseline changes. The affected record is visited, and SyntaxUpdate::record erases actual fields absent from after (source_project.rs:1293–1305). All three explicit forms are admitted: node defaults at language.rs:1841, Exchange serde option at model.rs:561–562, and semantic null at language/semantic_source.rs:107–109. Recommendation: explicitly synthesize every admitted optional/default slot in comparison records at its applicable typed context, including these existing skips, while preserving JSON serialization. Add each witness with byte-level expected output. This is a design clarification before implementation acceptance; the candidate's general complete-shape invariant already points toward the correct fix.

  2. P2 — Correct the rename witness and strengthen the test trigger. Candidate principal finding line 26 says a compact shared writer erases defaults when renaming an element. A pure name change does not exercise this failure: update_bundle calls update.header separately (source_project.rs:932), then SyntaxUpdate::value returns immediately when old/new fields compare equal (1191–1192; recursive equality at 1164–1175). An alias-only change without a changed self-reference likewise leaves that record's fields equal. The erasure occurs when a semantic sibling in the same record changes. For example, change a Scenario precondition while its explicitly authored empty description, root parent, and other default fields are unchanged; compact after lacks those keys, so the record erases them. Recommendation: correct the asserted witness and require unrelated payload edits at declaration, selected-constructor, and nested-record levels, alongside existing Rename/Alias/no-op cases. Otherwise the proposed preservation checks can pass while the principal failure remains.

Conclusions from attempted falsification

  • The source/factory distinction is correct. language.rs:1734–1742 explicitly overrides factory scenario viewpoints to MixedLegacy on omission; factories choose strict OA/SA viewpoints at model.rs:804–808. Source omission must govern export.
  • Required/nested inventory is consistent with the targeted source: declaration merging is shallow (language.rs:1995–2005), mandatory declaration gates remain separate (1720–1724, 1785–1797, 1837–1839, 1850–1851, 1862–1866), and selected payload fields follow their own serde contract. In particular, String minimum_length, Signal items, Branch fields, and ScenarioUse bindings must survive. Option-field/spec ambiguity is already identified rather than concealed by the candidate; runtime confirmation remains outstanding.
  • Keeping the complete comparison and compact insertion representations separate is necessary. New declarations consume after[id] (source_project/source_structure.rs:204–223), field insertion consumes after-text spans (source_project.rs:1327–1336), and shape replacement also consumes after text (1243–1247). A fragment renderer must retain typed path/context; pruning by the leaf's spelling alone is insufficient. Existing constructor-switch preservation is real (1268–1289) and must survive the split.
  • Signed-zero policy is correct. Omission of node position must compare coordinate bits rather than ordinary equality; Position has floating coordinates and derived PartialEq (model.rs:214–219). The updater's final model-equality guard (source_project.rs:1017–1025) cannot establish signed-zero preservation. The candidate already calls for separate bit checks; execute them through both exporters and lossless edits.

Recommendation: retain candidate C's approach, revise the two points above before treating it as an implementation-ready contract, then run the proposed targeted evaluators. No static finding justifies generic pruning, changing parser admission, or treating peer source8 work as integrated. This review establishes neither runtime correctness nor issue closure.