Replace TODOs with config-driven defaults and transport-based checks by briansrls · Pull Request #22 · gunb-ai/gunbc

briansrls · 2026-02-03T19:00:24Z

Summary

This PR resolves several TODO items by implementing proper configuration-driven defaults and clarifying the architecture for transport-based operations. The changes move away from hardcoded values and hidden I/O operations toward explicit, graph-based execution patterns.

Key Changes

Review pipeline config: Added default_branch field to ReviewPipelineConfig, sourced from GitConfig::default_branch. The graph builder now uses this config value instead of hardcoding "main", making the branching model configurable per repository.
Rust toolchain installation: Added apt as a valid installation option for Rust (via the rustc package). Updated the corresponding test to verify that Rust is now satisfiable on Ubuntu with only apt available. Added clarifying comments about why rustup is preferred but rustc is an acceptable fallback.
Codegen staleness checking: Replaced the stub needs_codegen() implementation with a conservative check that returns true if the registry contains any tools. Clarified in comments that staleness checking at runtime is handled by transport-based file existence checks in the CI graph (e.g., CIOp::PrepareCodegenExistsCheck).
Binary crate detection: Renamed file_exists() to is_binary_crate() and updated its documentation to explain that it currently assumes all members are library crates. Added guidance on how to extend this with transport-based detection via FileRequest::exists() in an upstream graph node, rather than hidden filesystem I/O.
Documentation improvements: Replaced vague TODOs with clear explanations of the current design, source-of-truth relationships, and how to extend functionality properly (e.g., via config files or transport nodes).

Implementation Details

The default_branch field in ReviewPipelineConfig uses a #[serde(default)] attribute with a helper function to maintain backward compatibility.
Comments now explicitly distinguish between CLI defaults (fallbacks) and config-driven values (source of truth).
The changes reinforce the principle that I/O operations should be explicit graph nodes, not hidden in utility functions.

https://claude.ai/code/session_01M9zdaGvWmhKAqTKpbGyvWb

- Wire GitConfig::default_branch into ReviewPipelineConfig so the review graph builder uses config instead of hardcoding "main" - Add serde default for backward-compatible deserialization of default_branch field - Replace needs_codegen() stub (always true) with meaningful check (!self.tools.is_empty()) - Replace file_exists stub in buck2 ops with is_binary_crate, removing dead binary-detection code paths and documenting transport-based path - Add apt install option (rustc) for Rust tooldef alongside existing brew (rustup), updating corresponding satisfiability test - Update codegen registry comment to document CLI default / pipeline config relationship https://claude.ai/code/session_01M9zdaGvWmhKAqTKpbGyvWb

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 85de7a12ba

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Each tool in the Rust dependency chain (RUST → CARGO → CLIPPY/RUSTFMT) now declares its own install options per package manager instead of relying on the is_base_pm shortcut (empty install_options). On brew, all levels resolve to the `rustup` package (idempotent). On apt, each maps to its own system package: rustc, cargo, rust-clippy, rustfmt. The depends_on chain composes satisfiability checks through the dependency graph. This replaces the previous model where CARGO/CLIPPY/RUSTFMT had empty install_options and were treated as unconditionally satisfiable. https://claude.ai/code/session_01M9zdaGvWmhKAqTKpbGyvWb

Delete 7 tests that just re-read static ToolDef/PlatformDef fields and assert them back (e.g. CARGO.depends_on contains "rust", APT.id == "apt"). These add no regression protection beyond what the compiler enforces. Keep 18 tests that exercise actual composition logic: satisfiability graph walks, dependency chain resolution, error paths, install planning, and platform registry inheritance. https://claude.ai/code/session_01M9zdaGvWmhKAqTKpbGyvWb

Remove stale reference to the retired i64-carrier-only test name; point at uint64_upper_half_literal_tokenizes_and_narrows for R3 gate #22. Co-authored-by: Cursor <cursoragent@cursor.com>

* WIP: R3 gate #22: int lit full magnitude consumer * WIP: R3 gate #22: int lit full magnitude consumer * WIP: R3 gate #22: int lit full magnitude consumer * WIP: R3 gate #22: int lit full magnitude consumer * WIP: R3 gate #22: int lit full magnitude consumer * WIP: R3 gate #22: int lit full magnitude consumer * refactor(v3-emit): rename Int literal pattern binding to decimal render_value (emit / rust / python) and lens_testgen: LiteralBits::Int payload is a signed decimal string; use `decimal` instead of `n` and clone the string. Co-authored-by: Cursor <cursoragent@cursor.com> * test(integration): refresh parse corpus manifest after Int surface String SG-2 handwritten parse snapshot hashes drift when SurfaceLiteral::Int Debug output changes; regenerate via refresh_handwritten_parse_snapshot_manifest. Co-authored-by: Cursor <cursoragent@cursor.com> * chore: regen bootstrap snapshots after rebase onto main Re-run regen_bootstrap so committed fixtures match merged .dag + compiler emit after resolving rebase conflicts. Co-authored-by: Cursor <cursoragent@cursor.com> * docs(test): align gate #21 doc with UInt64 full-magnitude path Remove stale reference to the retired i64-carrier-only test name; point at uint64_upper_half_literal_tokenizes_and_narrows for R3 gate #22. Co-authored-by: Cursor <cursoragent@cursor.com> * WIP: R3 gate #22: int lit full magnitude consumer * fix(v3): preserve full-magnitude range bounds in where synthesis range(min/max) synthesis no longer requires i64::from_str on bounds, which sent valid ultrawide Int literals down the placeholder path (fail-closed gap vs R3 decimal-string carrier). Validate with BigInt::from_str and thread the authored decimal text into SurfaceLiteral::Int. Also fix evaluator unit test helpers to use literal_bits_int after LiteralBits::Int became String-backed. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>

