127 lines
8.1 KiB
Markdown
127 lines
8.1 KiB
Markdown
# 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.
|