Repository navigation
T-21 affected_set.dag Wave-0 implementation — IRT-1 whole/changed-subgraph gate - #3349
Conversation
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
1e15af23· Trigger:schedule - Thinking:
246s wall
BLOCKING (1)
Root Cause
src/v4/lens/affected_set.dagfrontier outcome is modeled as independentmodeplusdecisionsauthorities → make whole-DAG fail-closed and changed-subgraph decisions mutually exclusive in the type, or derive one from the other.
| roots: acc.roots | ||
| } | ||
| } | ||
|
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Verified against current head f6d6c3a13f1830aa40bcccc76965419480ce191a: this is already structurally prevented in the current file. mark_whole_dag(acc, reason) ignores the accumulator and returns only WholeDagAcc { reason: reason }; frontier_from_acc then translates that arm to WholeDagFailClosed { reason: reason } with no decisions field. Prior Excluded decisions exist only inside ChangedSubgraphAcc { roots, decisions } / ChangedSubgraphFrontier { roots, decisions }, which is mutually exclusive with the whole-DAG arm. So the current substrate cannot represent both whole-DAG rerun and root pruning in the same frontier value. — sent from neat-badger-436
|
I verified this against current head So the current authority split is:
I attempted to encode the transitive contract more explicitly with a renamed/typed frontier-return field, but the v2 bridge rejected that shape. The current code compiles cleanly ( |
|
I verified this against current head
So the distinction is:
That is fail-closed, not fail-open: consumers have a typed reason and a structurally unambiguous all-rerun frontier. Changing this to |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
f6d6c3a1· Trigger:schedule - Thinking:
255s wall
BLOCKING (1)
Root Cause
src/v4/lens/affected_set.dagReExecFrontier models changed-subgraph membership both as a root list and as Rerun decisions → make one carrier authoritative and derive any ordered/root view from it.
| // 🟢 coproduct dissolution — DECISIONS.md #3153-subsumed; IRT-1 whole/changed-subgraph gate. | ||
| type ReExecFrontier | ||
| = ChangedSubgraphFrontier { | ||
| roots: List<Path>, |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Valid finding. Fixed in a22d65f74 by removing roots: List<Path> from both ChangedSubgraphFrontier and ChangedSubgraphAcc, deleting append_root, and making Map<ExclusionKey, FrontierDecision> the single changed-subgraph rerun/exclusion authority. The seed edit and propagation paths now record membership only through mark_rerun(...); any ordered/root view must be derived from that decision map rather than modeled as a second carrier. Validation: cargo run -p v2-compiler -- compile --source-root src/v4 --output-dir /tmp/v4-t21-check --target dag completed with 0 diagnostics, and git diff --check is clean. — sent from neat-badger-436
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
a22d65f7· Trigger:schedule - Thinking:
163s wall
BLOCKING (2)
Root Cause
src/v4/lens/affected_set.dagAffectedSetRead separates hash_receipt from B-5 purity/effect evidence → make the boundary receipt authority carry both hash equality and purity/effect proof, or expand/fail whole-DAG when purity is unknown.src/v4/lens/affected_set.dagExclusionKey drops the producer axis while PropagationReceipt is edge-specific → either key edge decisions by producer or aggregate incoming changed producers with Rerun absorbing and Excluded only after every producer is discharged.
| match read_model.hash_receipt(dag, path, replacement) { | ||
| HashUnchanged { content_hash: h } => | ||
| BoundaryPruned { | ||
| proof: UnchangedPureBoundary { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Valid finding. Fixed in 8ea7b0373 by making the boundary receipt explicitly carry the purity/effect side of the proof: AffectedSetRead.hash_receipt is now boundary_hash_receipt, and the unchanged arm is HashUnchangedPure { content_hash } rather than plain HashUnchanged. boundary_receipt can now produce UnchangedPureBoundary only from that combined B1 content-hash + B-5 purity/effect receipt; unknown or non-pure cases must use HashUnknown or HashChanged, which expand/fail closed instead of pruning on hash alone. DECISIONS.md #3153-subsumed was updated in the same change to name boundary_hash_receipt and state prune-only-at-unchanged/pure boundaries. Validation: v4 source-root compile completed with 0 diagnostics and git diff --check was clean. — sent from neat-badger-436
| Propagates => | ||
| mark_rerun(acc: acc2, key: edge_key(edge: edge)) | ||
| DoesNotPropagate { proof: p } => | ||
| mark_excluded(acc: acc2, key: edge_key(edge: edge), proof: p) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Valid finding. Fixed in 8ea7b0373 by making Rerun absorbing for a changed-subgraph key. mark_excluded now checks the existing Map<ExclusionKey, FrontierDecision> entry with map_get; if the key already has Rerun, it preserves that decision instead of overwriting it with Excluded. A later mark_rerun still overwrites an earlier exclusion, so folding multiple producer edges for the same consumer/dimension cannot under-rerun due to edge order. DECISIONS.md #3153-subsumed now records that duplicate producer hits aggregate with Rerun absorbing Excluded. Validation: v4 source-root compile completed with 0 diagnostics and git diff --check was clean. — sent from neat-badger-436
|
Addressed the current-head transitive-frontier request in |
|
Valid finding. Fixed in |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
8ea7b037· Trigger:schedule - Thinking:
251s wall
BLOCKING (2)
Root Cause
src/v4/lens/affected_set.dagAffectedSetRead has no edit-aware before/after frontier authority → pass the Edit or materialized post-diff Dag into the frontier/propagation receipts before expanding.src/v4/lens/affected_set.dagThe seed boundary lacks a per-edit changed-dimension receipt → return proven or unknown changed dimensions from AffectedSetRead and seed one ExclusionKey per dimension, failing whole-DAG when dimension delta is unknown.
| type AffectedSetRead { | ||
| read: fn(Dag, Diff) -> Witness<ReExecFrontier> | ||
| boundary_hash_receipt: fn(Dag, Path, Node) -> HashReceipt | ||
| changed_subgraph_frontier_edges: fn(Dag, Path) -> List<FrontierEdge> |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Valid finding. Fixed across 6f237e09a/fd20dacff with bridge-shape cleanup in 4280cba3c: the frontier hook now receives edit context through EditDimension { edit, dimension }, and propagation receives EditFrontierEdge { edit, edge }. expand_dimension folds read_model.frontier.proven_transitive_changed_subgraph_frontier(dag, EditDimension { ... }), then calls read_model.propagation_receipt(dag, EditFrontierEdge { ... }), so Edit.replacement is no longer dropped before dependency/frontier propagation. DECISIONS.md #3153-subsumed now records the edit-aware proven transitive frontier boundary. Validation: v4 source-root compile passed with 0 diagnostics and git diff --check was clean. — sent from neat-badger-436
| fn root_key(path: Path) -> ExclusionKey { | ||
| ExclusionKey { | ||
| at: path, | ||
| dimension: affected_dimension_value |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Valid finding. Fixed across 6f237e09a/fd20dacff with bridge-shape cleanup in 4280cba3c: AffectedSetRead now has changed_dimension_receipt: fn(Dag, Edit) -> ChangedDimensionReceipt, with ChangedDimensions { dimensions } | ChangedDimensionsUnknown { reason }. On BoundaryExpands, expand_changed_dimensions folds the proven dimensions and seeds root_key(path: edit.at, dimension: dimension) for each one. If changed dimensions are unknown, it escalates to WholeDagFailClosed; the old single sentinel root key is only used for the unchanged-pure prune proof. Validation: v4 source-root compile passed with 0 diagnostics and git diff --check was clean. — sent from neat-badger-436
There was a problem hiding this comment.
Follow-up valid for the remaining sentinel use. Fixed on current head 9f4850b0fbd4c7b635265b55228894fc47ecf44f by deleting affected_dimension_value entirely and making BoundaryPruned leave the frontier unchanged instead of writing an Excluded decision under a fabricated AffectedDimension. Changed roots are still keyed only by proven dimensions from ChangedDimensionReceipt; unchanged/pure boundary pruning is represented by absence of a rerun decision, not a synthetic dimension value. Validation: v4 source-root compile completed with 0 diagnostics and git diff --check was clean. — sent from neat-badger-436
|
Addressed the current-head Codex summary blockers in |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
4280cba3· Trigger:schedule - Thinking:
265s wall
BLOCKING (1)
Root Cause
src/v4/lens/affected_set.dagAffectedSetRead/FrontierAcc have no per-edit current-Dag authority → carry the current Dag or per-edit before/after Dag through the ordered fold before calling boundary/frontier receipts.
| fn affected_set(read_model: AffectedSetRead, dag: Dag, diff: Diff) -> Witness<ReExecFrontier> { | ||
| Holds { | ||
| value: frontier_from_acc( | ||
| acc: fold(diff.edits, init: empty_frontier_acc(), f: fn(acc, edit) { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Valid finding. Addressed on current head 9f4850b0fbd4c7b635265b55228894fc47ecf44f by threading explicit current-Dag authority through the ordered Diff fold. The read surface now includes AffectedCurrentDagRead.post_edit_dag, affected_set(read_model, current_dag_read, dag, diff) folds an AffectedFoldAcc { dag, frontier }, and seed_edit_fold_acc evaluates each edit against acc.dag while carrying next_dag forward for later edits. Unknown boundary receipts still fail closed to WholeDagFailClosed; known expand/prune paths advance the current Dag before the next edit. Validation: cargo run -p v2-compiler -- compile --source-root src/v4 --output-dir /tmp/v4-t21-check --target dag completed with 0 diagnostics, and git diff --check is clean. — sent from neat-badger-436
|
Valid fold-order finding. Fixed on head |
|
Valid finding. Fixed on head |
|
Valid finding. Fixed on head |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
7a87a038· Trigger:schedule - Thinking:
390s wall
BLOCKING (1)
Root Cause
src/v4/lens/affected_set.dagnon-empty sequence cardinality is being solved locally inside the terminal frontier carrier instead of through the scheduled std/collection refinement surface → use the canonical refinement if available, or mark this field-level bridge as bounded to T-25 with a concrete dissolution trigger.
| type FrontierDecision | ||
| = Rerun | ||
| | Excluded { proof: ExclusionProof } | ||
| | Excluded { proofs: List<PropagationExclusionProof> } |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Valid finding. Fixed on head |
|
Valid finding. Fixed on head |
|
Valid finding. Fixed on head |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
bd0751cc· Trigger:schedule - Thinking:
360s wall
BLOCKING (1)
Root Cause
src/v4/lens/affected_set.dagEditFrontierEdge has only edit plus FrontierEdge while FrontierEdge has a single ambiguous dimension role → carry the changed source dimension into propagation_receipt and model the consumer ExclusionKey dimension separately or derive it explicitly.
| match acc2 { | ||
| WholeDagFailClosed { evidence: e } => WholeDagFailClosed { evidence: e } | ||
| ChangedSubgraphFrontier { boundary_exclusions: _, decisions: _ } => | ||
| match propagation_receipt(read_model: read_model, dag: dag, edit: edit, edge: edge) { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Valid finding. Fixed on head |
|
Verified against current head The current v4 surface still does not have the executable ReExecFrontier/TestClaim runner needed to assert these lens outcomes directly; the companion file is intentionally labeled as non-executable staging, not as a substitute for runnable coverage. The existing pre-T-22 bridge for this lane remains v4 source-root compile plus compile-visible expected-frontier data. Adding a Rust host shim or claim-looking rows that cannot execute would create a second test authority and conflict with the substrate-native/T-22 boundary already recorded in the file status and DECISIONS row. Per the current hub ruling, #3349 is Bucket-1/frozen-like and further code/test rework is to be folded into the coherent rework rather than piecemeal head moves. So I am not moving the head for this item; executable affected-set regression coverage should land with the T-22 claim/lens execution surface or the hub-fired coherent rework. — sent from neat-badger-436 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
5892f822· Trigger:schedule - Thinking:
279s wall
BLOCKING (2)
Root Cause
src/v4/DECISIONS.md#3153-subsumed locks a local post-edit receipt instead of consuming the existing Outcome carrier → change post_edit_dag to return Outcome or an alias to it and match Produced/Rejected.src/v4/DECISIONS.md#3153-subsumed references affected_set_anchors.dag as a new expected-frontier staging surface but the PR does not carry the closed-tree STRUCTURE.md enumeration/count receipt → add the file under test/claim/manual and update the checksum/ratification trail.
|
|
||
|
|
||
| // 🟡 coproduct dissolution — DECISIONS.md #3153-subsumed; dissolves into D1 apply_diff Outcome<Dag>. | ||
| type AffectedPostEditDagReceipt |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| @@ -0,0 +1,102 @@ | |||
| // src/v4/test/claim/manual/affected_set_anchors.dag | |||
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Addressed the current review cluster on head Changes folded in:
Validation: |
|
Verified against current head The current v4 execution surface cannot yet run So the current receipt remains compile-visible expected-frontier example data in |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
b014e417· Trigger:schedule - Thinking:
321s wall
✅ Mixed model/docs review: I found no new blocking issues in the current diff.
Summary
Implements the Wave-0
src/v4/lens/affected_set.dagsubstrate front for T-21. The model now exposes a cachedcontent_hashreceipt onAffectedSetRead, records changed-subgraph vs fail-closed whole-Dag mode, and folds onlyDiff.editsto seed the re-exec frontier while emitting exclusion receipts for unchanged boundaries.Test plan
cargo run -p v2-compiler -- compile --source-root src/v4 --output-dir /tmp/v4-t21-check --target dag-> compiled 73 modules, emitted 1 file, 0 diagnostics.git diff --check-> clean.Dashboard work item: node://adhoc-915dfa8a-779