Skip to content

R3 Verification Mgr — lane through R3 close - #3060

Merged
briansrls merged 2 commits into
mainfrom
session/neat-raven-162
May 14, 2026
Merged

briansrls merged 2 commits into
mainfrom
session/neat-raven-162

Conversation

@briansrls

Copy link
Copy Markdown
Contributor

Auto-opened by session-dashboard for session neat-raven-162.
Pushing to session/neat-raven-162 advances this PR.

Worker attestation

Before flipping this PR to ready for review, confirm each item:

  • Title describes the change (not the session id or branch).
  • PR body summarises what and why (replace the TODO below).
  • Tests run: name the command (e.g. npm test, cargo test) and the result.
  • If this closes a work item, the body contains a Closes #N directive.
  • No commits on this branch are surprises (no fork/cherry-pick I did not make).
  • No secrets / credentials / large binaries staged.

Summary

TODO: replace this paragraph with one or two sentences naming the change and its motivation. Reviewers read this first.

Test plan

  • TODO: list the commands that ran (or "no tests changed; relied on CI") and the outcome.

@briansrls
briansrls marked this pull request as ready for review May 14, 2026 04:07
@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: openai-pro / gpt-5-5-pro
  • Commit: f014ae98 · Trigger: manual
  • Comparison: main @ 7efdd2ba ... session/neat-raven-162 @ f014ae98
  • Conversation: View conversation

1. Story of the diff

This PR tightens the GeneratedFromDag test-runner predicate so ResolvedFact is no longer treated like PendingFact plus ignored payload. The runner still consumes the shared output_path, but now, when the manifest arm is ResolvedFact, it requires dag_source to be a declaration reference and source_hash to be a non-empty string before downstream byte-regeneration checks can trust the arm as shape-materialized (src/v3/compiler/src/test_runner.rs:5042-5047, src/v3/compiler/src/test_runner.rs:5101-5141). The added regression test constructs a tiny .dag suite with a ResolvedFact whose source_hash is empty and verifies the predicate fails closed at the runner boundary (src/v3/compiler/tests/integration/test_runner_test.rs:669-706).

2. Invariant categories

  1. LAYER MODEL (substrate vs implementation).

N/A — this is implementation-only runner logic plus a Rust regression test; it does not add or alter a Dag substrate type, dag.rs shape, cross-pass stored fact, or new substrate variant.

  1. INVARIANTS.md + modeling-discipline.md.

Compliant — fail-closed / facts-flow-forward: ResolvedFact now rejects missing or malformed dag_source via ClaimResult::Fail (src/v3/compiler/src/test_runner.rs:5109-5121) and rejects missing, non-string, or empty source_hash (src/v3/compiler/src/test_runner.rs:5122-5140) instead of allowing downstream consumers to infer whether resolution data was real.

  1. CODING.md.

Compliant — the change stays within the existing data-and-pattern-match style: it decomposes variant_payload locally (src/v3/compiler/src/test_runner.rs:5102-5108) and returns explicit ClaimResult::Fail values for each invalid boundary shape (src/v3/compiler/src/test_runner.rs:5111-5119, src/v3/compiler/src/test_runner.rs:5124-5138), with no new hidden state, object hierarchy, or mutable builder surface.

  1. TESTING.md.

Compliant — the added test is behavior-driven and focused on one contract: an empty ResolvedFact.source_hash fails closed (src/v3/compiler/tests/integration/test_runner_test.rs:669-706). The fixture is small, and the assertion targets the runner result rather than incidental manifest internals.

  1. LOCKED DESIGN DECISIONS.

N/A — the diff does not modify a locked design document or introduce a semantic divergence from the Pure Bootstrap / verification direction. It narrows an existing runner acceptance path by making ResolvedFact carry real resolution shape before the later byte-regeneration layer consumes it (src/v3/compiler/src/test_runner.rs:5044-5047).

  1. TRACKED vs UNTRACKED DEBT.

Compliant — no new scaffold, TODO, temporary file, or extra authority is introduced. The only forward reference is to the later byte-regeneration check, and this PR reduces the interim gap by ensuring ResolvedFact is no longer accepted with inert dag_source / source_hash payload (src/v3/compiler/src/test_runner.rs:5044-5047, src/v3/compiler/src/test_runner.rs:5101-5141).

2.5. Top-down PM intent review

Compliant — the change preserves the high-level verification intent: a generated-from-DAG manifest entry marked ResolvedFact must now provide structural evidence that it is resolved, rather than relying on a label while ignoring the payload (src/v3/compiler/src/test_runner.rs:5101-5141). I do not see a PM-level dilution where a must-have verification target is postponed, converted into permanent scaffolding, or redirected into a parallel hand-maintained authority; the Rust test is a focused regression for the existing runner surface, not a new verification architecture (src/v3/compiler/tests/integration/test_runner_test.rs:669-706).

3. Verdict

APPROVE. The PR is a narrow fail-closed tightening of an existing verification predicate, with an appropriate regression test and no substrate, locked-design, or debt-tracking issues visible in the diff.

@briansrls
briansrls merged commit 7540bdd into main May 14, 2026
8 checks passed
@briansrls
briansrls deleted the session/neat-raven-162 branch May 14, 2026 04:24

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review metadata

  • Provider / model: codex / unknown
  • Commit: f014ae98 · Trigger: schedule
  • Thinking: 237s wall

BLOCKING (1)

Root Cause

  • src/v3/compiler/tests/integration/test_runner_test.rs The new ResolvedFact negative test reused the older non-generated-path fixture shape → switch the fixture to a generated manifest path so source_hash validation is the first failing obligation.

