From 25d24219bf1b1afec7f7ab46d77fbce895e4efa7 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 27 May 2026 20:21:29 -0400 Subject: [PATCH] perf(#311): index subrepo routing by first path segment (#390) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .changeset/311-subrepo-routing-index.md | 5 ++ get-shit-done/bin/lib/commands.cjs | 50 ++++++++++++++---- tests/commands.test.cjs | 70 +++++++++++++++++++++++++ 3 files changed, 114 insertions(+), 11 deletions(-) create mode 100644 .changeset/311-subrepo-routing-index.md diff --git a/.changeset/311-subrepo-routing-index.md b/.changeset/311-subrepo-routing-index.md new file mode 100644 index 000000000..0343a6f5e --- /dev/null +++ b/.changeset/311-subrepo-routing-index.md @@ -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). diff --git a/get-shit-done/bin/lib/commands.cjs b/get-shit-done/bin/lib/commands.cjs index 85e4850d5..855276de1 100644 --- a/get-shit-done/bin/lib/commands.cjs +++ b/get-shit-done/bin/lib/commands.cjs @@ -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, 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, diff --git a/tests/commands.test.cjs b/tests/commands.test.cjs index 8ba596a34..cd93be5d4 100644 --- a/tests/commands.test.cjs +++ b/tests/commands.test.cjs @@ -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) // ─────────────────────────────────────────────────────────────────────────────