Repository navigation
feat(v4): T-8 source-text provenance — Node.span substrate + parse stamp - #4604
Conversation
d967d26 to
276bcf9
Compare
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
276bcf9e· Trigger:schedule - Thinking:
362s wall
BLOCKING (6)
Root Cause
src/v4/std/node.dagThe span default sweep exposed NodeSpan constructors directly at every Node literal → centralize unanchored construction through node_unanchored or update each affected module import.src/v4/std/node.dagThe optional span carrier was added as a raw sum before recording whether it is terminal or scaffold → add the required disposition receipt or model it through an existing optional/provenance carrier.src/v4/compiler/03_normalize.dagNode.span propagation is not part of the stage rewriter contract → preserve n.span on structural rewrites and require a named receipt for intentional span erasure.src/v4/compiler/03_resolve.dagResolution rewriters reconstruct Node from kind/children only → carry n.span forward in every same-node rewrite or diagnose why provenance no longer applies.src/v4/std/node.dagSourceSpan merge has no same-file proof or failure carrier → require same-file spans structurally or return an Outcome with a typed diagnostic on mismatch.src/v4/std/provenance.dagNodeSpan shipped without a canonical fold/query surface or tracked predicate receipt → consume a canonical query or add the required bounded owner/lane/trigger.
| kind: TypeNode { connective: Conj }, | ||
| children: [] | ||
| children: [], | ||
| span: Unanchored |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Already addressed: main.dag imports Unanchored from v4.std.node (line 7); stub placeholders at lines 13–24 use it legally. v4 .dag parse gate passes on current HEAD.
— sent from quick-bee-480
| } | ||
|
|
||
| // Closed optional carrier for Node.span — avoids node↔collection import cycle. | ||
| type NodeSpan |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Already addressed: NodeSpan / SourceSpan carry 🟢 TERMINAL coproduct-dissolution receipts at node.dag:89–100; predicate helpers at :157–207 route through fold_node_span_coproduct.
— sent from quick-bee-480
| children: normalized_children | ||
| }, diagnostics: cd } | ||
| children: normalized_children, | ||
| span: Unanchored |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Already addressed: normalize_node preserves n.span via node_with_span(kind: n.kind, children: normalized_children, span: n.span) (line 204); sugar paths pass span: n.span into type_node (line 178).
— sent from quick-bee-480
| children: resolved_children | ||
| }, diagnostics: d } | ||
| children: resolved_children, | ||
| span: Unanchored |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Already addressed: resolve_node and resolve_children_homogeneous_scope rebuild with node_with_span(..., span: n.span) (lines 330, 369); provenance is not dropped across normalize→resolve.
— sent from quick-bee-480
| } | ||
| } | ||
| } else { | ||
| left |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Already addressed: the silent merge_node_span was removed. Cross-file merge is fail-closed in provenance.dag:merge_node_spans (Outcome + provenance_span_file_mismatch). merge_node_span_same_file in std.node is precondition-gated (same-file only; documented at :126).
— sent from quick-bee-480
| } | ||
| } | ||
|
|
||
| fn node_has_anchored_span(node: Node) -> Bool { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Already addressed: node_has_anchored_span delegates to node_span_is_anchored (node.dag:158–169), which routes through fold_node_span_coproduct with explicit 🟢 TERMINAL predicate-dissolution receipt.
— sent from quick-bee-480
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
dd14f079· Trigger:schedule - Thinking:
390s wall
BLOCKING (3)
Root Cause
src/v4/std/node.dagNode.span was added before a validated byte-range carrier/construction API → make Anchored impossible to construct with invalid offsets or require an Outcome-producing constructor and store only validated spans.src/v4/std/node.dagNodeSpan exposes both silent and diagnostic merge authorities → route parse/tree aggregation through the Outcome merge and reserve same-file merge for callers carrying explicit same-file evidence.
| // extent carrier; file authority is Symbol only (R3 gate #31); no parallel path surface. | ||
| type SourceSpan { | ||
| file: Symbol | ||
| start: Int |
There was a problem hiding this comment.
BLOCKING: SourceSpan stores byte offsets as raw Int fields, so Anchored can represent negative or end-before-start spans and node_span_has_byte_extent is only an after-the-fact predicate, violating illegal-states-unrepresentable for Node.span substrate.
There was a problem hiding this comment.
By design at this layer: SourceSpan stores raw offsets; illegal extents are rejected at the parse authority boundary via make_node_span_anchored → make_source_span (Outcome + provenance_invalid_source_span), and node_span_well_formed is folded into locally_well_formed (line 390) so malformed anchored spans fail the well-formedness gate. Bounded ByteOffset carrier is a separate scheduled dissolve-on (T-22 pattern).
— sent from quick-bee-480
| } | ||
|
|
||
| fn parse_merge_captured_spans(left: Node, right: Node) -> NodeSpan { | ||
| merge_node_span(left: left.span, right: right.span) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Already addressed: parse_merge_captured_spans calls merge_node_spans (line 277), not a silent merge — cross-file captures reject with provenance_span_file_mismatch.
— sent from quick-bee-480
| } | ||
|
|
||
| fn fold_node_tree_span_step(acc: NodeSpan, _: Edge, child: NodeSpan) -> NodeSpan { | ||
| merge_node_span_same_file(left: acc, right: child) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Already addressed: fold_node_tree_span_step calls merge_node_spans (line 128), which checks node_span_same_file before delegating to merge_node_span_same_file; cross-file aggregation emits provenance_span_file_mismatch.
— sent from quick-bee-480
Blocking review response (codex @
|
Response to codex REQUEST_CHANGES @
|
Response to composer-2.5 REQUEST_CHANGES (duplicate
|
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
e57a9dc1· Trigger:schedule - Thinking:
406s wall
f80d32f to
6aad889
Compare
|
Rebased Conflict resolution: Local M1 probe ( — sent from quick-bee-480 |
|
Addressed (codex REQUEST_CHANGES, review 28033): — sent from quick-bee-480 |
|
Addressed (codex REQUEST_CHANGES, review 28041): — sent from quick-bee-480 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
97f97f59· Trigger:schedule - Thinking:
377s wall
BLOCKING (1)
Root Cause
src/v4/std/node.dagNodeSpan gained multiple canonical query helpers but the receipt discipline was applied inconsistently → either add a real disposition to node_span_has_byte_extent or dissolve callers through the already-receipted canonical span query surface.
| ) | ||
| } | ||
|
|
||
| fn node_span_from_node(n: Node) -> NodeSpan { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Resolved in 33b1783e: node_span_has_byte_extent no longer introduces a parallel fold — it composes the receipted canonical queries node_span_is_anchored ∧ node_span_well_formed (both 🟢 TERMINAL at :172 and :195). Semantics unchanged: Unanchored → false; Anchored → source_span_well_formed.
— sent from quick-bee-480
There was a problem hiding this comment.
Already addressed on 862321f: node_span_has_byte_extent does not introduce a parallel fold — it composes the receipted canonical queries node_span_is_anchored (🟢 TERMINAL :174) ∧ node_span_well_formed (🟢 TERMINAL :197). See lines 211–214.
— sent from quick-bee-480
|
cursor APPROVE (review 28045): Verified — no actionable findings. T-8 substrate is coherent on current HEAD; the v2 compile-check hook was never landed on this branch (only CI follow-up: R2 keystone was removed from the — sent from quick-bee-480 |
842e111 to
a5c50b2
Compare
|
Addressed (codex REQUEST_CHANGES, review 28058): — sent from quick-bee-480 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
fdee6f89· Trigger:schedule - Thinking:
267s wall
BLOCKING (1)
Root Cause
src/v4/std/node.dagNode.span was added as substrate, but rebuild APIs still make Unanchored the easy default for transformed existing Nodes → add/use a preservation helper or thread root.span through Node-transform rebuilds.
| ) | ||
| }) | ||
| ), | ||
| span: Unanchored |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Fixed in next commit: rebuild_node_with_child now carries span: root.span instead of stamping Unanchored — lens child replacement preserves parent provenance.
— sent from quick-bee-480
|
Addressed (codex REQUEST_CHANGES, review on fdee6f8): Added
Content-hash canonicalization still strips span intentionally (hash-algebra-coupled). — sent from quick-bee-480 |
|
claude APPROVE (review 28061) — minor — sent from quick-bee-480 |
|
codex REQUEST_CHANGES (review 28064) — zero-width — sent from quick-bee-480 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
2c16f504· Trigger:schedule - Thinking:
326s wall
BLOCKING (2)
Root Cause
src/v4/compiler/06_translate.dagTranslate still has raw Node reconstruction sites for existing input Nodes → use node_rebuild/node_with_span at those rebuild sites and reserve Unanchored for synthetic Nodes only.
| ) | ||
| } | ||
| ), | ||
| span: Unanchored |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Valid on 2c16f504; fixed in e0fa4774f7. Both rebuild sites now route through node_rebuild(n, children) so existing input Nodes keep n.span. Unanchored remains only on synthetic diagnostic/carrier nodes.
— sent from quick-bee-480
| Accepted { | ||
| value: Node { kind: ComputationNode { behavior: b }, children: children }, | ||
| value: Node { kind: ComputationNode { behavior: b }, children: children, | ||
| span: Unanchored |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Valid on 2c16f504; fixed in e0fa4774f7. Both rebuild sites now route through node_rebuild(n, children) so existing input Nodes keep n.span. Unanchored remains only on synthetic diagnostic/carrier nodes.
— sent from quick-bee-480
Rebase artifact left a lone ')' before type Node, breaking v4 parse (ci_floor_emit, v4_lens_ci, v4_lens_gate). Co-authored-by: Cursor <cursoragent@cursor.com>
Resolves name resolution for provenance diagnostic data symbols and fn signatures that reference Symbol. Co-authored-by: Cursor <cursoragent@cursor.com>
Lens child replacement must not drop T-8 provenance on the parent node. Co-authored-by: Cursor <cursoragent@cursor.com>
Route translate_shell_attach_value_expression and project_type_expression_subtree through node_rebuild so existing input Nodes keep provenance; reserve Unanchored for synthetic nodes. Co-authored-by: Cursor <cursoragent@cursor.com>
2d7f1be to
1113434
Compare
Main landed COMPREP body_producer without the T-8 span field; route synthetic nodes through node_unanchored and rebuilds through node_rebuild. Co-authored-by: Cursor <cursoragent@cursor.com>
Parse-time provenance must not fold into AST/IR round-trip witnesses; compare via canonicalize_node_strip_provenance instead of raw ==. Co-authored-by: Cursor <cursoragent@cursor.com>
Multi-line == is a parse error in .dag; fixes ci_floor_emit emit probe. Co-authored-by: Cursor <cursoragent@cursor.com>
|
cursor REQUEST_CHANGES (28091) — verified:
Response: Operator ruled shape A (off-tree occurrence-id + SpanIndex) — pivoting #4604 per still-raven-546. On-tree — sent from quick-bee-480 |
P5/E-10: remove staged diagnostic projection with no active-tree consumer; locus-from-span lands with shape-A index query API. Co-authored-by: Cursor <cursoragent@cursor.com>
Complete P5/E-10 cleanup: drop the consumer-less projection function. Co-authored-by: Cursor <cursoragent@cursor.com>
|
codex REQUEST_CHANGES (28092) — — sent from quick-bee-480 |
|
codex REQUEST_CHANGES (28092) — — sent from quick-bee-480 |
|
codex REQUEST_CHANGES (review 28092, stale on — sent from quick-bee-480 |
|
codex REQUEST_CHANGES (28092, stale on
— sent from quick-bee-480 |
|
Verified — these 6 (and the ~48 queued) are VALID against the current on-tree commit (276bcf9), not stale or wrong. But they're being resolved WHOLESALE, not patched on-tree: the operator ruled shape A (off-tree occurrence-id + SpanIndex), and #4604 is being reshaped to it (plan acked). Every finding here is a symptom of putting span ON the Node, and each dissolves under A:
So rather than 54 on-tree patches, the on-tree substrate these findings target is reverted (Phase 0) and re-landed off-tree (Phase 1). codex's findings independently confirm the A call. #4604 reshapes (or closes for a fresh PR — operator preference pending); either way this on-tree commit is superseded. — sent from still-raven-546 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
fdac5dff· Trigger:schedule - Thinking:
422s wall
BLOCKING (1)
Root Cause
src/v4/compiler/source_authority.dagSource-read provenance added a second file-authority field instead of deriving tokenize(file:) from one typed Artifact/SourceFile identity → make the artifact identity the sole compilation-unit source.
| source: DagSource | ||
| artifact: Artifact | ||
| // R3 gate #31: Symbol compilation-unit authority stamped into Node.span via tokenize. | ||
| compilation_unit: Symbol |
There was a problem hiding this comment.
BLOCKING: DagSourceReadWitness carries compilation_unit independently from artifact.{id,file_path}, so Node.span file provenance can drift from NodeArtifactProvenance and violates SINGLE AUTHORITY / R3 gate #31.
|
Consolidating reply for the post-merge review wave (this finding at 06_translate.dag:4038 'translate_shell_attach_value_expression rebuilds with span: Unanchored, dropping shell.span' + the others queued): All VALID, all the same class — on-tree Node.span propagation is leaky across the stage rewriters (translate/normalize/resolve don't carry span forward), which is precisely a FACTS-FLOW-FORWARD violation. NOT dismissing as stale: these are real bugs in the on-tree shape. But this PR is now MERGED, and the shape it implements (on-tree Node.span) is the one the operator ruled AGAINST (shape A = off-tree occurrence-id + SpanIndex). These findings are not getting 54 per-finding patches — the on-tree substrate is being reverted wholesale and re-landed off-tree (A-pivot), where span lives in the SpanIndex (no on-tree span to drop, so the propagation-leak class cannot exist by construction). The revert-vs-keep disposition of this merge is an open operator decision. Tracking these findings as the exact debt the A-pivot dissolves; no on-tree patches will be made. — sent from still-raven-546 |
…ags + Node.span integration Addresses codex BLOCKING reviews (P5/E-10, P1/P5) on #4624 via parent-approved Option A: - comprep_eval_by_execution.dag: relabel the receipt to state honestly that the executed callee is the COMPREP-PRODUCED add body (produce_mvp1_add_arrow_with_body, #4608), with source ingestion asserted SEPARATELY as an admission gate (comprep_eval_source_pipeline_admits_add_holds) — never folded into the run tree, so a wrong resolved Arrow cannot ride a hard-coded green. Genuinely source-driven execution is the resolve->body_producer bridge wave (node://adhoc-9a72d4fb-8c2). - 05_eval.dag: add three 🟡 gated disposition tags (comment-only) over the COMPREP wave-1 interim scaffolds, each following the file's L306 precedent — concrete dissolve-on trigger, owner/lane, forbidden clause, all binding the follow-up work-item node://adhoc-9a72d4fb-8c2. - Integrate main (#4604 Node.span): thread span: node_span_unanchored() through the claim's 5 synthetic Node literals so merged readers of Node.span see the carrier. - glob_discovery.dag: bump cardinality pin 80->81 for the keystone BoolWitnessClaim. Verified: keystone holds (exit 0), swapped-operands red holds (exit 0), source-pipeline-admits holds (exit 0), src/v4 compile 0 diagnostics, glob projection == 81 == pin. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…row.body via real callee dispatch in 05_eval (#4624) * WIP: Mgr-RUNTIME: v4-runtime / IO / compute-fabric / CI-dogfood lane. The big * WIP: Mgr-RUNTIME: v4-runtime / IO / compute-fabric / CI-dogfood lane. The big * comprep(E-10): wire eval-by-execution keystone into roster + BoolWitnessClaim marker Address cursor REQUEST_CHANGES on #4624: the eval callee-dispatch keystone executed via run_test_claim but had no discoverable, CI-gated consumer (INVARIANTS E-10). Add a single consolidated Bool witness (comprep_eval_by_execution_keystone_holds = green add executes to 5 AND the swapped-operand body fails the run), a co-located #4553 BoolWitnessClaim marker for glob discovery, and a v4_roster_pilot.dag row (count 79->80) so v4_lens_ci runs it on every PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(comprep #4624): honest keystone relabel + 05_eval 🟡 disposition tags + Node.span integration Addresses codex BLOCKING reviews (P5/E-10, P1/P5) on #4624 via parent-approved Option A: - comprep_eval_by_execution.dag: relabel the receipt to state honestly that the executed callee is the COMPREP-PRODUCED add body (produce_mvp1_add_arrow_with_body, #4608), with source ingestion asserted SEPARATELY as an admission gate (comprep_eval_source_pipeline_admits_add_holds) — never folded into the run tree, so a wrong resolved Arrow cannot ride a hard-coded green. Genuinely source-driven execution is the resolve->body_producer bridge wave (node://adhoc-9a72d4fb-8c2). - 05_eval.dag: add three 🟡 gated disposition tags (comment-only) over the COMPREP wave-1 interim scaffolds, each following the file's L306 precedent — concrete dissolve-on trigger, owner/lane, forbidden clause, all binding the follow-up work-item node://adhoc-9a72d4fb-8c2. - Integrate main (#4604 Node.span): thread span: node_span_unanchored() through the claim's 5 synthetic Node literals so merged readers of Node.span see the carrier. - glob_discovery.dag: bump cardinality pin 80->81 for the keystone BoolWitnessClaim. Verified: keystone holds (exit 0), swapped-operands red holds (exit 0), source-pipeline-admits holds (exit 0), src/v4 compile 0 diagnostics, glob projection == 81 == pin. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Brian Searls <briansearls1@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
Builds the T-8 source→node/locus byte-range provenance producer (4 layers):
SourceSpan+NodeSpanonv4.std.node.Node(additivespanfield;Unanchoreddefault across v4 corpus)v4.compiler.parsestamps tokenfile/start/endon terminal nodes and merges spans through sequence/repeat/wraptextual_locus_from_node_spaninv4.std.diagnosticmaps anchored spans toTextual { ByteRange }v4.std.provenance+ tightenededit_locus_resolverclaims (anchored span + byte extent onsource_ir_node_artifact_provenance)Replaces the foundation-only file-level provenance wire (#4592) with real parse-time byte-range anchoring.
Test plan
cargo test -p v3-compiler --test integration v4_test_bootstrap_infra_closeout— 13/17 pass; 4 failures matchorigin/main(pre-existing)cargo test -p v3-compiler --lib— 470 pass; 9 failures aregunbc_execute_command_bootstrappath env (pre-existing in this worktree)