…tub (#2637)

Add integration tests for i128::MAX / i128::MIN decimal literals narrowing to Int128 with preserved LiteralBits::Int strings (gate #22 consumer beside UInt64/UInt128 cases). Co-authored-by: Cursor <cursoragent@cursor.com>

) Add integration tests for i128::MAX / i128::MIN decimal literals narrowing to Int128 with preserved LiteralBits::Int strings (gate #22 consumer beside UInt64/UInt128 cases). Co-authored-by: Cursor <cursoragent@cursor.com>

…tion)

…#3080) * WIP: R3 gate #22: int_lit_full_magnitude_consumer (T-Numeric-Construction) * docs(r3): name gate 22 predicate harness * docs(r3): tie gate 22 receipt to PR * docs(r3): refresh gate 22 audit counts * docs(r3): reconcile close audit counts with main

chatgpt-codex-connector Bot reviewed Feb 3, 2026

View reviewed changes

Comment thread core/ir/src/transport/tool.rs Outdated

claude added 2 commits February 3, 2026 19:18

briansrls merged commit 884d6ae into main Feb 3, 2026
1 check passed

briansrls added a commit that referenced this pull request May 9, 2026

WIP: R3 gate #22: int lit full magnitude consumer

3570d4a

briansrls added a commit that referenced this pull request May 9, 2026

WIP: R3 gate #22: int lit full magnitude consumer

34c09c5

briansrls added a commit that referenced this pull request May 9, 2026

WIP: R3 gate #22: int lit full magnitude consumer

e726fef

briansrls added a commit that referenced this pull request May 9, 2026

WIP: R3 gate #22: int lit full magnitude consumer

643dbe8

briansrls added a commit that referenced this pull request May 9, 2026

WIP: R3 gate #22: int lit full magnitude consumer

a5dc7dd

briansrls added a commit that referenced this pull request May 9, 2026

WIP: R3 gate #22: int lit full magnitude consumer

439f1db

briansrls added a commit that referenced this pull request May 9, 2026

WIP: R3 gate #22: int lit full magnitude consumer

f97384c

briansrls added a commit that referenced this pull request May 9, 2026

WIP: R3 gate #22: int lit full magnitude consumer

e04080c

briansrls added a commit that referenced this pull request May 9, 2026

WIP: R3 gate #22: int lit full magnitude consumer

9ca8611

briansrls added a commit that referenced this pull request May 9, 2026

WIP: R3 gate #22: int lit full magnitude consumer

6a54498

briansrls added a commit that referenced this pull request May 9, 2026

WIP: R3 gate #22: int lit full magnitude consumer

bb4a602

briansrls added a commit that referenced this pull request May 9, 2026

WIP: R3 gate #22: int lit full magnitude consumer

4168a1a

briansrls added a commit that referenced this pull request May 9, 2026

WIP: R3 gate #22: int lit full magnitude consumer

b61daff

briansrls added a commit that referenced this pull request May 9, 2026

WIP: R3 gate #22: int lit full magnitude consumer

dcd4fbf

briansrls added a commit that referenced this pull request May 9, 2026

WIP: R3 gate #22: int lit full magnitude consumer

0998f54

briansrls added a commit that referenced this pull request May 9, 2026

WIP: R3 gate #22: int lit full magnitude consumer

c67a041

briansrls added a commit that referenced this pull request May 9, 2026

WIP: R3 gate #22: int lit full magnitude consumer

9b12c99

briansrls mentioned this pull request May 9, 2026

R3 gate #22: int lit full magnitude consumer #2428

Merged

6 tasks

briansrls added a commit that referenced this pull request May 9, 2026

WIP: R3 gate #22: int lit full magnitude consumer

e53cb21

briansrls added a commit that referenced this pull request May 10, 2026

WIP: R3 T-Numeric residual cluster — gates #19/#21/#22/#23 dispatch s…

286c744

…tub (#2637)

briansrls mentioned this pull request May 11, 2026

R3 gate #22: int lit full magnitude consumer #2675

Merged

6 tasks

briansrls mentioned this pull request May 14, 2026

R3 gate #22: int_lit_full_magnitude_consumer (T-Numeric-Construction) #3080

Merged

briansrls added a commit that referenced this pull request May 14, 2026

WIP: R3 gate #22: int_lit_full_magnitude_consumer (T-Numeric-Construc…

2394dc6

…tion)

briansrls deleted the claude/review-todos-Lpjxl branch June 1, 2026 18:41

Navigation Menu

Repository navigation

Replace TODOs with config-driven defaults and transport-based checks - #22

Replace TODOs with config-driven defaults and transport-based checks#22
briansrls merged 3 commits into
mainfrom
claude/review-todos-Lpjxl

briansrls commented Feb 3, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

briansrls commented Feb 3, 2026

Summary

Key Changes

Implementation Details

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants