Skip to content

Resolve all five codegen hacks; update parse node examples with real responses - #32

Merged
briansrls merged 8 commits into
mainfrom
claude/review-todos-8zr6W
Feb 4, 2026
Merged

briansrls merged 8 commits into
mainfrom
claude/review-todos-8zr6W

Conversation

@briansrls

Copy link
Copy Markdown
Contributor

Summary

This PR completes the resolution of all five codegen hacks documented in TODO/TODONE/TODO_hacks.md and TODO/TODO_consolidate_di.md. The primary focus is updating parse node examples across three DAGs to test both real transport responses (with exact output assertions) and skip propagation, plus making dependency injection violations explicit in gist-ops.

Key Changes

1. Parse Node Examples: Real Transport Responses (Hack #2 - FIXED)

All parse node examples now provide realistic transport responses instead of only testing skip propagation:

  • bootstrap/graph_mock.rs: parse_scan_result now includes a ShellResponse::ok() example with exact assertions on crate_count and crate_names
  • ci/graph_mock.rs:
    • parse_deps_exists — added FileResponse { exists: Some(true) } example
    • parse_codegen_exists — added FileResponse { exists: Some(true) } example
    • parse_codegen_result — added ShellResponse::ok() with skip=false example
    • parse_build — added ShellResponse::ok() with skip=false example
    • parse_test — added ShellResponse::ok() with skip=false example
  • lib/llm-ops/src/graph_mock.rs: All four LLM parse nodes (openai, anthropic, code_review, secrets) now include real mock_openai_response() or mock_anthropic_response() REST examples

Each parse node retains its skip propagation example as a second test case, ensuring both paths are covered.

2. Dependency Injection: Make Violations Explicit (Hack #1 Phase 1 - FIXED)

lib/gist-ops/src/lib.rs:

  • Removed non-injectable wrapper functions sanitize_branch_for_filename() and generate_gist_filename()
  • Consolidated to single injectable variants requiring &FilesystemHandle and SystemTime parameters
  • GistOps::PrepareRequest::execute() now explicitly constructs FilesystemHandle and captures SystemTime::now() inline with a comment marking this as a Phase 2 DI violation
  • All tests updated to use fixed timestamps, making them fully deterministic
  • Updated all test helpers to pass required dependencies explicitly

3. OutputMap Migration (Consolidation §15 - COMPLETED)

Migrated remaining raw HashMap::new() output construction to OutputMap:

  • lib/tools/cargo/src/ops.rs: 3 sites in Mockable::mock_outputs()
  • lib/tools/deps/src/graph.rs: 2 sites in Mockable::mock_outputs()
  • lib/primitives/src/collection.rs: 1 site in SetOp::execute()

Remaining HashMap usage (4 sites in core/ir/src/transport/cli.rs, 2 test helpers) cannot use OutputMap due to dependency constraints.

4. Documentation Updates

Implementation Details

  • ValueExpr pipeline: The value_expr.rs pipeline already supported serializing Value::Response and Value::Request to Rust source; no codegen changes were needed — only the graph_mock examples required updating.
  • Deterministic tests: All gist-ops tests now use fixed SystemTime values, eliminating race

https://claude.ai/code/session_017jwfqhevE97hrxHZdk8fGL

- Hack 2: Add real transport response examples to all parse node
  examples (bootstrap, CI, LLM) — exercises actual parsing logic,
  not just skip propagation. Skip-path examples retained alongside.

- DI Items 1+5: Remove non-injectable wrappers in gist-ops.
  sanitize_branch_for_filename() and generate_gist_filename() now
  require explicit FilesystemHandle and SystemTime parameters.
  Production caller constructs them at call site (visible for Phase 2).
  All tests updated to use fixed timestamps (deterministic).

- Consolidation §15: Migrate 6 remaining HashMap::new() output sites
  to OutputMap builder in cargo/ops.rs, deps/graph.rs, collection.rs.

- Update TODO docs to mark completed checkboxes.

https://claude.ai/code/session_017jwfqhevE97hrxHZdk8fGL
Mark Hack 2 checkbox as complete (parse node examples updated in
previous commit). Update status to Done and move to TODONE/.

https://claude.ai/code/session_017jwfqhevE97hrxHZdk8fGL
Remove the non-injectable Installer::new() constructor that hid
Platform::detect() inside its body. All call sites now use
Installer::for_platform() explicitly:

- ops.rs: Installer::for_platform(Platform::detect()) with DI comment
- Tests: Installer::for_platform(Platform::Linux) for determinism
- Default impl: delegates to for_platform(Platform::detect())

Phase 2 will add a PlatformEnv DAG node to inject Platform through
edges instead of detecting inline.

https://claude.ai/code/session_017jwfqhevE97hrxHZdk8fGL
Testgen auto-generates boundary presence (Pattern A), resource
presence (Pattern D), and self-chain validation (Pattern C) tests.
Remove the hand-written duplicates across 7 files:

- 10 Pattern A tests (boundary `.is_some()` checks)
- 4 Pattern D tests (resource `.is_some()` checks)
- 1 Pattern C test (self-chain validation)

Kept: value-checking tests (Pattern B) and failure-scenario tests
(Pattern E) that exercise specific mock spec behavior.

https://claude.ai/code/session_017jwfqhevE97hrxHZdk8fGL
Add AuthMethod::resolve_env_vars() with injectable lookup function that
converts EnvVar(name) → Bearer(value) and EnvVarHeader → ApiKey.
Resolution now happens in TransportOps::Execute (ops.rs) before
calling into the executor. The executor no longer reads env vars
inline — unresolved variants hit a debug_assert.

Auth handling is now fully testable without setting real environment
variables. 8 new tests cover all resolution paths.

Phase 1 is now complete: all 4 DI violations (filesystem handle,
platform, clock, auth env vars) have been made explicit.

https://claude.ai/code/session_017jwfqhevE97hrxHZdk8fGL
Add ToolPathResolver trait with resolve(binary) -> Result<PathBuf>.
WhichResolver is the production implementation (shells out to `which`).
resolve_tool_path() and upsert_tool() now accept &dyn ToolPathResolver.

EnvOp::execute() (the I/O boundary) constructs WhichResolver explicitly,
making the system dependency visible. Tests can inject a mock resolver
that returns fixed paths without shelling out.

Also updates testgen-improvements.md Phase 8.4 checkboxes for the
Pattern A/C/D test deletions completed earlier.

https://claude.ai/code/session_017jwfqhevE97hrxHZdk8fGL
Generated load_step_inputs_from_env() and emit_step_outputs() now accept
&HashMap<String, String> instead of calling env::vars()/env::var() directly.
The env dict is captured once at the top of run_single_step(), making the
single env capture point visible and the helper functions testable with
mock environments.

This completes all actionable DI items in TODO_consolidate_di.md:
Items 1-5 and 7 are all Phase 1 fixed. Only Item 6 remains (already at
the correct boundary, documented as LOW severity).

https://claude.ai/code/session_017jwfqhevE97hrxHZdk8fGL
All actionable dependency-injection violations are now Phase 1 fixed:
- Item 1: FilesystemHandle parameter injection
- Item 2: Installer::for_platform() everywhere
- Item 3: AuthMethod::resolve_env_vars() before executor
- Item 4: Generated CI runners accept env dict
- Item 5: SystemTime parameter injection
- Item 7: ToolPathResolver trait

Item 6 (CI detect env vars) remains LOW severity, already at boundary.

https://claude.ai/code/session_017jwfqhevE97hrxHZdk8fGL
@briansrls
briansrls merged commit 77ba885 into main Feb 4, 2026
1 check passed
briansrls added a commit that referenced this pull request May 9, 2026
…_retired (#2448)

Mark T-Bridge-Retirement gate PASSING with ledger + dag.rs ratchet receipts.
Refresh ROADMAP NominalOpacity / Secret thesis rows for std nominal_opaque
authority and retired name-keyed bootstrap; align §2.3 BridgeLedgerZero prose.

Co-authored-by: Cursor <cursoragent@cursor.com>
briansrls added a commit that referenced this pull request May 10, 2026
…iation per §1.8

Three valid cell-level findings from codex schedule review on sha 1d95e61.
All caught the same root issue: §3 cells didn't reconcile against §1.8
ledger + r3-structure.md canonical authority before landing.

#2 — T-Numeric-Construction blocker (line 424):
   Cell said "Float migration + Real/base-carrier convention HELD on
   proud-raven-495 G2 Phase 2 Substrate S8 ApproximateField<F>" but
   §1.8 #18 + #24 explicitly say "CONSUMER_LANDED + PASSING for
   Grounding G2 primitive rows (2026-05-10, PR #2570 squash b96a51a)"
   — the work landed. Updated cell to: PR #2570 closes the prior HELD;
   remaining blocker is broader Real<N> emission demonstrations under
   S9/Shape-A follow-ons per §1.8 #18 close-criterion.

#3 — T-Bridge-Retirement count (line 427):
   Cell said 3 remaining sub-bridges including mark_bootstrap_secret_
   nominal_opacity, but §1.8 #32 PASSING + §2.3 explicitly says that
   bridge is closed. Corrected count: 3/5 sub-bridges retired (gate #32
   prior-cycle Secret nominal-opacity + gate #33 this cycle canonical
   lens + include_str this cycle), 2 remaining (SourceSpan.file
   participation + patch_lower_helpers residual).

#4 — T-Free-Consequences-Demonstration over-attribution (line 430):
   Cell credited gates #10/#33/#37/#40/#72 to T-Free, but §1.8 assigns
   those to other lanes:
   - #10 → T-V-L4-L7-Direct
   - #33 → T-Bridge-Retirement
   - #37 + #40 → T-CostLens-Composition
   - #72 → T-E-P-Producer-Broadening
   T-Free's canonical demo gate range is #43-#52. Only #43
   (auto_parallelism_independent_binds_emit_parallel) MERGED this cycle
   for T-Free. Updated cell + compile-note to credit each landing only
   to its canonical-lane row.

Compile-note also reconciled per the same §1.8 single-authority pass:
T-CostLens-Composition + T-E-P-Producer-Broadening now credited their
own gates instead of attributing them to T-Free.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
briansrls added a commit that referenced this pull request May 10, 2026
…SING

Two valid findings from codex schedule review on sha f6a3a13 (review
id 4259176210):

#5 — T-Bridge-Retirement count conflated PR-merge with gate-PASSING:
   Cell said "3/5 retired" but §1.8 truth: #32 PASSING, #33 DECLARED,
   #34 DECLARED, #35 PASSING. PR #2449 + PR #2459 ARE merged but the
   gates haven't been promoted from DECLARED → PASSING (separate status
   drift sweep step, e.g., per PR #2399 cadence). Reframed cell to
   distinguish PR-merge evidence from canonical §1.8 status: 2/5
   gate-PASSING (#32 + #35), 2/5 PR-merged-pending-promotion (#33 + #34),
   plus SourceSpan.file participation (Substrate-owned hand-Rust audit
   sites; not in numbered §1.8) + residual semantic patching
   (`bridge_exact_string_semantic_patching_residual` Open per #35
   close-criterion).

#6 — T-Free-Consequences over-claim on PR-merge:
   Cell said "gate #43 MERGED" but §1.8 #43 still DECLARED (PR #2495 is
   evidence toward promotion, not the promotion event). Same fix:
   reframe as PR-merge evidence accruing toward §1.8 gate promotion;
   canonical status authoritative.

Compile-note also reframed: explicitly distinguishes PR-merge evidence
from §1.8 gate-PASSING promotion. PR-merge events are listed as evidence
accruing toward promotion; canonical gate status varies per §1.8.

Common root: future Monday compiles must mechanically reconcile each
"landed/retired" claim against §1.8 status, NOT PR-merge events.
Discipline recorded in feedback_pm_compile_audits_pre_existing_errors
(updated to include PR-merge-vs-gate-promotion distinction).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
briansrls added a commit that referenced this pull request May 10, 2026
…2583)

* docs(r3): §3 lane-status weekly compile (2026-05-11 Monday cadence)

PM-derived compile per §9.1 weekly cadence. Updates Status / Current
dispatch / Blocker / ETA-to-close columns based on observable PR merge
data + worker session activity + silent-ram-834 status report at
gunbc#828 c#4414611117.

Lanes with substantial movement this cycle:
- T-LensProducer-Retirement: gate #5 lens_apply.rs in flight (valiant-otter-715)
- T-Numeric-Construction: u128 mirror sync MERGED #2526; gates #17 + #20 active
- T-Free-Consequences-Demonstration: 6 gates merged (#10/#33/#37/#40/#43/#72)
- T-Bridge-Retirement: 2/5 sub-bridges retired (PR #2459 + #2449)
- T-Lens-Behavioral-Parity: #73 + #78 active under Substrate Mgr
- T-Debt-Paydown (standing): Mgr re-spawn (gentle-newt-665 → silent-ram-834);
  Phase 3 fleet 8/10 closed/absorbed; orphan PR #2503 closed
- T-Omni-Shape-B: gate #25 salvage path under PB Mgr; #26/#27 mis-parented

Lanes with no observable change this cycle:
- T-V-L4, T-V-L5-Corpus, T-FixedPoint, T-Anthropic-Wire, T-V2-Retirement,
  T-Tests-As-Data-Completeness — substrate work continues but no clear
  gate-level deltas surfaced

Mgr canvas refreshes remain formal authority per §3 framing; lane-owning
Mgrs may correct/override any PM-derived cell.

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

* docs(r3): address codex BLOCKING findings on PR #2583 §3 compile

3 valid findings from codex review:

1. T-Lens-Behavioral-Parity status was "RED→YELLOW (PM-derived; Mgr
   ratification welcome)" — created parallel-representation hedge in a
   single-authority cell (INVARIANTS P2 violation; per
   feedback_parallel_representation_debt). Resolved: commit fully to
   YELLOW as the PM-compiled value (the §3 disclaimer note covers Mgr
   override authority). The hedge in the cell was worst-of-both-worlds.

2. PM compile note said T-Tests-As-Data-Completeness had "no observable
   change this cycle" but the table cell records PR #2287 (Verification
   V1 TC1 first slice) MERGED 2026-05-10. Self-contradicting. Resolved:
   moved T-Tests-As-Data-Completeness to "lanes with substantial
   movement" list. Also added T-Anthropic-Wire (PR #2506), T-V2-Retirement
   (PR #2334), T-V-L7 (gate #10 / PR #2394), T-Tier3-Dissolution
   (clever-bear-180 active), T-Lens-Application-Surface (crisp-raven-202
   active) to the movement list — all had cell-level deltas in the table
   that the compile note had missed.

3. PR #2394 merge date inconsistency: T-V-L7 cell said "2026-05-09",
   T-Free-Consequences cell said "2026-05-10". Verified merge timestamp
   2026-05-10T00:26:42Z UTC; corrected T-V-L7 cell to 2026-05-10.

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

* docs(r3): fix T-LensProducer-Retirement blocker (codex BLOCKING #2 on PR #2583)

Pre-existing error in §3 cell that prior PM compile preserved instead of
correcting. The original cell named "T-FixedPoint + R2-Evaluator" as
T-LensProducer-Retirement's blocker, but per the canonical sequence:

- r3-structure.md:357: critical path is `R2-Evaluator → T-LensProducer-
  Retirement → T-FixedPoint → T-V2-Retirement`
- r3-program-plan.md:360-363: "T-LensProducer-Retirement comes BEFORE
  T-FixedPoint, not after; T-FixedPoint depends on SG-0 zero from
  T-LensProducer"

T-LensProducer-Retirement coming AFTER T-FixedPoint creates a circular
dependency in the weekly snapshot. Corrected to use the canonical
R2-close-dependency from r3-structure.md §"Lane structure":
R2-Evaluator (interpreter-as-data; LANDED) + PB-1 generated bin-shim
pattern + R2-T-Ground-Lifetime-Analyzer a/b/c basic cases.

Also added warm-crab-600's gate #7 work-in-flight signal (regen_lens.rs
retirement; the 3rd sub-gate of T-LensProducer-Retirement) per latest
subtree status digest. All 3 sub-gates now in flight: #5 valiant-otter-
715, #6 same-cascade, #7 warm-crab-600.

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

* docs(r3): address codex BLOCKING #2/#3/#4 — single-authority reconciliation per §1.8

Three valid cell-level findings from codex schedule review on sha 1d95e61.
All caught the same root issue: §3 cells didn't reconcile against §1.8
ledger + r3-structure.md canonical authority before landing.

#2 — T-Numeric-Construction blocker (line 424):
   Cell said "Float migration + Real/base-carrier convention HELD on
   proud-raven-495 G2 Phase 2 Substrate S8 ApproximateField<F>" but
   §1.8 #18 + #24 explicitly say "CONSUMER_LANDED + PASSING for
   Grounding G2 primitive rows (2026-05-10, PR #2570 squash b96a51a)"
   — the work landed. Updated cell to: PR #2570 closes the prior HELD;
   remaining blocker is broader Real<N> emission demonstrations under
   S9/Shape-A follow-ons per §1.8 #18 close-criterion.

#3 — T-Bridge-Retirement count (line 427):
   Cell said 3 remaining sub-bridges including mark_bootstrap_secret_
   nominal_opacity, but §1.8 #32 PASSING + §2.3 explicitly says that
   bridge is closed. Corrected count: 3/5 sub-bridges retired (gate #32
   prior-cycle Secret nominal-opacity + gate #33 this cycle canonical
   lens + include_str this cycle), 2 remaining (SourceSpan.file
   participation + patch_lower_helpers residual).

#4 — T-Free-Consequences-Demonstration over-attribution (line 430):
   Cell credited gates #10/#33/#37/#40/#72 to T-Free, but §1.8 assigns
   those to other lanes:
   - #10 → T-V-L4-L7-Direct
   - #33 → T-Bridge-Retirement
   - #37 + #40 → T-CostLens-Composition
   - #72 → T-E-P-Producer-Broadening
   T-Free's canonical demo gate range is #43-#52. Only #43
   (auto_parallelism_independent_binds_emit_parallel) MERGED this cycle
   for T-Free. Updated cell + compile-note to credit each landing only
   to its canonical-lane row.

Compile-note also reconciled per the same §1.8 single-authority pass:
T-CostLens-Composition + T-E-P-Producer-Broadening now credited their
own gates instead of attributing them to T-Free.

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

* docs(r3): address codex BLOCKING #5/#6 — PR-merge evidence ≠ gate-PASSING

Two valid findings from codex schedule review on sha f6a3a13 (review
id 4259176210):

#5 — T-Bridge-Retirement count conflated PR-merge with gate-PASSING:
   Cell said "3/5 retired" but §1.8 truth: #32 PASSING, #33 DECLARED,
   #34 DECLARED, #35 PASSING. PR #2449 + PR #2459 ARE merged but the
   gates haven't been promoted from DECLARED → PASSING (separate status
   drift sweep step, e.g., per PR #2399 cadence). Reframed cell to
   distinguish PR-merge evidence from canonical §1.8 status: 2/5
   gate-PASSING (#32 + #35), 2/5 PR-merged-pending-promotion (#33 + #34),
   plus SourceSpan.file participation (Substrate-owned hand-Rust audit
   sites; not in numbered §1.8) + residual semantic patching
   (`bridge_exact_string_semantic_patching_residual` Open per #35
   close-criterion).

#6 — T-Free-Consequences over-claim on PR-merge:
   Cell said "gate #43 MERGED" but §1.8 #43 still DECLARED (PR #2495 is
   evidence toward promotion, not the promotion event). Same fix:
   reframe as PR-merge evidence accruing toward §1.8 gate promotion;
   canonical status authoritative.

Compile-note also reframed: explicitly distinguishes PR-merge evidence
from §1.8 gate-PASSING promotion. PR-merge events are listed as evidence
accruing toward promotion; canonical gate status varies per §1.8.

Common root: future Monday compiles must mechanically reconcile each
"landed/retired" claim against §1.8 status, NOT PR-merge events.
Discipline recorded in feedback_pm_compile_audits_pre_existing_errors
(updated to include PR-merge-vs-gate-promotion distinction).

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@briansrls
briansrls deleted the claude/review-todos-8zr6W branch June 1, 2026 18:41
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.

2 participants