perf(#311): index subrepo routing by first path segment (#390)

cmdCommitToSubrepo routed each changed file to a sub-repo via
subRepos.find(...) inside the file loop — O(files * repos). Extract a pure
groupFilesBySubrepo() that buckets sub-repos by first path segment and scans
only the matching bucket, dropping it to expected O(files + repos). First-
match-in-array-order semantics (incl. multi-segment sub-repos) are preserved
exactly. Adds the first behavior-lock test for the routing path.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-05-27 20:21:29 -04:00
committed by GitHub
parent 09123e395e
commit 25d24219bf
3 changed files with 114 additions and 11 deletions

View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 311
---
Route changed files to sub-repos via a first-segment bucket index instead of an O(files*repos) linear scan (#311).

View File

@@ -366,6 +366,43 @@ function cmdCommit(cwd, message, files, raw, amend, noVerify) {
output(result, raw, hash || 'committed');
}
/**
* Route a list of changed files to their sub-repo prefixes.
*
* Bucket sub-repos by their first path segment. Any file that matches a
* sub-repo prefix must share that sub-repo's first segment, so we only scan
* the (small) bucket for the file's first segment instead of all sub-repos
* — O(F + R) expected vs the prior O(F*R) find-in-loop. Candidates stay in
* sub-repo array order, preserving the original first-match semantics
* (incl. multi-segment sub-repos like "vendor/pkg", which resolve via the
* inner startsWith). (#311)
*
* @param {string[]} files - changed file paths (relative to project root)
* @param {string[]} subRepos - sub-repo path prefixes from config.sub_repos
* @returns {{ grouped: Object<string,string[]>, unmatched: string[] }}
*/
function groupFilesBySubrepo(files, subRepos) {
const reposByFirstSeg = new Map();
for (const repo of subRepos) {
const firstSeg = String(repo).split('/')[0];
let bucket = reposByFirstSeg.get(firstSeg);
if (!bucket) { bucket = []; reposByFirstSeg.set(firstSeg, bucket); }
bucket.push(repo);
}
const grouped = {};
const unmatched = [];
for (const file of files) {
const candidates = reposByFirstSeg.get(file.split('/')[0]);
const match = candidates ? candidates.find(repo => file.startsWith(repo + '/')) : undefined;
if (match) {
(grouped[match] ||= []).push(file);
} else {
unmatched.push(file);
}
}
return { grouped, unmatched };
}
function cmdCommitToSubrepo(cwd, message, files, raw) {
if (!message) {
error('commit message required');
@@ -383,17 +420,7 @@ function cmdCommitToSubrepo(cwd, message, files, raw) {
}
// Group files by sub-repo prefix
const grouped = {};
const unmatched = [];
for (const file of files) {
const match = subRepos.find(repo => file.startsWith(repo + '/'));
if (match) {
if (!grouped[match]) grouped[match] = [];
grouped[match].push(file);
} else {
unmatched.push(file);
}
}
const { grouped, unmatched } = groupFilesBySubrepo(files, subRepos);
if (unmatched.length > 0) {
process.stderr.write(`Warning: ${unmatched.length} file(s) did not match any sub-repo prefix: ${unmatched.join(', ')}\n`);
@@ -1015,6 +1042,7 @@ function cmdCheckCommit(cwd, raw) {
}
module.exports = {
groupFilesBySubrepo,
determinePhaseStatus,
cmdGenerateSlug,
cmdCurrentTimestamp,

View File

@@ -1388,6 +1388,76 @@ describe('commit command', () => {
});
});
// ─────────────────────────────────────────────────────────────────────────────
// groupFilesBySubrepo tests (#311)
// ─────────────────────────────────────────────────────────────────────────────
describe('groupFilesBySubrepo (#311)', () => {
const { groupFilesBySubrepo } = require('../get-shit-done/bin/lib/commands.cjs');
test('single-segment subrepos route files correctly and unmatched collected', () => {
const result = groupFilesBySubrepo(
['packages/a.js', 'docs/x.md', 'README.md'],
['packages', 'docs']
);
assert.deepStrictEqual(result.grouped, { packages: ['packages/a.js'], docs: ['docs/x.md'] });
assert.deepStrictEqual(result.unmatched, ['README.md']);
});
test('multi-segment subrepo matches deep files, not shallow sibling', () => {
const result = groupFilesBySubrepo(
['vendor/pkg/x.js', 'vendor/other.js', 'vendor/pkg/y.js'],
['vendor/pkg']
);
assert.deepStrictEqual(result.grouped, { 'vendor/pkg': ['vendor/pkg/x.js', 'vendor/pkg/y.js'] });
assert.deepStrictEqual(result.unmatched, ['vendor/other.js']);
});
test('first-match-in-array-order wins (not longest-prefix)', () => {
// 'app' comes before 'app/sub' in subRepos array; 'app/sub/f.js' matches 'app' first
const result = groupFilesBySubrepo(
['app/sub/f.js'],
['app', 'app/sub']
);
assert.deepStrictEqual(result.grouped, { app: ['app/sub/f.js'] });
assert.deepStrictEqual(result.unmatched, []);
});
test('file with no slash does not match a same-name subrepo', () => {
const result = groupFilesBySubrepo(['top'], ['top']);
assert.deepStrictEqual(result.grouped, {});
assert.deepStrictEqual(result.unmatched, ['top']);
});
test('file with slash after prefix routes correctly', () => {
const result = groupFilesBySubrepo(['top/a'], ['top']);
assert.deepStrictEqual(result.grouped, { top: ['top/a'] });
assert.deepStrictEqual(result.unmatched, []);
});
test('empty files list returns empty grouped and unmatched', () => {
const result = groupFilesBySubrepo([], ['a']);
assert.deepStrictEqual(result.grouped, {});
assert.deepStrictEqual(result.unmatched, []);
});
test('empty subRepos list puts all files in unmatched', () => {
const result = groupFilesBySubrepo(['a/b'], []);
assert.deepStrictEqual(result.grouped, {});
assert.deepStrictEqual(result.unmatched, ['a/b']);
});
test('non-string subRepos entry does not throw and string entries still route (#311)', () => {
// Old inline code coerced non-string repos via `repo + '/'` and never threw.
let result;
assert.doesNotThrow(() => {
result = groupFilesBySubrepo(['a/b', 'README.md'], [null, 'a']);
});
assert.deepStrictEqual(result.grouped, { a: ['a/b'] });
assert.deepStrictEqual(result.unmatched, ['README.md']);
});
});
// ─────────────────────────────────────────────────────────────────────────────
// cmdWebsearch tests (CMD-05)
// ─────────────────────────────────────────────────────────────────────────────