Skip to content

runner: keep readCapped's truncation on a rune boundary - #2307

Merged
Soph merged 1 commit into
mainfrom
soph/readcapped-rune-boundary
Sep 7, 2026
Merged

runner: keep readCapped's truncation on a rune boundary#2307
Soph merged 1 commit into
mainfrom
soph/readcapped-rune-boundary

Conversation

@Soph

@Soph Soph commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

https://entire.io/gh/entireio/cli/trails/1255

readCapped embeds repo docs (CLAUDE.md, AGENTS.md, README.md, go.mod) into the prompt entire runner tune sends to a model. Its cap is a byte budget applied as s[:maxLen], while both callers describe it in characters:

tuneDocCap    = 6000 // max chars embedded per doc (CLAUDE.md etc.)
tuneReadmeCap = 2000

A doc whose 6000th byte lands inside a multi-byte rune therefore put an invalid UTF-8 sequence into the prompt. Nothing downstream validates it, so it was invisible from the call site.

The fix

A continuation byte at the cut is exactly what "we cut mid-rune" means, so back off the continuation bytes — at most UTFMax-1 of them, the furthest a rune's start can be. utf8.RuneStart answers that from one byte.

Validating the prefix instead (utf8.ValidString(s[:cut])) answers a different question and is the wrong tool twice over: it walks the whole 6KB, and it returns false for a doc that is not UTF-8 at all, so chasing it back finds no valid prefix and drops the file's content in favour of a bare truncation marker. Invalidity our cut did not cause is the file's own, and the under-cap path already passes those bytes through.

Two traps, both pinned by tests

Getting here took two wrong turns, and the tests exist so neither comes back:

  • An unbounded backoff empties a non-UTF-8 file. A latin-1 README came back as nothing but …(truncated)… — worse than the mid-rune cut being fixed.
  • The floor has to be explicit, not implied by an iteration count. At maxLen=1 over a file of continuation bytes, a count-based bound indexes s[-1] and panics. Latent — the smallest cap any caller passes is go.mod's 400 — which is the reason to pin it rather than shrug.

Tests

These are the first tests readCapped has had: the boundary itself, a short file, a missing one, a non-UTF-8 doc keeping its content, and every cap from 0 to 7 across three body shapes.

The last one asserts the contract as a disjunction — the cut lands on a rune boundary, or it keeps the file's own bytes — rather than "not empty". An empty result is correct below one rune's width, where no non-empty valid prefix exists; an earlier draft of the test asserted non-empty and was wrong about the contract, not the code.

Why it's separate

Split out of #2290 (os.Root coverage follow-ups) on review feedback: a byte cap cutting a rune in prompt text has nothing to do with filesystem anchoring, and this is the piece of that branch that stands alone. #2290 no longer touches runner_gather.go.

Verification

mise run lint 0 issues, mise run test:ci exit 0, and GOOS=windows|linux|darwin go vet ./... all clean.

🤖 Generated with Claude Code


Note

Low Risk
Localized change to doc truncation for tuning prompts with thorough tests; no auth, network, or data-path impact.

Overview
readCapped (used when entire runner tune embeds CLAUDE.md, README.md, etc. into the model prompt) no longer uses a naive s[:maxLen] cut when content exceeds the byte cap. It walks back at most utf8.UTFMax-1 bytes to land on a rune start, and if no start exists in that window (e.g. latin-1 or continuation-byte-only content), it keeps the original byte cut so truncated output still includes file bytes instead of only the truncation marker.

Adds the first readCapped tests: valid UTF-8 rune-boundary truncation, short/missing files, non-UTF-8 content preservation, bounded backoff, and small caps (0–7) to prevent an index-out-of-range panic on tiny caps.

Reviewed by Cursor Bugbot for commit d096983. Configure here.

readCapped's cap is a byte budget, applied as `s[:maxLen]`, while both callers'
constants describe it in characters:

    tuneDocCap    = 6000 // max chars embedded per doc (CLAUDE.md etc.)
    tuneReadmeCap = 2000

So a CLAUDE.md whose 6000th byte falls inside a multi-byte rune put an invalid
UTF-8 sequence into the prompt `entire runner tune` sends to a model. Nothing
downstream validates, so it was invisible from the call site.

A continuation byte at the cut is exactly what "we cut mid-rune" means, so the
fix backs off the continuation bytes — at most UTFMax-1 of them, which is the
furthest a rune's start can be. `utf8.RuneStart` answers that from one byte,
where validating the prefix walks the whole 6KB and answers a different question:
"is this valid?" is false for a doc that is not UTF-8 at all, and chasing it back
drops the file's content in favour of a bare truncation marker. Invalidity our
cut did not cause is the file's own, and the under-cap path passes those bytes
through too.

Two traps, both pinned by tests rather than described:

- A latin-1 README came back as just the truncation marker when the backoff had
  no bound, which is worse than the mid-rune cut it was fixing.
- The floor has to be explicit rather than implied by an iteration count. At
  maxLen=1 over a file of continuation bytes, a count-based bound indexes s[-1]
  and panics — latent, since the smallest cap any caller passes is go.mod's 400.

First tests for readCapped: the boundary itself, a short file, a missing one, a
non-UTF-8 doc keeping its content, and every cap from 0 to 7 over three body
shapes asserting the contract as a disjunction — the cut lands on a rune
boundary, or it keeps the file's own bytes. An empty result is correct below one
rune's width, where no non-empty valid prefix exists.

Split out of the os.Root coverage follow-ups (#2290): a byte cap cutting a rune
in prompt text is unrelated to filesystem anchoring, and this is the piece of
that branch that stands alone.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 01M1XMH1AYKXWZHY2VHXNK3MR6
Copilot AI lite review requested due to automatic review settings September 7, 2026 09:52
@Soph
Soph requested a review from a team as a code owner September 7, 2026 09:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The change is localized, preserves the intended byte-cap behavior, and includes thorough tests for the key edge cases and regressions.

Pull request overview

This PR hardens readCapped (used by entire runner tune to embed repo docs into a model prompt) so truncation respects UTF-8 rune boundaries, preventing invalid UTF-8 from being introduced when truncating by a byte budget.

Changes:

  • Adjust readCapped truncation to back off up to utf8.UTFMax-1 continuation bytes to land on a rune boundary (when possible), otherwise preserve the original byte cut for non-UTF-8 content.
  • Add unit tests covering rune-boundary truncation, short and missing files, non-UTF-8 preservation, and tiny cap edge cases (0–7) to prevent index-underflow panics.
File summaries
File Description
cmd/entire/cli/runner_gather.go Updates readCapped to avoid mid-rune truncation while keeping non-UTF-8 bytes intact under truncation.
cmd/entire/cli/runner_gather_test.go Adds comprehensive tests to pin truncation semantics and regressions around small caps and non-UTF-8 inputs.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Soph added a commit that referenced this pull request Sep 7, 2026
The byte cap cutting a multi-byte rune in prompt text has nothing to do with
os.Root coverage, and it is the one piece of this branch that stands alone, so it
goes to #2307 rather than riding along here. Raised in review of the re-land.

runner_gather.go and runner_gather_test.go are now identical to main on this
branch, so the two PRs do not overlap and either can land first.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 01M1XMSQWVJ8XN0J5JPV6N7MX6
@Soph
Soph merged commit a835364 into main Sep 7, 2026
15 checks passed
@Soph
Soph deleted the soph/readcapped-rune-boundary branch September 7, 2026 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants