rcsheets commented

Owner

Two visible changes plus a prompt tightening:

Whole-file fragments now read "(entire file)" instead of "(whole
file)". Less ambiguous about whether the bot saw the literal full
contents.
Query fragments now include the extractor's note alongside the
question — typically a path:start-end line range — so a reader can
tell which bytes the bot actually saw, not just what it asked
about. Falls back to the question-only format when the extractor
produced no note.

The extract sub-pass's tool-schema description and system prompt are
tightened to require the path:start-end format explicitly (e.g.
"foo.go:5-34"), with a comma-separated list when the excerpt spans
multiple non-contiguous regions. Previously the prompt just asked for
"line range covered, or why nothing was found" and the LLM was free
to write things like "the auth handler block" — informative but
not jump-to-line-able.

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

Two visible changes plus a prompt tightening: - Whole-file fragments now read "(entire file)" instead of "(whole file)". Less ambiguous about whether the bot saw the literal full contents. - Query fragments now include the extractor's note alongside the question — typically a path:start-end line range — so a reader can tell which bytes the bot actually saw, not just what it asked about. Falls back to the question-only format when the extractor produced no note. The extract sub-pass's tool-schema description and system prompt are tightened to require the path:start-end format explicitly (e.g. "foo.go:5-34"), with a comma-separated list when the excerpt spans multiple non-contiguous regions. Previously the prompt just asked for "line range covered, or why nothing was found" and the LLM was free to write things like "the auth handler block" — informative but not jump-to-line-able. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

rcsheets added 1 commit

2026-05-01 09:07:19 +00:00

fix: clarify whole-file vs. extracted fragments in the context summary

ci / check (pull_request) Waiting to run

Details

4476952cf3

Two visible changes plus a prompt tightening:

- Whole-file fragments now read "(entire file)" instead of "(whole
  file)". Less ambiguous about whether the bot saw the literal full
  contents.
- Query fragments now include the extractor's note alongside the
  question — typically a path:start-end line range — so a reader can
  tell which bytes the bot actually saw, not just what it asked
  about. Falls back to the question-only format when the extractor
  produced no note.

The extract sub-pass's tool-schema description and system prompt are
tightened to require the path:start-end format explicitly (e.g.
"foo.go:5-34"), with a comma-separated list when the excerpt spans
multiple non-contiguous regions. Previously the prompt just asked for
"line range covered, or why nothing was found" and the LLM was free
to write things like "the auth handler block" — informative but
not jump-to-line-able.

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

pr-reviewer-bot commented

2026-05-01 09:07:20 +00:00

Collaborator

Automated review by pr-reviewer v0.19.0 | Full Review | Claude Sonnet 4 | tracking id r-f46d48-5fc52c
This is an AI-generated review and may contain mistakes.

Status: ✅ Completed

✅ Verdict: Approve — Clean refactoring that improves clarity and enforces better structure in LLM prompts while maintaining backward compatibility.
Complexity: moderate

Code Review

This PR makes three coherent improvements to how the context summary displays repository fragments:

Changes Summary

Terminology clarity: "whole file" → "entire file" removes ambiguity about whether the bot saw complete file contents
Enhanced fragment annotations: Query fragments now show the extractor's line range note (e.g. "limits.go:5-12") alongside the question, making it easier for reviewers to understand exactly what the bot analyzed
Stricter LLM prompting: Both the system prompt and tool schema now explicitly require the path:start-end format for line ranges

Technical Assessment

Prompt Engineering (prompts.go)

The addition to ExtractFragmentPrompt is well-crafted:

Clear format specification with concrete examples
Handles multi-region excerpts appropriately
Maintains the existing fallback behavior for extraction failures

Schema Updates (tools.go)

The note field description in emitExcerptSchema() is much more precise than the previous "line range covered, or why nothing was found" - this should lead to more consistent LLM output formatting.

Display Logic (format.go)

The FormatContextSummary changes are solid:

Graceful fallback when frag.Note is empty (maintains backward compatibility)
Clear conditional logic that's easy to follow
The terminology change from "whole file" to "entire file" is more precise

Test Coverage (format_test.go)

Excellent test additions:

