Skip to content

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

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)#9628
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, but pushLine budgets only content lines — the truncation marker was appended via lines.push unbudgeted, so a truncated result overshot maxOutputChars by up to the marker's length + a newline, and when the first line alone exceeded the budget it returned only an over-budget marker. 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, 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 < marker length → return the empty string, never an over-budget marker.

Tests

test/unit/repo-map.test.ts: tightened the truncation test to the strict <= maxOutputChars bound; added a case asserting the bound across a spread of budgets 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. 100% line + branch coverage on the changed file (verified via lcov: 119/119 lines, 90/90 branches).

@RealDiligent
RealDiligent requested a review from JSONbored as a code owner July 29, 2026 01:32
@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:55:10 UTC

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

🛑 Suggested Action - Fix Blockers

Review summary
This is a scoped, correct fix for #9617: `TRUNCATION_MARKER` is extracted to a const and reserved from the budget via a pop-until-it-fits loop before being appended, so `renderRepoMap` now guarantees `result.length <= maxOutputChars` in all cases, including returning `""` when the budget can't even hold the marker. I traced the `length`/pop bookkeeping (repo-map.ts:405-411) against `pushLine`'s accounting and it's internally consistent — the `lines.length === 0 ? removed.length : removed.length + 1` subtraction correctly mirrors how each line's length was added, and the non-truncated path is returned unchanged with no reserved headroom as claimed. Tests were rewritten to the new `<=` contract and add real coverage of the three break sites plus the sub-marker-budget empty-string case.

Nits — 5 non-blocking
  • The PR description claims '100% line + branch coverage… 119/119 lines, 90/90 branches' but CI STATUS shows codecov/patch FAILED at 22.22% of diff hit (target 99%) — worth reconciling before merge since the two numbers can't both be right for the same diff.
  • repo-map.ts:344-346: the new marker-reservation doc comment duplicates most of the existing function-level doc above it; could be folded into one comment block instead of two adjacent ones.
  • The external brief flags `9617` as an unexplained magic number in the new comment (repo-map.ts:345) — it's just an issue-reference in prose, not a literal used in logic, so this is a non-issue, but a `// see engine(repo-map): renderRepoMap's truncation marker is appended outside its own char budget #9617` style citation is slightly cleaner if you want to placate coverage-of-comments tooling.
  • Confirm the codecov/patch failure isn't hiding an actual untested branch (e.g. the `lines.length === 0` ternary arm inside the pop loop at repo-map.ts:409) despite the description's claimed 100% coverage.
  • Consider a short inline comment on the pop-loop's length bookkeeping (repo-map.ts:406-411) since the off-by-one-newline accounting is subtle enough that a future edit could break it silently.

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 budgeting bug is genuinely fixed: the marker is now reserved via a post-hoc trim loop so truncated output stays within maxOutputChars, a too-small budget returns "", and the non-truncated path is left byte-for-byte unchanged, matching the issue's algorithmic requirements while preserving the pushLine/outer-loop structure. However, several deliverables are only loosely matched rather than

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: not available
  • 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.

…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.
@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.07%. Comparing base (8e3e4f5) to head (c11ab36).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
packages/loopover-engine/src/miner/repo-map.ts 55.55% 8 Missing ⚠️

❌ Your patch check has failed because the patch coverage (55.55%) is below the target coverage (99.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9628      +/-   ##
==========================================
- Coverage   90.08%   90.07%   -0.01%     
==========================================
  Files         889      889              
  Lines      112024   112041      +17     
  Branches    26587    26590       +3     
==========================================
+ Hits       100921   100926       +5     
- Misses       9773     9781       +8     
- Partials     1330     1334       +4     
Flag Coverage Δ
backend 95.56% <100.00%> (-0.01%) ⬇️
engine 66.85% <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

@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