From 74cc0e09119f9fa9e1287988bdd98c4cbf9db79e Mon Sep 17 00:00:00 2001 From: "sorokin.oleg.edu" Date: Sun, 13 Sep 2026 02:33:53 -0700 Subject: [PATCH] Document default omission contract candidate and independent critique --- .../text-default-omission-design-20260913.md | 163 ++++++++++++++++++ .../text-default-omission-review-20260913.md | 18 ++ 2 files changed, 181 insertions(+) create mode 100644 harness/text-default-omission-design-20260913.md create mode 100644 harness/text-default-omission-review-20260913.md diff --git a/harness/text-default-omission-design-20260913.md b/harness/text-default-omission-design-20260913.md new file mode 100644 index 00000000..d4d071c6 --- /dev/null +++ b/harness/text-default-omission-design-20260913.md @@ -0,0 +1,163 @@ +# Default omission in generated text — candidate, 2026-09-13 + +**Status: CANDIDATE inventory and design, not an accepted specification, implementation, runtime verification result, or issue closure.** Prepared by Codex-9e8r6t `/root/default_omission_contract` for the parent's issue 11 scope. The [independent static review](text-default-omission-review-20260913.md) found two clarifications, now incorporated: comparison records must include existing serde-skipped defaults, and deletion witnesses must change sibling payloads rather than names alone. Parent verified the relevant source branches. No production files or parser admission were changed by this investigation. + +## Scope, evidence, and user-derived criteria + +Inspected production source on branch `codex/text-diagnostics-9e8r6t` at `9fd76aef3ae2810667d3f2b031fc43692fc04c89`; concurrent evidence work was separate from the frozen compilation inputs. File/line references below refer to that commit unless another commit is named. Read current `AGENTS.md`, `PROJECT_GOAL.md`, and the relevant integrated checkpoint. The parent owns shared coordination and reports issue 11 claimed/rechecked in comment 911. No separate ownership claim is made here. + +The parent's issue 11 task supplies the user's complaint about `description ""`, `parent none`, and similar unnecessary noise, requesting omission. `PROJECT_GOAL.md` user-owned Textual notation requirements (lines 90–96) additionally require reduced clutter/cognitive load, easy human/AI editing, and textual development of large and small models; its General description requires graphical/textual parity and Git-friendly text. Those requirements imply these evaluation criteria: + +1. Remove mechanically repeated values that absence already determines unambiguously. +2. Keep distinctions that affect engineering meaning and make exceptional choices visible. +3. Preserve full persistent-model fidelity and authored source during ordinary editing. +4. Maintain one understandable omission contract; avoid magic value pruning or parallel drifting formatters. +5. Keep current source compatibility, deterministic output, identities, references, and reviewable diffs. + +This is a repository-source investigation, not a new runtime experiment. Existing executed evidence is identified explicitly; other behavioral conclusions are static analysis and need the proposed evaluators before implementation acceptance. + +## Principal findings + +- **The two reported omissions already parse.** Project description initializes empty at `language.rs:1499`; element/relation descriptions default empty at 1705/1783; top-level parent defaults absent at 1708 and 2018. Nested parent is the lexical owner, with conflicting explicit parent rejected at 1710–1718. The spec already permits these (`docs/language.rst:81`, 1636, 1672, 1813). +- **Generation introduces the noise.** `language.rs:650–747` emits project description, every element description/parent, relation description, and serialized default payloads. `source_project.rs:726–744` shares declarations but independently writes the project description in the modular entry. Neither writer compares values with source omission defaults. +- **Some defaults already disappear.** `OccurrenceBody::Exchange.operation`, empty `NodePlacement.port_anchors`, and absent `TraceBaseline.semantic` have serialization skips (`model.rs:561`, 1556, 1604). Do not describe the current writer as emitting literally every field. +- **Default merging is shallow and typed.** `language.rs:1995–2005` replaces supplied top-level values; it does not merge nested constructor fields with a different constructor's factory payload. A selected constructor obeys its own deserializer. Nullability is not a universal omission rule (`docs/language.rst:157–165`). +- **Source defaults are not graphical factory defaults.** `ElementBody::new` creates strict OA/SA scenario viewpoints (`model.rs:808–817`), but source omission deliberately uses `MixedLegacy` (`language.rs:1734–1742`; `docs/language.rst:658`). Omitting `OperationalActivity` or `Functional` by comparison with the factory would silently change the model. +- **A direct shared-writer replacement is unsafe.** Lossless update computes before/after declaration text with `format_declarations` (`source_project.rs:853–854`). `SyntaxUpdate::record` erases every actual field absent from after (1293–1305). With compact after text, changing a Scenario precondition can erase its explicitly authored unchanged defaults, violating `docs/language.rst:1891–1898`. Pure Rename is not a discriminating witness: header changes are separate, and unchanged field trees short-circuit at 1191–1192. Canonical export and lossless edit need separate rendering policies over shared typed data. + +## Candidate contract + +An **omission default** is a value assigned by the admitted source version, declaration kind or selected constructor, and lexical context when a particular named field is absent. It is not necessarily the model factory's default and is not equivalent to any empty, false, zero, or null value found anywhere. + +For canonical generated source, omit a field exactly when (a) its specific source context permits absence, (b) the complete typed value is identical to that omission default, and (c) omission preserves all persistent distinctions, including floating-point sign where promised. Preserve every other field. Keep required constructor tags, required payloads, names, identities, aliases, list entries/order, and references. A whole optional composite may be omitted if its complete typed value equals its default; otherwise render it using its own constructor/record omission rules. + +Canonical flat output remains flat: a non-root element's `parent @owner` is necessary and remains. Nested authored/new syntax may omit parent equal to the lexical owner. It must never permit nested `parent none` to override ownership. Root absence means no parent; it does not relax model rules requiring particular kinds to have owners. + +Explicit spellings of default values remain accepted. Empty description is exactly the empty string; whitespace-only strings are retained, with no trimming. Optional string `none` differs from `""`. Omission does not suppress completeness warnings or validate an otherwise invalid model. Empty records still use the existing brace grammar; one-line formatting is a separate optional readability decision, not necessary to implement this contract. + +## Declaration-level inventory + +All values in this table are **already supplied on absence by the inspected parser**. Candidate canonical policy is to omit equal values, except required slots explicitly identified below. Current full serialization writes most of them. The declaration decoder is `language.rs:1705–1880`; body construction is `model.rs:754–855` and 1324–1425; normative catalogs are `docs/language.rst:87–145`, 257–334, 1049–1218. + +| Declaration/context | Omission defaults eligible for compact generation | Fields that remain mandatory or exceptional | +| --- | --- | --- | +| Project | `description ""` | Version, project alias/name, canonical explicit ID. | +| Every element | `description ""`; parent absent at document level or equal to lexical owner when nested | Non-root flat `parent`; lexical-parent conflicts remain errors. | +| Entity/Actor/Role/Activity/Capability/Mission, SA System/Actor/Function/Capability/Mission, StateMachine/Region | No additional payload | Names/IDs remain, even for otherwise empty bodies. | +| Process/FunctionalChain | `steps []` | Nonempty ordered occurrence list is retained; containment does not imply list membership. | +| ExchangeItem | `kind Information`, `mechanism Unspecified`, `supertype none` | Other kinds/mechanisms/references retained. | +| ExchangeCategory | `family Work`, `exchanges []` | Root-only semantic constraint remains. | +| DataType | `definition { String { minimum_length 0; maximum_length none; }; }`, `default none`, `unit ""` | Another selected definition uses its own contract below. | +| DataField | `type_ref String`, `minimum 1`, `maximum 1`, `unit ""`, `direction Unspecified`, `storage Value`, `default none` | `maximum none` is unbounded and MUST remain; `default []` is present empty collection and MUST remain. | +| Interface | `supertype none` | Membership remains identified relations. | +| Port | `legacy none` | `contract` is explicitly required (`language.rs:1720`); never omit even `Legacy`. | +| Scenario | `viewpoint MixedLegacy`, `context none`, empty pre/postconditions, `steps []` | Strict graphical viewpoints remain explicit. | +| Lifeline | `represented none` | Required ownership/engineering checks unchanged. | +| Occurrence | `body` equal to complete empty Exchange constructor; empty pre/postconditions | Nondefault constructor tag retained, even if its optional payload fields all disappear. | +| State/Mode and each named state vertex | `kind` equal to declaration-selected StateKind; empty entry/exit/do_activity | Explicit mismatching kind remains invalid. | +| Event | `body { Signal { items []; }; }` | Other event constructors and required fields retained. | +| Constraint | `expression ""`, `constrained []` | No change to expression semantics. | +| Every relation | `description ""` | `source`, `target` are required. ContextualLink has structured endpoints. | +| InterfaceItemAllocation | `order 0`, `transmission Unspecified`, `acquisition Unspecified` | Explicit nondefaults retained. | +| DataAssociation | Empty source/target roles; minima 0; maxima none; navigability false | Zero/none defaults here must not be reused for DataField. | +| Interaction/FunctionalExchange | `items []` | FunctionalExchange `source_port` and `target_port` required. | +| BoundaryExchange / CommunicationMean | `items []`; `oriented true` / `oriented false` respectively | False on BoundaryExchange and true on CommunicationMean MUST remain. | +| CapabilityExtend / SequenceLink | `condition ""` / `guard ""` | Endpoint requirements retained. | +| StateTransition | `transition_kind External`; empty trigger/guard/effect; `triggers []`, `effects []` | Local/Internal and nonempty work/event references retained. | +| StateAction | `phase Do` | Entry/Exit retained. | +| ContextualLink | Only common description default | `owner`, `source`, `target`, `purpose` explicitly required, despite construction placeholders. | +| Remaining relations | Only common description default | Required endpoints retained. | +| Diagram | `description ""`, `layer Oa`, `projection Overview`, `routes []`, complete default `view_options` | SA layer and other projections retained. | +| Node | `position { x +0.0; y +0.0; }`, `size { width 224.0; height 94.0; }`, `collapsed false`, `port_anchors []` | `diagram`, `element` required; no name in header; negative zero must not disappear. | +| Artifact | `description ""`, `relation References`, `uri ""`, `revision none`, `fragment none` | `owner` explicitly required; optional present empty strings retained. | +| Trace | `description ""`, `kind Refinement`, `baseline none` | `oa`, `sa` explicitly required; historical content is not disposable. | + +`Id::nil()` placeholders used in relation/default record construction are not source omission permission. Required-field checks run separately (`language.rs:1785–1802`, 1837–1868). + +## Nested constructors and records + +This inventory separates established omission contracts from static findings needing targeted verification/spec reconciliation. **Do not expand parser admission merely to make every nested empty field disappear.** Whole declaration-field omission and selected-constructor-field omission are different operations. + +| Typed context | Candidate compact omission | Retain / evidence limit | +| --- | --- | --- | +| Occurrence Exchange | Absent exchange/sender/receiver/reply_to/operation → none; message_kind → Asynchronous; values → [] | Existing executed OA/SA evidence and spec, cited below. | +| Occurrence Action / State / SequenceReference | Optional action/lifeline, state/lifeline, sequence → none | Keep Action/State/SequenceReference tag; absence of whole body selects Exchange. | +| Occurrence ScenarioUse | scenario → none | bindings required even `[]`. | +| Occurrence Control / Fragment / Branch | None | Required kind; operator/branches; guard/occurrences respectively, including empty string/list. | +| MessageValue | path → [] | field and expression required, even empty expression. `model.rs:519–525`; `docs/language.rst:703`. | +| OperationUse | allocation → none | Preserve present `operation {}`: unresolved operation intent differs from absent operation. `model.rs:527–532`; spec 712–715. | +| LifelineBinding | None | referenced/local required (`model.rs:514`). | +| MessageKind Timer | None | expression required, even `""` (`model.rs:500–510`). Other variants are unit constructors. | +| DataTypeDefinition Integer/Real | Optional minimum/maximum → none (static serde finding) | Spec table labels these fields required (935–944), so audit and reconcile that wording before relying on omission in new output. | +| DataTypeDefinition String | maximum_length → none (static serde finding) | minimum_length is required inside explicitly selected String, even 0. Spec required-column ambiguity as above. | +| DataTypeDefinition Record | supertype → none (static serde finding) | Selected Record tag retained; same spec ambiguity. | +| DataTypeDefinition Boolean / Enumeration | No Boolean payload; no Enumeration field omission | literals required; empty literal string is not a default. `model.rs:324–342`. | +| TypeRef / DataValue | None inside selected payload | Named reference and Boolean/Integer/Real/String/Enumeration values required; false/0/"" are real values (`model.rs:298–304`, 347–355). | +| PortContract | None | FunctionInput incoming_items, FunctionOutput outgoing_items, Component kind/orientation required; Legacy is a unit tag (`model.rs:380–392`). | +| LegacyPortContract | interface → none (static serde finding) | direction required (`model.rs:374–377`); audit before compact output. | +| Event Signal / Time / Change | None inside a selected constructor | items; kind/expression; expression required (`model.rs:450–461`). Whole default Signal body can still disappear at declaration level. | +| OccurrenceEndpoint / ContextualLinkPurpose | None | references and occurrence; Precedence guard / Exchange exchange / CommonWork work required (`model.rs:537–547`), including references [] and guard "". | +| Projection StateMachine | None | machine required (`model.rs:1431–1439`). | +| ViewOptions | hidden_edges []; relation_kinds All; work_categories All; participant_categories All; coverage All; work_contexts All; exchange_labels Name | Struct has field defaults (`view_options.rs:24–74`). Only([])/Any([]) payloads and their tags remain: they do not equal All. | +| EdgeRoute | waypoints []; label none | edge required; retain the route entry itself, even if it then contains only edge (`model.rs:1503–1508`; spec 1158–1162). | +| Position / Size / EdgeLabel / PortAnchor | None inside a supplied record | x/y; width/height; fraction/offset; port/side/offset required (`model.rs:216–225`, 1512–1516, 1541–1545). Optional whole default Node position/size differs from required nested coordinate presence. | +| TraceBaseline / TracedProperties | Absent semantic already omitted | oa/sa snapshots and their name/description strings required, including empty strings (`model.rs:1595–1605`). | +| Semantic historical baseline | No new recursive pruning in this candidate | Dedicated typed writer and token-slot validation (`language/semantic_source.rs:87–188`). Preserve applicability tags, payloads, identities, selectors, and list entries. `Value none`, NotApplicable, absent facet, and present empty values are distinct (`docs/language.rst:1287–1294`). | + +Reusable executed evidence: [constructor omission review](release-constructor-defaults-review-20260911.md) and its [experiment](experiments/constructor-defaults-20260911/README.md) record 54 accepted comparisons and 12 required-field refusals for source 7 across OA/SA, including full Project comparisons, lossless name/no-op checks, and scenario MixedLegacy cases. They do not establish other constructor families or all older versions. Their canonical outputs intentionally expand defaults and remain historical evidence; do not rewrite them as if the old experiment tested the proposed writer. + +## Counterexamples to generic pruning + +- `maximum none` in a DataField cannot disappear: absence means 1, not infinity. +- `default []` differs from `default none`; `default { Boolean false; }` differs from no default; a String value `""` is still a value. +- `operation { allocation none; }` may compact to `operation {};` after verification, but removing `operation` changes explicit unresolved intent into ordinary channel semantics. +- `work_categories { Any []; };` and `relation_kinds { Only []; };` select nothing; omission selects All. +- `body { Branch { guard ""; occurrences []; }; };` must retain the required fields. A selected `Signal {}` is not currently sufficient even though omitting a whole Event body selects Signal with empty items. +- Root parent omission is safe; dropping flat non-root parent loses containment; nested explicit none is invalid, not redundant syntax to silently repair. +- Node position with x = -0.0 must remain explicit under the signed-coordinate-zero guarantee (`docs/language.rst:1837–1840`). Ordinary Rust/JSON numeric equality may equate signed zeros; omission comparison must preserve the required bits. +- Empty historical description/name snapshots and typed `Value none` are evidence, not boilerplate to prune. + +## Writer/update boundaries and implementation alternatives + +| Candidate | Noise / clarity | Fidelity / compatibility | Maintainability / decision | +| --- | --- | --- | --- | +| A. Delete only empty description and root parent in public writers | Addresses examples, leaves many repeated defaults and nested references | Small change, but still unsafe if shared updater comparison is compacted | Useful probe, incomplete issue scope; reject as final design. | +| B. Recursively remove empty/none/false/zero or compare everything against GUI factories | Superficially compact | Fails the counterexamples, required fields, and MixedLegacy semantics | Reject. Value shape is not a contract. | +| C. Source-context default contract, compact public/generated syntax, complete internal comparison | Removes every inventoried redundant default while retaining required distinctions | Can preserve existing parser admission, lossless bytes, IDs and historical facts | **Recommended smallest complete design.** Small explicit typed contexts, shared data extraction/rendering, focused tests. | +| D. New syntax/version with universal nested optionality and a general schema/reflection framework | Could reduce more required empty payloads, at learning/compatibility cost | Alters admission and historical interpretation; larger migration surface | Defer; no demonstrated need to change language semantics for the reported problem. | + +Recommended implementation shape (names are illustrative, not an API commitment): + +1. Separate source-default construction from graphical creation where they differ. Give parser and compact writer a common source-context default definition. Keep explicit required-field gates separate. Freeze older-version defaults; do not replace them with the current factory accidentally. +2. Retain one typed field extraction/writing path with an internal purpose such as `CompleteForEdit` versus `CompactGenerated`. CompleteForEdit remains an internal comparison representation, not another user-facing export. It must explicitly synthesize every admitted optional/default slot in its applicable typed context, including currently serde-skipped `Exchange.operation`, `NodePlacement.port_anchors`, and `TraceBaseline.semantic`; simply retaining today's serializer does not produce a complete comparison. CompactGenerated applies the allowlisted declaration/constructor rules; no global recursive emptiness predicate and no JSON persistence serialization changes. +3. `format` and `format_bundle` use CompactGenerated. Share or consistently apply project-header description omission. Deterministic UUID/category order, alias use, indentation, escapes, LF, final newline, and explicit IDs stay as specified. +4. Lossless update compares complete old/new shapes and preserves unchanged actual tokens, including explicit defaults. A changed explicitly authored field may be updated to its default in place. A previously absent field is inserted only when its semantic value changes. Compact canonical export is the operation that normalizes all redundant spellings. +5. Render genuinely new declarations and inserted/replaced value fragments using compact context-aware generation, while feeding complete shapes to the comparison algorithm. Audit this split carefully: `source_structure.rs:201–223` currently receives the same after map for new declarations; `source_project.rs:1327–1336` copies insertion values from comparison text. Changing only top-level exporter calls leaves generated edit noise; changing all after maps corrupts lossless behavior. Constructor changes must preserve shared authored fields/comments. +6. Do not rewrite untouched source or old evidence fixtures to create a misleading impression of coverage. Update normative canonical wording (`docs/language.rst:174–177`, 1842–1843, 2210–2211) and any canonical-dependent tests/examples alongside the implementation. The user-owned specification is untouched. + +## Compatibility and peer boundary + +The inspected public source parser admits versions 3–7 (`language.rs:1476–1481`), current schema constant is 7 (`model.rs:9`), and canonical format emits the current version. Omission already admitted by the same reader does not itself require a schema change. Do not change older-version admission gates, migrations, or UUID stabilization. Omitted IDs generate identities during authoring; canonical exports must continue writing all IDs for repeatable identity and Git behavior. + +If broader constructor testing exposes behavior/spec disagreement, resolve it explicitly in the appropriate versioned contract; do not silently reject existing accepted source or add untested defaults. In particular, DataTypeDefinition's required-column wording needs a narrow compatibility clarification if static Option omission is confirmed. Retain V1/V2 legacy writers/adapters (`language.rs:2226` onward) as frozen compatibility mechanisms unless a separately justified change is needed. + +Parent supplied and fetched peer S2c commit `f1d79df4979121335ed7b4ce4da84b2fca8a9ef4`. This investigation inspected its exact `language/analysis_source/write.rs` header and commit stat via `git show`: it describes a **private** bounded typed writer, explicitly not connected to public formatting, version dispatch, or source edits. This is not evidence of integrated runtime behavior. Parent reports issue 7 comment 910 labels it source-only/runtime-unverified, with public source8/source-project integration outstanding. The peer is on another machine; matching local paths do not establish its current state. + +Before implementation, parent must coordinate the public declaration/default helper and source-project update interfaces with that peer against named fetched commits. Do not modify or generic-prune the private analysis/expression writer or assume source7 rules apply to analysis constructors, exact numeric values, operators, or parameter binding. Once source8 public wiring is identified, audit its added fields explicitly, preserve its budget/error contracts, and test the combined result. This candidate neither reserves nor changes peer-owned files. + +## Acceptance evaluators (proposed, not run here) + +1. **Independent authoring matrix:** Handwritten explicit-versus-omitted sources with fixed UUIDs for every declaration default and each eligible nested field, individually and in combination, both layers when legal. Compare with literal typed expectations, not expectations generated by the omission function. Include all-default and one-nondefault variants; preserve engineering warning behavior. +2. **Constructor refusal matrix:** Remove each required field from selected constructors and assert its specific missing-field diagnostic. Cover empty Branch strings/lists, Signal items, port lists/contracts, contextual endpoint paths, positions, MessageValue expression, projection machine, and trace snapshot strings. Check unknown/duplicate fields remain errors. +3. **Canonical invariants:** For valid representative models, `parse(format(P)) == P`, `format(parse(format(P))) == format(P)`, and `compile_bundle(format_bundle(P)).project == P`; verify identities, list order, baselines, optional values, and float bit patterns separately where ordinary equality is weaker. Cover fully empty projects, nested models exported flat, SA diagrams, strict scenarios, nondefault orientations, unbounded multiplicity, empty selected filters, and unresolved operation intent. +4. **Output contract checks:** Assert absence of each eligible field at its actual typed path, not global string absence. Assert mandatory/nondefault counterexamples remain. Test multiline nonempty/whitespace descriptions, escaping, non-ASCII names, signed zero, and deterministic output. Tests that mutate canonical text via replacing now-omitted defaults must instead author the invalid case explicitly; search for such tests (e.g. `tests/language.rs:791`, 1041). +5. **Lossless transformations:** Explicit-default and omitted-default fixtures with comments, CRLF/LF, unusual number/string spelling, nested declarations and multiple files. No-op and unrelated Rename/Alias edits retain defaults and bytes. Also require unrelated payload changes at each record level: Scenario precondition with explicit common defaults, node position with explicit `port_anchors [];`, an Exchange sibling with explicit `operation none;`, and a baseline prose snapshot with explicit `semantic none;`. These exercise the record-erasure branch that pure Rename bypasses. Default→nondefault inserts only necessary fields; nondefault→default updates safely; constructor changes retain shared fields/comments; reparent/move preserves lexical semantics; new declarations/fields use compact generation. Recompile exact expected model and check source diffs. Existing `tests/source_project.rs:900` omission-preservation tests are a starting point, not coverage of this newly identified explicit-default deletion risk. +6. **Version matrix:** Existing frozen old-version fixtures still migrate to identical full models and reports; explicit disallowed new fields still reject even when default-valued. Existing source7 readers parse the new compact source7 output. Combined source8 checks follow named integrated peer code, not assumptions about S2c. +7. **Human/AI and GUI workflow:** From a fresh small OA/SA model, inspect generated Text, create and edit a contained work/sequence/diagram, Apply, navigate to graphical state, change a description/value, return to Text and Save/reopen. Assess whether only meaningful nondefaults and necessary relationships remain, with no hidden semantic change or unexplained expansion. Use a larger multi-module model for a scoped change and inspect Git diff locality. Track boilerplate field count and changed lines against fixed models, but require correctness and a task-based readability review, not a token-count target alone. +8. **API/source parity:** Exercise flat and modular exports and ordinary source updates through the public consumers that call the shared helpers. Persistent JSON/API models remain identical. Run focused core source/lossless tests first, then project-required integration checks once at the combined identified commit. Fresh independent review should challenge the context inventory, signed-zero handling, and lossless insertion/comparison split. + +## Open implementation risks and next handoff + +The largest verified design risk is lossless updater coupling; the next are source-vs-factory default drift, nested required/optional ambiguity outside the existing occurrence experiment, and numerical equality weaker than the promised persistence fidelity. Peer source8 integration adds an unverified boundary requiring coordination, not speculative implementation here. Output-dependent tests and documentation currently assert explicit defaults and must be reconciled without erasing historical evidence. + +Parent should publish this candidate, solicit focused criticism, settle the contract/peer interface, then implement and run the evaluators after its frozen issue 12 acceptance boundary permits source changes. No portion of this static inventory establishes that issue 11 is fixed or that the current public artifact already generates compact source. diff --git a/harness/text-default-omission-review-20260913.md b/harness/text-default-omission-review-20260913.md new file mode 100644 index 00000000..026bb090 --- /dev/null +++ b/harness/text-default-omission-review-20260913.md @@ -0,0 +1,18 @@ +# 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.