Skip to content

fix(engine): don't charge per-file-skipped files against repo-map's aggregate budget - #7270

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
dhgoal:fix/repo-map-aggregate-budget-skip
Jul 19, 2026
Merged

fix(engine): don't charge per-file-skipped files against repo-map's aggregate budget#7270
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
dhgoal:fix/repo-map-aggregate-budget-skip

Conversation

@dhgoal

@dhgoal dhgoal commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Closes #7247.

Problem

buildRepoMap enforces two byte budgets: a per-file cap (maxSourceBytes) and an aggregate cap across all parsed files (maxTotalSourceBytes). It added each file's byte count to totalSourceBytes before the check that skips a file for exceeding the per-file cap:

const sourceBytes = Buffer.byteLength(file.sourceText, "utf8");
totalSourceBytes += sourceBytes;               // charged unconditionally...
if (sourceBytes > maxSourceBytes || totalSourceBytes > maxTotalSourceBytes) {
  entries.push({ ..., skipped: "resource_limit" });   // ...even for a file skipped here, never parsed
  continue;
}

So a single oversized file (a vendored/minified asset or generated bundle that slips into the input list) that is itself skipped for exceeding maxSourceBytes still burned its full byte count against maxTotalSourceBytes — the aggregate budget meant to bound total parsed work. Every subsequent small, legitimate file then also tripped totalSourceBytes > maxTotalSourceBytes and was marked skipped: "resource_limit", producing a near-empty, order-dependent repo map with no error surfaced to the driver that consumes it.

Fix

Accrue to the aggregate after the per-file cap check, so only files that pass the per-file cap (and are actually parsed) count against the aggregate. Files under the per-file cap accrue and gate against the aggregate exactly as before — the only behavior change is that a per-file-oversized, unparsed file no longer poisons the aggregate for the files after it.

Tests

Added a regression test in repo-map.test.ts (imports the engine via its source barrel, so the changed lines are instrumented): an oversized first file (28 bytes, over a maxSourceBytes: 20 / maxTotalSourceBytes: 20 config) is skipped, and the small legitimate file after it is still parsed (symbols extracted) rather than starved of aggregate budget. The existing per-file-cap and aggregate-cap tests continue to cover both skip branches.

Verification

  • tsc --noEmit (root typecheck) — clean.
  • npm run build --workspace @loopover/engine — clean.
  • repo-map.test.ts — 35 tests green.
  • Changed file repo-map.ts: 113/113 lines, 12/12 functions, 84/84 branches covered (v8/lcov).

…ggregate budget

Closes JSONbored#7247

buildRepoMap added each file's byte count to totalSourceBytes before the check
that skips a file for exceeding the per-file maxSourceBytes cap. A single
oversized file (a vendored/minified asset or generated bundle) that was itself
skipped without being parsed still consumed its full byte count against
maxTotalSourceBytes, exhausting the aggregate budget so every subsequent small,
legitimate file was also marked skipped: resource_limit — a silent,
order-dependent near-empty repo map.

Move the aggregate accrual to after the per-file cap check so only files that
actually pass the per-file cap (and are parsed) count against the aggregate.
In-cap files accrue exactly as before.
@dhgoal
dhgoal requested a review from JSONbored as a code owner July 19, 2026 11:33
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.81%. Comparing base (3c72fce) to head (676854c).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7270   +/-   ##
=======================================
  Coverage   93.81%   93.81%           
=======================================
  Files         704      704           
  Lines       69457    69459    +2     
  Branches    18899    18899           
=======================================
+ Hits        65161    65163    +2     
  Misses       3302     3302           
  Partials      994      994           
Flag Coverage Δ
shard-1 43.60% <0.00%> (-0.01%) ⬇️
shard-2 37.33% <100.00%> (+0.11%) ⬆️
shard-3 33.18% <0.00%> (+0.08%) ⬆️
shard-4 33.74% <0.00%> (-0.59%) ⬇️
shard-5 32.89% <0.00%> (+0.70%) ⬆️
shard-6 45.48% <0.00%> (-0.30%) ⬇️

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 100.00% <100.00%> (ø)

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 19, 2026
@loopover-orb

loopover-orb Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-19 11:46:38 UTC

2 files · 1 AI reviewer · no blockers · readiness 86/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This correctly fixes the ordering bug in buildRepoMap: the per-file cap check now runs before totalSourceBytes is incremented, so a single oversized/unparsed file no longer poisons the aggregate budget for subsequent legitimate files. The fix is minimal, in the right layer (source of the accrual, not a downstream symptom), and the regression test exercises the exact real path described in the issue (28-byte file over a 20-byte cap, followed by a small file that must still parse). The existing per-file-cap and aggregate-cap tests still pass and correctly continue to cover both skip branches independently.

Nits — 3 non-blocking
  • The inline comment at repo-map.ts:271-276 is fairly long for a single conditional; could be trimmed to a sentence since the PR description already explains the rationale in detail.
  • No test explicitly confirms totalSourceBytes stays at 0 (not partially incremented) when the oversized file is skipped — the existing test infers this indirectly via the second file parsing successfully.
  • Consider a brief unit assertion on the byte-accounting invariant directly (e.g., exposing or asserting via a follow-up small file whose size is exactly at the remaining budget) to make the aggregate-budget behavior even more explicit, though the current test already demonstrates the fix adequately.

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 #7247
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ❌ 8/20 High review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 103 registered-repo PR(s), 63 merged, 35 issue(s).
Contributor context ✅ Confirmed Gittensor contributor dhgoal; Gittensor profile; 103 PR(s), 35 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The diff moves totalSourceBytes accrual after the per-file maxSourceBytes check and skips oversized files before charging the aggregate, exactly matching the requested reordering, and adds the specified regression test proving a small file after an oversized one is no longer starved of aggregate budget.

Review context
  • Author: dhgoal
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: JavaScript
  • Official Gittensor activity: 103 PR(s), 35 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Add a concise scope and risk note.
  • Then work through the remaining 1 step 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

@loopover-orb loopover-orb Bot 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.

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit e66f869 into JSONbored:main Jul 19, 2026
16 checks passed
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.

repo-map's aggregate byte budget is poisoned by files it declines to parse

1 participant