Files

127 lines
8.1 KiB
Markdown
Raw Permalink Normal View History

# S1 contract independent review — 2026-09-06
Read-only review of `release-chain-assembly-contract-probe-20260906.md`, its
isolated resolver probe, current validation/reference/source/recovery seams, and
the proposed S2 contract. No production edits, Cargo, or native input.
The central representation is sound: explicit caller owner, ordered reference
**occurrence** IDs, and a leaf occurrence distinguish repeated and nested uses.
Keeping old direct SequenceLink unchanged avoids inventing historical semantics.
The three closed purposes appropriately avoid equating common work with merged
execution. The following corrections should be frozen before production wiring.
## 1. Deterministic admission must not depend on resolver cache history
**P1 contract correction.** `Index::resolve` in the probe rejects a new address when
`cache.len() >= max_addresses`. The contract proposes sharing the accepted-revision
index with GUI endpoints, validation and other consumers. Together, those choices
can make a model's apparent validity depend on which preview addresses were visited.
Minimal falsifier: max_addresses=1; authored model needs only valid address B.
Fresh index resolves B. On the same accepted Project, first preview unrelated
valid A, then resolve B: the probe returns `resolved address limit`. Reversing the
interaction order changes the result without changing the model or authored budget.
The probe demonstrates a cache cap, not yet deterministic model admission.
Separate three budgets: deterministic counts of **authored** path components,
per-request/per-analysis expansion work, and cache storage capacity. Evict or skip
cache insertion when cache capacity is reached; never turn a cache miss into a
structural-invalid endpoint. Oversized requested expansion may explicitly refuse
or return an incomplete-analysis notice. Test cold/warm/evicted cache and changed
preview order yielding identical structural results and exact endpoints.
## 2. Shared source7 must freeze nested historical message serialization
**P1 compatibility correction.** S1 alone adds a new constructor and can preserve
existing element serialization. S2 adds `operation` inside existing Exchange
occurrence bodies. The combined migration cannot reconstruct old fingerprints by
changing only Project.schema_version and borrowing all current `p.elements`.
Current `recovery/source5.rs::Project5` deliberately serializes borrowed elements;
after a normally serialized default `operation: None` is added, an old message gains
`"operation":null` in those bytes. That changes its fingerprint even though the
accepted source has only a version-token change. The current empty-element literal
fingerprint test would not catch this. Current semantic recovery also explicitly
clears impact approvals on the admitted upgrade; preserve that behavior, rather
than treating unchanged engineering facts as proof that old plan approval survives.
Use one coordinated version7 admission/migration owner. Frozen source5 and source6
views must omit new message fields and reject new constructors, including explicitly
empty/default new fields under old advertised versions. Either a historical
serializer or carefully proved omission policy can preserve the old byte shape;
neither should be assumed from serde defaults alone. Pin old5/6 literal Projects
containing Call, Reply, values, direct SequenceLink, semantic history, and a saved
state/view selector, plus invalid raw workflow/property/source drafts. Verify exact
old hashes before restoring/replanning; do not rewrite raw invalid buffers.
New source7 combined S1+S2 snapshots need separate exact restore tests. This is a
coupled migration gate, not two independent schema-number increments.
## 3. Freeze deletion versus retarget refusal; never shorten an address
**P2 contract ambiguity.** The probe correctly rejects paths after referenced
records are removed/retargeted, but that does not define the normal Delete command's
accepted result. The proposed DTO has no unresolved owner/path/leaf representation.
The implementation must choose explicit cleanup or reject the whole command; it
cannot simply drop a deleted path component and reinterpret the same leaf directly.
Smallest consistent policy: normal Delete of caller/path reference/leaf removes
each dependent ContextualLink in the same reference-cleanup transaction, retaining
unrelated links and reusable definitions. Reidentify rewrites every exact typed
component. A generic reference-target edit that would invalidate a surviving link
rejects; an explicit previewed repair batch may delete/replace that link. Deleting
an assembly-purpose exchange/work similarly removes or explicitly repairs its
dependent link, never changes its purpose to Precedence. Freeze this policy or a
clearly specified alternative, then test Undo/source restoration and R2 isolation.
Also test caller-owner deletion when both leaves survive in external procedures:
using generic leaf source/target accessors alone will miss that ownership edge.
The accepted owner/reference visitor must include owner, each path member, leaf,
and purpose work/exchange. A source module update must follow caller ownership,
not the defining module of a surviving leaf.
## 4. Define which edges feed analysis before declaring the graph complete
**P2 semantic decision.** The contract distinguishes Precedence from Exchange and
CommonWork, but does not yet state which feed entry/exit/reachability/control
cardinality and behavior-coverage diagnostics. Current `validate_sequence` builds
direct SequenceLink adjacency; treating every new purpose as another execution
edge could create false split/merge warnings or suppress missing-coverage warnings.
Ignoring all contextual edges could report the accepted R1.B connection unreachable.
Freeze separate projections: contextual precedence adjacency, exchange/work
assembly incidence, and coverage evidence. Specify how direct links inside a
referenced procedure are qualified into each caller instance and how an outer
reference-node predecessor relates to entry/exit boundaries; do not count the
collapsed preview and its expanded contributions as two executions. Keep shared
P's own diagnostics separate from caller O's context. On analysis limits, emit an
explicit incomplete result, not success or guessed missing-path warnings.
A bounded discriminating test can use exactly R1/R2, A→B→C and X→R1.B, then add
CommonWork and Exchange purposes independently. Verify their intended diagnostic
effects and unchanged standalone P/R2 facts. This does not require a simulator or
a general execution semantics framework.
## Additional implementation checks, not new scope
- Generic source/target accessors returning leaf IDs are safe only after auditing
caller-owner consumers. New contextual self-links must compare full addresses:
R1.B→R2.B is not a self-link merely because the leaf ID is equal. Behavior
navigation must select the explicit caller, not the leaf's defining procedure.
- Transition closure needs caller, every reference occurrence/target sequence,
leaf, purpose work/exchange and their mappings. A target path is mapped component
by component and re-resolved; neither the global work ID nor the first mapping
is enough. Include two references to one callee and a conflicting retarget case
in one frozen-base semantic-review test.
- Qualified source/target/purpose are coupled values. Semantic review can expose
closed facets but must validate the complete composed relation and compare actual
slots across rows; owner/path changes cannot silently reinterpret old leaf drafts.
- The probe's Precedence branch currently returns Ok without production guard-text
or work/exchange adjacency checks, and it does not enforce the proposed ban on
all-direct contextual Precedence. Its pass is endpoint-resolution feasibility,
not complete admission parity. Add those decisive negatives when implementing;
retain existing direct SequenceLink tests unchanged.
No further material address-identity defect was found in this bounded read. The
proposal remains viable after these decisions; none requires broadening S1 into
scenario gates, physical paths, scheduling, or arbitrary cross-owner edges.