feat(glob): pattern body header, parenthetical footer, structured total#45
Merged
Conversation
Replace the renderer-side `parse_truncation_footer` round-trip with a
structured `ToolMetadata::truncated_total` field that the producer
populates directly. The model still receives the `(Showing X of Y ...)`
prose footer in `content` (it needs the total to adapt strategy), but
the result-view parser no longer reverse-engineers the count from a
string the same module emits.
`glob_files` now returns a `GlobOutput { content, truncated_total }`;
the run-loop attaches the total via `ToolOutput::with_truncated_total`.
A lightweight `is_truncation_footer` gate replaces the count-parsing
helper so unknown trailing prose still falls through to text rather
than misparsing as a path. Resumed sessions whose JSONL predates the
field fall back to `files.len()` as the total — same forward-compat
shape used for `diff_chunks`.
Add `pattern: String` to `ToolResultView::GlobFiles` and emit a dim `pattern (visible of total)` row above the path list. Keeps the block self-describing once the status header (`Glob(**/*.rs)`) scrolls out of view — long chats currently strand the body without a way back to "what did this glob match again?". The header sits outside `MAX_TOOL_OUTPUT_LINES`: that budget gates content density (path rows), not metadata rows. Empty results suppress the header — the explicit `No files found` row already labels the result, and `(0 of 0)` would just be noise. The `(visible of total)` shape is uniform across truncation states; even `(5 of 5)` honestly tells the reader "nothing hidden", and asymmetric formatting costs more cognitive load than it saves redundancy.
Rework `glob`'s truncation footer to mirror the existing grep shape: lead with what the TUI elided (`+N files`), follow with a parenthetical disclosing the cap context (`(showing X of Y)`). Replaces the prior `... +N files of Y total` form, which mixed counted-noun and footnote grammar in one footer. The no-elision-but-truncated branch becomes `... showing N of N` mirroring grep's `... limit reached` — a single descriptor of the cap context, no `+N` to qualify.
Three tool-result renderers each open-coded the same `Line::from(vec![ Span::styled(STATUS_LINE_CONT, border) , Span::styled(text, style) ])` + `wrap_line` pair across body rows, headers, and footers. Five call sites in total — past the rule-of-three threshold for extraction. The new `bordered_row` module is a sibling to `numbered_row`, not a mode of it: numbered rows carry column-width state for line-number padding and a separator span, which an unnumbered mode would dilute. Always-wrap behavior is a small bug fix — long footers in narrow terminals previously overflowed the bar; they now wrap with the correct continuation prefix.
Add bordered_row.rs to the per-variant tool body tree, and refresh the glob.rs description to reflect the new pattern body header and parenthetical footer wording. Add `misparse` to the cspell wordlist for a doc comment in tool/glob.rs.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Codecov flagged three new code paths from this branch as uncovered: the `with_truncated_total` helper, the `if let Some(total)` attach in `run`, and the `extract_input_field(input, "pattern")?` guard in `parse_files_view`. Add `run_truncated_attaches_total_to_metadata` (end-to-end pin for the producer-side wiring) and `result_view_falls_back_when_input_has_no_pattern` (defensive arm for malformed input) to close the gap.
Footer arithmetic combined the unbounded universe with TUI-side hiding (`visible = total - hidden_in_tui`), producing a number that matched neither what the user saw nor a meaningful intermediate. Header already discloses both counts (`pattern (visible of total)`); footer just flags the cap with grep's `(limit reached)` token.
`parse_files_view` is a misnomer post the metadata refactor — it
no longer parses the unbounded total, only validates input and
classifies trailing prose. Rename to `build_files_view` to match.
Drop `.trim()` from `is_truncation_footer` (and its test): the
caller already passes a clean footer chunk from the
`content.trim_end()` + `rsplit_once("\n\n")` pipeline. Align the
truncation-test offset with its `glob_files` sibling so the magic
numbers do not drift.
Sessions recorded before `truncated_total` landed in metadata still carry the count in the `(Showing X of Y matches. ...)` prose footer that `glob_files` bakes into `content`. Without recovery, resumed truncated views silently under-report `total = files.len()` and the header reads `(N of N)` instead of `(N of Y)`.
Sweep over comments added/expanded in this PR. Drops restated-WHAT lines, narrative-of-change comments, speculative future-use notes, and exhaustive caller lists in module docs. Keeps load-bearing rationale (defensive arms, suppressed headers, intentional trade-offs) but trims to one line each.
hakula139
added a commit
that referenced
this pull request
Apr 28, 2026
PRs #44 / #45 / #46 omit the `crates/<crate>/src/` prefix on crate-source paths in the Changes table (writing `tool.rs`, not `crates/oxide-code/src/tool.rs`) and keep the full path for repo-root files. Pin that as a rule in both the PR template and CLAUDE.md so future PRs match without re-deriving the convention from prior art. Also add `pnpm lint` and `pnpm spellcheck` to the verification checklist. The original PR landed with a `cspell` failure on the new code because the node-check CI job wasn't in the local pre-commit loop.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Lands the four P2 follow-ups deferred from PR #44 as four atomic commits. The user-facing wins are the glob block staying self-describing once the status header scrolls away (a dim
pattern (visible of total)row above the path list) and a footer shape that mirrors grep's(limit reached)parenthetical. The two internal cleanups — metadata-driven total and a sharedbordered_rowhelper — drop a string round-trip and three open-coded row-and-wrap pairs.ToolMetadata::truncated_totalletsglob_filesship the unbounded match count alongsidecontentinstead of the renderer reverse-engineering it from the(Showing X of Y matches. ...)prose footer it just emitted. The model still sees that footer; it's the renderer that stops parsing it.parse_truncation_footeris replaced by a lightweightis_truncation_footershape gate so unknown trailing prose still falls through to text rather than misparsing as a path. Resumed sessions whose JSONL predates the field fall back tofiles.len()— same forward-compat shape used fordiff_chunks.ToolResultView::GlobFilesgains apattern: String. Renderer emitspattern (visible of total)above the path list when results are non-empty. Header sits outsideMAX_TOOL_OUTPUT_LINES— that budget gates content density, not metadata rows. Empty results suppress the header (theNo files foundrow already labels the result, and(0 of 0)would just be noise).... +N files (showing X of Y)for the combined case,... showing N of Nfor the no-elision-but-tool-truncated case,... +N filesunchanged for TUI-only elision. The prior... +N files of Y totalform mixed counted-noun and footnote grammar in one footer.bordered_rowmodule replaces five open-codedLine::from(vec![Span::styled(STATUS_LINE_CONT, ...), Span::styled(text, ...)])+wrap_linepairs acrosstext.rs,grep.rs, andglob.rs. Sibling tonumbered_row, not a mode of it — the column shape is different enough that one renderer for both modes would dilute each contract. Always-wrap behaviour is a small bug fix: long footers in narrow terminals previously skippedwrap_lineentirely and would have overflowed the bar.Changes
tool.rsToolMetadata::truncated_total: Option<usize>field,with_truncated_totalfluent helper, andpattern: StringonToolResultView::GlobFiles. Registry-routing test pins the full structured shape.tool/glob.rsglob_filesreturnsGlobOutput { content, truncated_total }; the run-loop attaches the total via metadata.parse_files_viewreads the total from metadata, drops theparse_truncation_footercount parser, and keeps a lightweightis_truncation_footershape gate. Tests cover the boundary, embedded-blank-line, inconsistent-metadata, and unrecognised-trailing-prose paths.tui/components/chat/blocks/tool.rspatternthrough toglob::render. Newmod bordered_row;declaration.tui/components/chat/blocks/tool/bordered_row.rs(new)[bar] [text]row primitive —render(out, ctx, border_style, text, text_style)always wraps. 3 unit tests pin span shape, wrap-and-bar continuation, and text-style preservation across wraps.tui/components/chat/blocks/tool/glob.rspatternparameter, dim header row above the file list, parenthetical footer wording. Refactored to callbordered_row::render. Tests cover the header shape, header suppression on empty, and the new footer wording.tui/components/chat/blocks/tool/grep.rsbordered_row::renderfor path headers and the combined footer. No behaviour change — wording stayed... +N lines (limit reached)since grep's truncation is by line count, not match count, so it can't yet disclose the equivalent ofshowing X of Y.tui/components/chat/blocks/tool/text.rsbordered_row::renderfor body rows and the+N linesfooter.tui/components/chat.rsGlobFilesshape (withpattern) and asserts both the header and the new footer wording.CLAUDE.mdbordered_row.rs;glob.rsdescription refreshed for the new header / footer..cspell/words.txtmisparse(used in a doc comment intool/glob.rs).Test plan
cargo fmt --all --checkcargo buildcompiles cleanlycargo clippy --all-targets -- -D warnings— zero warningscspell— cleancargo test— 1066 passed (was 1061 pre-PR; +5 net: dropped 3parse_truncation_footertests, added 3is_truncation_footer+ 3bordered_row+ 2 producer-side coverage tests)Out of scope
Cross-renderer footer convergence is partial: glob now uses
(showing X of Y), grep stays on(limit reached). Grep's truncation is by line count (itshead_limit), not match count, so disclosing the equivalent ofYwould require the tool to track an unbounded total it doesn't currently compute. The metadata slot (truncated_total) is now in place for when grep can populate it — converging then becomes a one-renderer change.