fix: decouple ffgrep per-file cap from page size (#825)

[gustav-fff]

Closes #825

Root cause

packages/pi-fff/src/index.ts passed maxMatchesPerFile: pageSize for grep (:908, :941) and multi-grep (:1191). The engine's grep cursor is a file offset (crates/fff-core/src/grep/types.rs:87-89, next_file_offset), and maxMatchesPerFile caps per-file matches (types.rs:84). Clamping the per-file cap down to the page size means once a file's first pageSize matches are collected, the next cursor advances past that file, so its remaining matches are unreachable on every later page. page_limit is only a soft cap that finishes the current file (types.rs:209-215), so the two limits must stay decoupled.

Fix

Add GREP_MAX_MATCHES_PER_FILE = 200 (the engine default) and pass it as maxMatchesPerFile at all three grep call sites. pageSize still bounds total matches per page; the per-file cap no longer discards same-file overflow.

Steps to reproduce

Build the native lib and prepare the bun package on pre-fix main:

make prepare-bun

Save as packages/fff-bun/repro825.mjs (exercises the exact options pi-fff passes to the engine):

import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "node:fs";
import { join } from "node:path";
import { tmpdir } from "node:os";
import { FileFinder } from "./src/index.ts";

const root = mkdtempSync(join(tmpdir(), "fff825-"));
mkdirSync(join(root, "src"), { recursive: true });
writeFileSync(join(root, "noise.ts"),
  Array.from({ length: 30 }, (_, i) => `TODO fix ${String(i + 1).padStart(2, "0")}`).join("\n") + "\n");
writeFileSync(join(root, "src/app.ts"), "TODO app\n");
writeFileSync(join(root, "src/utils.ts"), "TODO utils\n");
writeFileSync(join(root, "README.md"), "TODO readme\n");

const finder = FileFinder.create({ basePath: root });
await finder.value.waitForIndexReady(5000);

function walk(perFile, page) {
  const got = []; let cursor = null; const pages = [];
  for (let i = 0; i < 10; i++) {
    const r = finder.value.grep("TODO", { mode: "plain", smartCase: true, maxMatchesPerFile: perFile, pageSize: page, cursor });
    got.push(...r.value.items.map((m) => m.lineContent.trim()));
    pages.push(r.value.items.length);
    cursor = r.value.nextCursor;
    if (!cursor) break;
  }
  return { got, pages };
}
const expected = [
  ...Array.from({ length: 30 }, (_, i) => `TODO fix ${String(i + 1).padStart(2, "0")}`),
  "TODO app", "TODO utils", "TODO readme",
];
const miss = (r) => expected.filter((e) => !r.got.includes(e));
const buggy = walk(20, 20);   // current pi-fff: maxMatchesPerFile === pageSize
const fixed = walk(200, 20);  // decoupled
console.log(JSON.stringify({
  current_pi_fff: { pages: buggy.pages, retrieved: buggy.got.length, missing: miss(buggy) },
  decoupled_cap:  { pages: fixed.pages, retrieved: fixed.got.length, missing: miss(fixed) },
}, null, 2));
rmSync(root, { recursive: true, force: true });

Run: cd packages/fff-bun && bun repro825.mjs

Expected: all 33 matches retrieved by walking cursors to exhaustion.
Actual (pre-fix, maxMatchesPerFile === pageSize === 20):

{
  "current_pi_fff": { "pages": [21, 2], "retrieved": 23,
    "missing": ["TODO fix 21","TODO fix 22","TODO fix 23","TODO fix 24","TODO fix 25","TODO fix 26","TODO fix 27","TODO fix 28","TODO fix 29","TODO fix 30"] },
  "decoupled_cap":  { "pages": [31, 2], "retrieved": 33, "missing": [] }
}

How verified

• SDK repro above: decoupled cap retrieves all 33; clamped cap drops matches 21-30.
• Added regression test ffgrep per-file cap (#825) asserting the tool passes maxMatchesPerFile: 200 decoupled from pageSize. cd packages/pi-fff && bun test test/ → 80 pass, 0 fail.
• oxfmt --check and oxlint clean on both changed files.

Note: files with more than GREP_MAX_MATCHES_PER_FILE (200) matches still truncate — that is the deeper file-offset limitation tracked in #365, out of scope here.

Automated triage via Gustav. Honk-Honk 🪿

Summary by CodeRabbit

• Bug Fixes
  • Improved grep pagination so searches can continue retrieving additional matches from the same file.
  • Per-file result limits are now independent of the page size, preventing relevant matches from being truncated prematurely.
• Tests
  • Added coverage to verify the updated grep result-limit behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: fb13a539-e9b6-4f18-8591-13ffc9e7a4b8

📥 Commits

Reviewing files that changed from the base of the PR and between 973c859 and cd42bcf.

📒 Files selected for processing (2)

• packages/pi-fff/src/index.ts
• packages/pi-fff/test/extension.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

The grep tools now keep maxMatchesPerFile at 200 instead of tying it to pageSize. Tests verify the separate limits through a configurable grep mock.

Changes

Grep pagination

Layer / File(s)	Summary
Separate grep match limits packages/pi-fff/src/index.ts	Primary grep, fuzzy fallback, and multiGrep use GREP_MAX_MATCHES_PER_FILE = 200. Comments describe the separate page-size behavior.
Test the per-file cap packages/pi-fff/test/extension.test.ts	The mock finder supports grep overrides. The test verifies pageSize: 20 and maxMatchesPerFile: 200.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to cd42b

The change allows later matches in the same file to remain reachable across pages while retaining bounded result processing. No actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: dmtrkovalenko, xwilludelu

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The implementation addresses the core pagination bug in #825 by setting maxMatchesPerFile to 200 while retaining pageSize for pagination. However, the added test only checks the passed options. It doe…	Add a regression test with more than 20 matches in one file. Consume cursors until exhaustion and assert that every indexed match is returned.
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the primary fix: decoupling the ffgrep per-file cap from the page size.
Out of Scope Changes check	✅ Passed	The production and test changes are directly related to #825. No unrelated code changes are shown.

Full details: Linked Issues check

Explanation

The implementation addresses the core pagination bug in #825 by setting maxMatchesPerFile to 200 while retaining pageSize for pagination. However, the added test only checks the passed options. It does not create same-file overflow or consume cursors to verify that all matches are retrieved.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch triage-bot/issue-825

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}