TestFormatContextSummaryQueryWithoutNote specifically validates the fallback behavior
Updated existing test to verify the new format with line ranges
Tests cover both the happy path and edge cases

Benefits

User Experience: Reviewers can now jump directly to the specific lines the bot analyzed
Debugging: When the bot makes questionable decisions, the line ranges help humans understand what context it had
Consistency: Stricter prompting should reduce variance in LLM output format

Potential Concerns

The changes look safe:

Backward compatibility is maintained through the conditional logic
No breaking changes to existing data structures
Test coverage validates both old and new behaviors

The only minor consideration is that this increases coupling between the extraction prompt, the display logic, and the specific path:start-end format - but this is reasonable given the benefits and the format is well-documented in multiple places.

Verdict

This is a thoughtful improvement that enhances both user experience and system reliability. The implementation is clean, well-tested, and maintains backward compatibility.

📚 Context used in this review

Project guidance files:

CLAUDE.md

*Automated review by [pr-reviewer](https://git.brooktrails.org/brooktrails/pr-reviewer) v0.19.0 | Full Review | Claude Sonnet 4 | tracking id `r-f46d48-5fc52c`* *This is an AI-generated review and may contain mistakes.* **Status:** ✅ Completed --- **✅ Verdict: Approve** — Clean refactoring that improves clarity and enforces better structure in LLM prompts while maintaining backward compatibility. **Complexity:** moderate # Code Review This PR makes three coherent improvements to how the context summary displays repository fragments: ## Changes Summary 1. **Terminology clarity**: "whole file" → "entire file" removes ambiguity about whether the bot saw complete file contents 2. **Enhanced fragment annotations**: Query fragments now show the extractor's line range note (e.g. "limits.go:5-12") alongside the question, making it easier for reviewers to understand exactly what the bot analyzed 3. **Stricter LLM prompting**: Both the system prompt and tool schema now explicitly require the `path:start-end` format for line ranges ## Technical Assessment ### Prompt Engineering (prompts.go) The addition to `ExtractFragmentPrompt` is well-crafted: - Clear format specification with concrete examples - Handles multi-region excerpts appropriately - Maintains the existing fallback behavior for extraction failures ### Schema Updates (tools.go) The `note` field description in `emitExcerptSchema()` is much more precise than the previous "line range covered, or why nothing was found" - this should lead to more consistent LLM output formatting. ### Display Logic (format.go) The `FormatContextSummary` changes are solid: - Graceful fallback when `frag.Note` is empty (maintains backward compatibility) - Clear conditional logic that's easy to follow - The terminology change from "whole file" to "entire file" is more precise ### Test Coverage (format_test.go) Excellent test additions: - `TestFormatContextSummaryQueryWithoutNote` specifically validates the fallback behavior - Updated existing test to verify the new format with line ranges - Tests cover both the happy path and edge cases ## Benefits 1. **User Experience**: Reviewers can now jump directly to the specific lines the bot analyzed 2. **Debugging**: When the bot makes questionable decisions, the line ranges help humans understand what context it had 3. **Consistency**: Stricter prompting should reduce variance in LLM output format ## Potential Concerns The changes look safe: - Backward compatibility is maintained through the conditional logic - No breaking changes to existing data structures - Test coverage validates both old and new behaviors The only minor consideration is that this increases coupling between the extraction prompt, the display logic, and the specific `path:start-end` format - but this is reasonable given the benefits and the format is well-documented in multiple places. ## Verdict This is a thoughtful improvement that enhances both user experience and system reliability. The implementation is clean, well-tested, and maintains backward compatibility. <details> <summary>📚 Context used in this review</summary> **Project guidance files:** - `CLAUDE.md` </details>

rcsheets force-pushed fix/context-summary-fragment-detail from 4476952cf3

ci / check (pull_request) Waiting to run

Details

to 0840345c1d

ci / check (pull_request) Successful in 43s

Details

2026-05-01 09:07:24 +00:00

Compare

rcsheets requested review from pr-reviewer-bot

2026-05-01 09:07:29 +00:00

rcsheets merged commit 1a3613fa4e into main

2026-05-01 09:17:19 +00:00