Skip to content

fix(engine): keep renderRepoMap's output within maxOutputChars including the marker (#9617) - #9623

Closed
RealDiligent wants to merge 1 commit into
JSONbored:mainfrom
RealDiligent:fix/critical-issue-repo-map-truncation-marker-9617
Closed

fix(engine): keep renderRepoMap's output within maxOutputChars including the marker (#9617)#9623
RealDiligent wants to merge 1 commit into
JSONbored:mainfrom
RealDiligent:fix/critical-issue-repo-map-truncation-marker-9617

Conversation

@RealDiligent

Copy link
Copy Markdown
Contributor

Problem

Closes #9617.

renderRepoMap (packages/loopover-engine/src/miner/repo-map.ts) is documented as a bounded outline that stops once maxOutputChars would be exceeded and appends a truncation marker. But pushLine budgets only the content lines — the marker was appended via lines.push unbudgeted:

if (truncated) lines.push("… (repo map truncated to fit the output budget)");

So a truncated result overshot maxOutputChars by up to the marker's length + a newline, and when the very first line already exceeded the budget it returned only the marker — a string longer than the budget with no content. maxOutputChars is a token-budget guarantee the prompt packet relies on.

Fix

Reserve the marker from the budget (extracted to a TRUNCATION_MARKER const):

  • Not truncated → return the join unchanged (byte-for-byte identical, full budget, no reserved headroom).
  • Truncated → drop trailing content lines until the marker (plus its joining newline while content remains) fits, then append it, so result.length <= maxOutputChars always.
  • maxOutputChars < the marker's length → return the empty string rather than an over-budget marker.

Tests

test/unit/repo-map.test.ts: tightened the existing truncation test to the strict <= maxOutputChars bound; added a case asserting the bound holds across a spread of budgets (0, near/below/above the marker length, …) and that a sub-marker budget yields "" while a marker-sized budget yields the marker alone; updated the four break-site tests to the new contract (a below-marker budget truncates at each site to ""; the multi-symbol case keeps the fitting symbol alongside the reserved marker). 100% branch coverage on the changed file.

@RealDiligent
RealDiligent requested a review from JSONbored as a code owner July 29, 2026 01:01
@loopover-orb

loopover-orb Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Caution

🛑 LoopOver review result - fixes required

Review updated: 2026-07-29 01:23:53 UTC

2 files · 1 AI reviewer · no blockers · CI failing · unstable

🛑 Suggested Action - Fix Blockers

Review summary
This PR reserves TRUNCATION_MARKER's length from renderRepoMap's budget so a truncated result never exceeds maxOutputChars, replacing the previous behavior where the marker was pushed unbudgeted and could overshoot by its own length plus a newline. I traced the new logic (early return for untruncated output, the maxOutputChars < marker.length guard, and the pop-until-fits loop) against the new test cases and it is correct: every traced scenario (budget 0, sub-marker, exactly-marker, marker+content) produces a result that respects the <= maxOutputChars invariant. The change is narrow, closes issue #9617 as required, and the multi-symbol retention test correctly exercises the content-preserving branch of the pop loop.

Nits — 4 non-blocking
  • packages/loopover-engine/src/miner/repo-map.ts:341-345 — the new TRUNCATION_MARKER JSDoc block is inserted between the existing renderRepoMap doc comment and the function itself, so the original doc comment no longer directly precedes the function it documents (tooling that attaches JSDoc to the nearest following declaration will now attribute it to the const instead); move the marker's doc above the const and keep renderRepoMap's own doc comment immediately above `export function renderRepoMap`.
  • The PR description claims '100% branch coverage on the changed file' but codecov/patch reports only 22.22% of the diff hit against a 99% target — worth double-checking that the coverage tool is actually picking up the new test cases in test/unit/repo-map.test.ts before merging.
  • Verify locally that `pnpm test -- repo-map` plus the coverage reporter agrees with the PR's stated 100% branch coverage claim, since codecov/patch disagrees.
  • Consider moving the TRUNCATION_MARKER JSDoc above the const declaration so it doesn't visually detach renderRepoMap's own doc comment from the function (repo-map.ts:341-345).

CI checks failing

  • codecov/patch — 55.55% of diff hit (target 99.00%)

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #9617
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ❌ 5/25 Preflight is holding this PR: the review lane is unavailable, so it is not ready for automated review.
Contributor workload ✅ 10/10 Author activity: 266 registered-repo PR(s), 111 merged, 37 issue(s).
Contributor context ✅ Confirmed Gittensor contributor RealDiligent; Gittensor profile; 266 PR(s), 37 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Partially addressed
The core fix correctly reserves the marker's length (plus joining newline) from the budget, returns the unmodified string when nothing is truncated, and returns "" when maxOutputChars is smaller than the marker, all of which is exercised by new tests (empty-string case, multi-budget bound check, and updated break-site tests) — but the diff doesn't include the explicitly required new test asserting

Review context
  • Author: RealDiligent
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is registered but has no active allocation in the current snapshot.
  • Public profile languages: Python, Ruby, TypeScript, Svelte, Cuda, JavaScript, Markdown, MDX
  • Official Gittensor activity: 266 PR(s), 37 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Await review-lane availability.
  • Then work through the remaining 2 steps in the Signals table above.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 55.55556% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.01%. Comparing base (bd139a5) to head (7624ca6).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
packages/loopover-engine/src/miner/repo-map.ts 55.55% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9623      +/-   ##
==========================================
- Coverage   90.02%   90.01%   -0.01%     
==========================================
  Files         888      888              
  Lines      111983   112000      +17     
  Branches    26570    26573       +3     
==========================================
+ Hits       100810   100815       +5     
- Misses       9843     9851       +8     
- Partials     1330     1334       +4     
Flag Coverage Δ
backend 95.56% <100.00%> (-0.01%) ⬇️
engine 66.53% <22.22%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
packages/loopover-engine/src/miner/repo-map.ts 66.00% <55.55%> (-0.58%) ⬇️

... and 1 file with indirect coverage changes

…ing the marker

renderRepoMap's pushLine budgets every content line against maxOutputChars,
but the truncation marker was appended via lines.push unbudgeted — so a
truncated result overshot by up to the marker's length plus a newline, and in
the degenerate case returned a marker longer than the budget with no content.
maxOutputChars is a token-budget guarantee the prompt builder relies on.

Reserve the marker (plus its joining newline) from the budget: on truncation,
drop trailing content lines until the marker fits, and when the budget can't
hold even the marker alone, return the empty string rather than an over-budget
marker. A complete (non-truncated) render is byte-for-byte unchanged.
@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 29, 2026
@loopover-orb

loopover-orb Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

LoopOver is closing this pull request on the maintainer's behalf (CI is failing (codecov/patch)). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

@loopover-orb loopover-orb Bot closed this Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

engine(repo-map): renderRepoMap's truncation marker is appended outside its own char budget

1 participant