⚠️ One test fixture masks the behavior this PR is trying to verify.

file_name: "pb_test_file_generated_from_dag.v3",
predicate: GeneratedFromDag(census_authority, [
ResolvedFact {
output_path: "src/v3/compiler/tests/integration.rs",

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BLOCKING: This fixture uses a path already outside the generated-file authority, so the runner fails on path membership before reaching the new empty-source_hash diagnostic; use a known GENERATED_FILES path to make the P3 fail-closed check actually exercise the added branch.

briansrls added a commit that referenced this pull request May 14, 2026
* WIP: R3 Verification Mgr — lane through R3 close

* docs(r3): clarify L5 status bucket derivation

* docs(r3): downgrade L5 gate to scaffold evidence
briansrls added a commit that referenced this pull request May 14, 2026
…precondition) (#3095)

* docs(r3-v-l5): canvas — L5 corpus-policy substrate (gate #15 CONSUMER_LANDED → PASSING precondition)

Research-only canvas that enumerates the four Corpus Policy facts
(docs/design-cross-target-equivalence.md §"Corpus Policy") missing
from the HEAD L5 corpus rows landed via PR #3060 + #3039, and routes
the carrier shape to Director per INVARIANTS §P1 before any
src/v3/std/verification.dag edit.

Five Q's (effect class / numeric policy / coverage reason / expected
observation+oracle / per-row attachment shape) with structurally
distinct options + named disqualifiers + canvas-preliminary
recommendations. No substrate edits, no new TestPredicate variants;
dispatch sequence + post-ratification PR plan included.

Closes worker-side authoring for adhoc-6e83e29b-200 (R3 gate #15
T-V-L5-Corpus).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(r3-v-l5): fix Corpus Policy cardinality six → seven (cursor BLOCKING)

* docs(r3-v-l5): fix INVARIANTS anchor P5 → P2/P3 on Q2-B3 disqualifier (cursor APPROVE_WITH_COMMENTS)

* docs(r3-v-l5): correct ForAllTargets field summary — input_ref exists; ProgramOutputBind is a doc-comment, not a field (cursor BLOCKING)

* docs(r3-v-l5): L5CorpusRowPolicy uses typed TestClaim edge, not String name key (briansrls BLOCKING P2)

* docs(r3-v-l5): CoverageReason — add C4 with typed per-arm payload edges; drop coverage_description prose slot (briansrls BLOCKING P2)

* docs(r3-v-l5): reconcile carrier (single L5CorpusRow in std.r3_l5_corpus) + recommendation summary C3 → C4 (openai-pro REQUEST_CHANGES)

Two slips in the substrate handoff:

1. §5 recommendation summary said "A1 + B1 + C3 + D2 + E2" but C3 was
   disqualified earlier; the canvas recommends C4 (typed per-arm
   coverage payload). Updated to "A1 + B1 + C4 + D2 + E2".

2. Q5-E2 said `L5CorpusRow { claim, policy: L5CorpusRowPolicy }` (a
   two-record wrapper in `std.r3_l5_corpus`), but §5 declared a flat
   `L5CorpusRowPolicy { claim, ... }` placed in `verification.dag`.
   Reconciled to a single flat `L5CorpusRow` carrier in a new
   `src/v3/std/r3_l5_corpus.dag` module — Q5-E2 module placement, no
   parallel authority.

PR-3 / PR-4 dispatch-sequence references updated; boundary-consumer
ratchet refers to `L5CorpusRow` throughout.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(r3-v-l5): Q1 disqualify A1/A3 — EffectShape axis mismatch with locked Corpus Policy taxonomy (codex BLOCKING)

EffectShape's IsIdempotent|IsBreaking partition classifies along
idempotency, not along the locked design-cross-target-equivalence.md
§"Side-effect Policy" axis Pure|ControlledStdout|TypedFailure|
DeferredEffectful. Reusing it would narrow a locked policy taxonomy
into a different one (INVARIANTS §P1 faithfulness violation).

- Q1: disqualify A1 + A3 on axis mismatch; recommend A2 (new
  CorpusEffectClass) — orthogonal to EffectShape, not parallel.
- §2 facts table row 24 + summary paragraph: state that EffectShape
  exists but along a different axis.
- §5 substrate delta: add `type CorpusEffectClass`; L5CorpusRow.effect
  field type CorpusEffectClass; recommendation summary A1 → A2.
- §5 boundary-consumer ratchet: reference CorpusEffectClass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(r3-v-l5): align §1 + §6 PR-2 landing surface with §5 (new r3_l5_corpus.dag module; no verification.dag substrate edit) (cursor BLOCKING)

* docs(r3-v-l5): tighten §5 — import line lives in L5 fixture, not verification.dag (cursor BLOCKING)

* docs(r3-v-l5): Q2 — split NumericPolicy into two independent axes (int + float); B1 disqualified for forced mutual exclusivity (briansrls BLOCKING P2)

`NumericPolicy = Int64OverflowFree | NamedOverflowSemantics |
FloatExcluded | FloatPolicyDeferred` collapsed two orthogonal axes
into one sum, so a row mixing Int and Float observables could not
state both at once. New B4 option = two-axis record carrying both
`IntOverflowPolicy` and `FloatPolicy` simultaneously.

- Q2: B1 disqualified on forced mutual exclusivity; B4 added +
  recommended (carries both axes per row).
- §5 substrate delta: NumericPolicy now record `{int, float}` with
  two closed-sum types.
- Summary recommendation: A2 + B1 + C4 + D2 + E2 → A2 + B4 + C4 + D2 + E2.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant