Repository navigation
#444 — SourceSpan vocabulary + per-CTE byte-range retention - #452
Conversation
…tion Introduce THE position/span vocabulary (src/domain/span.rs: SourcePos + SourceSpan with the guarded half-open primitives contains / contains_byte / contains_range / overlaps + byte_range). Pure POD + serde derive only (gated by tests/domain_clean_arch.rs). Retire the dead cte::Span; attach the byte ranges the CTE engine computed and threw away onto the domain CteNode as a retained SourceSpan fact (the cute-dbt#40 pattern — the AST work stays in the adapter, writes POD facts back to the domain). slice_or_fallback now returns (String, Option<SourceSpan>); slice_terminal returns Some((String, SourceSpan)) with start.byte advanced PAST the same leading-trim prefix the text strips ([',', '\n', '\r', ' ', '\t'], Blocker-1) so compiled[span.byte_range()] byte-equals the trimmed terminal text, not the raw closing-paren-end offset. A non-terminal CteBody span is the untrimmed name AS ( … ) range and needs no adjustment. usize→u32 narrowing is guarded by a single checked u32::try_from + debug_assert! fallback (byte_u32), the ONE construction boundary every span byte field flows through. contains_range / overlaps never underflow on the end.byte==0 fallback span (empty-span guard, feedback refinement 4) — no end.byte-1 arithmetic anywhere. span.rs joins the strict crap4rs set (≤15; worst 14.0). Span arithmetic + slice functions verified by cargo-mutants (0 missed in new code). Exhaustive structured enumeration over the bounded byte cube (house style, no proptest). Goldens: NONE move (CteNode is not serialized into the report payload yet); report + explore goldens verified byte-identical via the headless zero-egress suite. Closes #444 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199AmBCec5kyVEEF1Qd7TSF
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
📝 WalkthroughWalkthroughAdds ChangesSourceSpan vocabulary and CTE span retention
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📄 Rendered report previewAll golden examples regenerated cleanly. 🟡 Golden examplesCommitted to
🐶 Live dogfood previewThis PR doesn't touch 🧭 Explore previewThe two-page 🟡 Golden exploreThe committed
🐶 Live exploreThis PR doesn't touch ▶ Open ↗ opens the report or explorer in your browser in one The Pages preview may take ~1 min to update after this comment Alternative: GitHub CLI# gh CLI >= 2.63 extracts into ./report-preview-playground/.
gh run download 27930442031 -R breezy-bays-labs/cute-dbt -n report-preview-playground
open report-preview-playground/playground-report.htmlPosted by |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new SourceSpan and SourcePos vocabulary in src/domain/span.rs to replace the old Span struct, enabling byte-faithful tracking of CTE bodies and terminal nodes. It updates the CTE engine to retain these source spans and includes comprehensive tests. The review feedback correctly identifies a bug in the overlaps method where empty spans can incorrectly report an overlap with non-empty spans, violating the documented behavior.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…mutant, unify line/col narrowing, merge main FIX 1 (never-a-false-claim): SourceSpan::overlaps now matches its doc — an empty `[k,k)` span (zero bytes) overlaps nothing, even when it sits arithmetically inside another span (e.g. (5,5) inside (0,10)). The exhaustive [0,N]^4 oracle was vacuous for this case (its reference def omitted the empty-span exclusion); corrected to `s_nonempty && o_nonempty && s0 < o1 && o0 < s1` plus explicit empty-span assertions in both argument positions (empty-inside / at-boundary / outside). FIX 2 (kill surviving mutant): pin a pos_at-derived col to its exact 1-based value in terminal_span_start_is_post_trim (end.col == 18, line 5) so the `+1 → *1` off-by-one on ByteIndex::pos_at's col is caught. `cargo mutants -f src/adapters/cte_engine.rs --re pos_at` → 0 MISSED in the pos_at function. FIX 3: cross-check the loc_to_pos start.col against the byte-derived char column in span_line_col_agree_with_bytes (so both col sources are pinned under the §3.1 invariant), and route all three SourcePos axes (byte/line/col) through one debug-asserted `clamp_u32` helper (renamed from byte_u32) — no bare saturating unwrap_or(u32::MAX) left on line/col. FIX 4: refresh the stale mod.rs comment that justified not re-exporting project_def::Span via the now-retired cte::Span — the canonical SourceSpan now owns the position vocabulary at the domain root. Merged origin/main (S0 #443 lineage hoist) cleanly — mod.rs and crap4rs.toml auto-merged additively (both S0 lineage + S1 span entries preserved). Goldens unmoved (domain-only + adapter-internal; CteNode not serialized into the report payload). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199AmBCec5kyVEEF1Qd7TSF
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crap4rs.toml`:
- Around line 41-42: The [[overrides]] header on line 41 is duplicated, creating
an unintended empty override entry that can interfere with proper override
parsing. Remove the duplicate [[overrides]] header, keeping only one instance of
this section header in the configuration file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b30f0b8d-2b86-4682-91f2-3fca0d5b2696
📒 Files selected for processing (6)
crap4rs.tomlsrc/adapters/cte_engine.rssrc/domain/cte.rssrc/domain/mod.rssrc/domain/project_def.rssrc/domain/span.rs
|
✅ Merged S1 (#444) — the SourceSpan vocabulary + per-CTE byte-range retention. What landed: one position vocabulary ( Quality: behavior-preserving (zero golden movement; 2391 nextest + 242 BDD; domain-pure). Council review (cao/ceng/cpo clean, cqo concerns) → Sequencing note: S1 ships ahead of S2 despite design D3's "collapse S1+S2" — this does not reintroduce the dead-code-slice smell D3 feared, because Merged autonomously under the overnight build mandate. Next: S2 (#445) — the |
1 participant
Summary
S1 of the source-map spine epic (#442). Introduces THE position/span vocabulary and retains the per-CTE byte ranges the CTE engine currently computes and throws away.
src/domain/span.rs—SourcePos {line, col, byte: u32}+SourceSpan {start, end}with the guarded half-open primitives (contains/contains_byte/contains_range/overlaps/byte_range). Pure POD + serde derive only (gated bytests/domain_clean_arch.rs).cte::Span(was used only in its own tests); the domainCteNodenow carries the retainedSourceSpanfact viasource_span(). The AST work stays in the adapter and writes POD facts back to the domain (the cute-dbt#40 pattern).slice_or_fallbackreturns(String, Option<SourceSpan>);slice_terminalreturnsSome((String, SourceSpan)). Attached to each node inbuild_nodes.start.byteis advanced PAST the same leading-trim prefix ([',', '\n', '\r', ' ', '\t']) the text strips, socompiled[span.byte_range()]byte-equals the trimmed terminal text — not the raw closing-paren-end offset. Non-terminal CteBody spans are the untrimmedname AS ( … )range, unadjusted.usize→u32via a singlebyte_u32(checkedu32::try_from+debug_assert!fallback) — the ONE construction boundary every span byte field flows through.contains_range/overlapsnever underflow on theend.byte==0fallback span; noend.byte - 1arithmetic anywhere.Gates run green
cargo fmt --all -- --checkcargo clippy --all-targets --locked -- -D warningscargo nextest run(2380 passed)cargo test --test bdd(242 scenarios passed)RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --lockedcargo deny checkheadless_zero_egress(12 passed,-- --ignored) +headless_toggle(123 passed,-- --ignored)span.rsjoins the strict ≤15 set (worst function 14.0); 0 functions above thresholdcargo-mutantstargeted on the span arithmetic + slice functions: 0 missed in new code (the 9 pre-existing survivors are untouched cte_engine.rs detectors, outside this slice's scope)tests/domain_clean_arch.rs) greenGoldens
Byte-identical (no movement). The domain
CteNodeis not serialized into the report payload yet, so no golden moves — verified via the headless zero-egress suite over the committed report AND explore examples.Closes #444
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Configuration