From 9d05ea2de766b150ef78c761e12e05fc68b06b7e Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Thu, 27 Aug 2026 14:43:09 +0000 Subject: [PATCH] fix(group)!: remove max_candidates_per_step and shared_libs (U6) Both keys were declared, defaulted, written into every generated group.yaml, and read by nothing -- the same three-station dead surface this PR removed for bm25_threshold, embedding_threshold and detect.embedding_fallback. Every other DetectConfig field gates a real extractor in sync.ts; shared_libs gates nothing, because 'lib' contracts come only from the operator-declared manifest extractor. MatchingConfig reaches matching.ts solely through buildNoisyContractFilter, which reads exclude_links_paths and exclude_links_param_only_paths and nothing else. Existing group.yaml files keep loading and keep their keys. parseGroupConfig spreads the raw block over its defaults, so a key the schema no longer knows about survives into the returned config -- which matters because `group add` and `group remove` round-trip the operator's file through loadGroupConfig -> yaml.dump -> write, so anything the parser dropped would be deleted from their checked-in file. The legacy-config test now pins both keys in the same cast form as its three siblings, and the fixture carries shared_libs so that assertion is not vacuous. Two stations that are easy to miss and are swept here: - gitnexus/bench/cross-repo-trace/verify.mjs GENERATES a fresh group.yaml. It is not a preserve-path fixture, so "leave YAML fixtures alone" does not cover it; the repo has two generators and both are updated. It is a .mjs file outside tsconfig's include, so no type gate would have caught it. - config-parser.test.ts asserted the removed default at runtime, which vitest DOES run. That assertion is gone from the defaults case (the key no longer has a default) and re-formed as a preserve assertion in the legacy case. Verification gate, corrected: "zero net new errors against origin/main" would have measured the whole branch delta and been red through no fault of this commit. Measured instead against the branch tip immediately before it -- tsc -p tsconfig.test.json --noEmit reports 989 before and 989 after. Twenty-four typed-literal sites across ten test files, none of them CI-gated, plus the two runtime sites above which are. Note the deliberate side effect: removing a key from the defaults also stops the group add round-trip from re-adding it to a file that never carried it. Nothing in src reads either key, so no behavior changes. BREAKING CHANGE: `matching.max_candidates_per_step` and `detect.shared_libs` are no longer part of the group.yaml schema and are no longer written into generated templates. Existing files carrying them continue to parse and retain them. src tsc clean; 1189/1189 across the group unit, group integration, locale-parity, help-registration and tool-schema suites. --- gitnexus/bench/cross-repo-trace/verify.mjs | 1 - gitnexus/src/core/group/config-parser.ts | 2 -- gitnexus/src/core/group/storage.ts | 2 -- gitnexus/src/core/group/types.ts | 2 -- .../group/group-sync-lock-concurrency.test.ts | 3 +-- .../test/unit/group/config-parser.test.ts | 8 +++++-- gitnexus/test/unit/group/matching.test.ts | 8 ------- .../group/registry-suppressed-stages.test.ts | 2 -- .../test/unit/group/sync-exact-only.test.ts | 3 +-- .../group/sync-partial-extraction.test.ts | 3 +-- .../unit/group/sync-unreadable-repos.test.ts | 3 +-- .../group/sync-windowed-resolution.test.ts | 3 +-- gitnexus/test/unit/group/sync.test.ts | 24 +++++++------------ gitnexus/test/unit/group/types.test.ts | 6 ++--- .../repo-manager-registry-strict-read.test.ts | 3 +-- 15 files changed, 22 insertions(+), 51 deletions(-) diff --git a/gitnexus/bench/cross-repo-trace/verify.mjs b/gitnexus/bench/cross-repo-trace/verify.mjs index a4d67f4b2..e965bfc07 100644 --- a/gitnexus/bench/cross-repo-trace/verify.mjs +++ b/gitnexus/bench/cross-repo-trace/verify.mjs @@ -58,7 +58,6 @@ packages: {} detect: http: true matching: - max_candidates_per_step: 3 `; } diff --git a/gitnexus/src/core/group/config-parser.ts b/gitnexus/src/core/group/config-parser.ts index e2b42c131..754f578f9 100644 --- a/gitnexus/src/core/group/config-parser.ts +++ b/gitnexus/src/core/group/config-parser.ts @@ -29,13 +29,11 @@ const DEFAULT_DETECT = { grpc: true, thrift: true, topics: true, - shared_libs: true, includes: false, workspace_deps: false, }; const DEFAULT_MATCHING = { - max_candidates_per_step: 3, exclude_links_paths: [] as string[], exclude_links_param_only_paths: false, }; diff --git a/gitnexus/src/core/group/storage.ts b/gitnexus/src/core/group/storage.ts index ea175216d..6e82fbb0a 100644 --- a/gitnexus/src/core/group/storage.ts +++ b/gitnexus/src/core/group/storage.ts @@ -98,10 +98,8 @@ detect: http: true grpc: true topics: true - shared_libs: true matching: - max_candidates_per_step: 3 # exclude_links_paths: [/ping, /health, /healthcheck] # exclude_links_param_only_paths: false `; diff --git a/gitnexus/src/core/group/types.ts b/gitnexus/src/core/group/types.ts index 578289444..39fcbe796 100644 --- a/gitnexus/src/core/group/types.ts +++ b/gitnexus/src/core/group/types.ts @@ -26,13 +26,11 @@ export interface DetectConfig { grpc: boolean; thrift: boolean; topics: boolean; - shared_libs: boolean; includes: boolean; workspace_deps: boolean; } export interface MatchingConfig { - max_candidates_per_step: number; /** * HTTP paths to exclude from cross-link matching. Contracts at these paths * are still extracted and visible in the registry, but they don't produce diff --git a/gitnexus/test/integration/group/group-sync-lock-concurrency.test.ts b/gitnexus/test/integration/group/group-sync-lock-concurrency.test.ts index 6a1064254..84a90c55d 100644 --- a/gitnexus/test/integration/group/group-sync-lock-concurrency.test.ts +++ b/gitnexus/test/integration/group/group-sync-lock-concurrency.test.ts @@ -51,11 +51,10 @@ const makeConfig = (name: string): GroupConfig => ({ grpc: false, thrift: false, topics: false, - shared_libs: false, includes: false, workspace_deps: false, }, - matching: { max_candidates_per_step: 3 }, + matching: {}, }); const parentContract: StoredContract = { diff --git a/gitnexus/test/unit/group/config-parser.test.ts b/gitnexus/test/unit/group/config-parser.test.ts index 93436a244..2ede066f4 100644 --- a/gitnexus/test/unit/group/config-parser.test.ts +++ b/gitnexus/test/unit/group/config-parser.test.ts @@ -59,7 +59,6 @@ repos: expect(config.links).toEqual([]); expect(config.packages).toEqual({}); expect(config.detect.http).toBe(true); - expect(config.matching.max_candidates_per_step).toBe(3); expect(config.matching.exclude_links_paths).toEqual([]); expect(config.matching.exclude_links_param_only_paths).toBe(false); }); @@ -77,6 +76,7 @@ repos: detect: http: true embedding_fallback: true + shared_libs: true matching: bm25_threshold: 0.7 embedding_threshold: 0.65 @@ -86,7 +86,6 @@ matching: expect(config.name).toBe('test'); expect(config.repos).toEqual({ app: 'my-app' }); expect(config.detect.http).toBe(true); - expect(config.matching.max_candidates_per_step).toBe(3); // Pinned behavior: PRESERVE, not strip. The parser spreads the raw block // over its defaults (`{ ...DEFAULT_MATCHING, ...raw.matching }`), so a key @@ -102,6 +101,11 @@ matching: expect((config.matching as unknown as Record).bm25_threshold).toBe(0.7); expect((config.matching as unknown as Record).embedding_threshold).toBe(0.65); expect((config.detect as unknown as Record).embedding_fallback).toBe(true); + // The two keys this commit removes, pinned the same way and for the same + // reason: an operator's group.yaml carries them today because + // `gitnexus group create` wrote them there. + expect((config.matching as unknown as Record).max_candidates_per_step).toBe(3); + expect((config.detect as unknown as Record).shared_libs).toBe(true); }); it('defaults thrift detection to true', () => { diff --git a/gitnexus/test/unit/group/matching.test.ts b/gitnexus/test/unit/group/matching.test.ts index 3b71fe32d..a10f1d017 100644 --- a/gitnexus/test/unit/group/matching.test.ts +++ b/gitnexus/test/unit/group/matching.test.ts @@ -636,7 +636,6 @@ describe('buildNoisyContractFilter (via runExactMatch)', () => { it('exclude_links_paths prevents cross-links for configured paths', () => { const matchingConfig: MatchingConfig = { - max_candidates_per_step: 3, exclude_links_paths: ['/ping'], exclude_links_param_only_paths: false, }; @@ -657,7 +656,6 @@ describe('buildNoisyContractFilter (via runExactMatch)', () => { it('excluded providers do not appear in matched', () => { const matchingConfig: MatchingConfig = { - max_candidates_per_step: 3, exclude_links_paths: ['/health'], exclude_links_param_only_paths: false, }; @@ -675,7 +673,6 @@ describe('buildNoisyContractFilter (via runExactMatch)', () => { it('excluded contracts do not appear in unmatched', () => { const matchingConfig: MatchingConfig = { - max_candidates_per_step: 3, exclude_links_paths: ['/ping'], exclude_links_param_only_paths: false, }; @@ -694,7 +691,6 @@ describe('buildNoisyContractFilter (via runExactMatch)', () => { it('exclude_links_param_only_paths filters /{param} and /{param}/{param}', () => { const matchingConfig: MatchingConfig = { - max_candidates_per_step: 3, exclude_links_paths: [], exclude_links_param_only_paths: true, }; @@ -715,7 +711,6 @@ describe('buildNoisyContractFilter (via runExactMatch)', () => { it('mixed routes like /users/{param} are NOT excluded by param_only', () => { const matchingConfig: MatchingConfig = { - max_candidates_per_step: 3, exclude_links_paths: [], exclude_links_param_only_paths: true, }; @@ -747,7 +742,6 @@ describe('buildNoisyContractFilter (via runExactMatch)', () => { it('trailing slash on contractId still matches configured exclusion', () => { const matchingConfig: MatchingConfig = { - max_candidates_per_step: 3, exclude_links_paths: ['/ping'], exclude_links_param_only_paths: false, }; @@ -766,7 +760,6 @@ describe('buildNoisyContractFilter (via runExactMatch)', () => { it('root path exclusion ["/"] suppresses http::GET::/ contracts', () => { const matchingConfig: MatchingConfig = { - max_candidates_per_step: 3, exclude_links_paths: ['/'], exclude_links_param_only_paths: false, }; @@ -788,7 +781,6 @@ describe('buildNoisyContractFilter (via runExactMatch)', () => { it('non-HTTP contracts are never filtered', () => { const matchingConfig: MatchingConfig = { - max_candidates_per_step: 3, exclude_links_paths: ['/ping'], exclude_links_param_only_paths: true, }; diff --git a/gitnexus/test/unit/group/registry-suppressed-stages.test.ts b/gitnexus/test/unit/group/registry-suppressed-stages.test.ts index 0bf1a2770..1fbaf51fa 100644 --- a/gitnexus/test/unit/group/registry-suppressed-stages.test.ts +++ b/gitnexus/test/unit/group/registry-suppressed-stages.test.ts @@ -40,12 +40,10 @@ const config: GroupConfig = { grpc: false, thrift: false, topics: false, - shared_libs: false, includes: false, workspace_deps: false, }, matching: { - max_candidates_per_step: 3, exclude_links_paths: [], exclude_links_param_only_paths: false, }, diff --git a/gitnexus/test/unit/group/sync-exact-only.test.ts b/gitnexus/test/unit/group/sync-exact-only.test.ts index 99085a2d4..541b83d86 100644 --- a/gitnexus/test/unit/group/sync-exact-only.test.ts +++ b/gitnexus/test/unit/group/sync-exact-only.test.ts @@ -28,11 +28,10 @@ describe('syncGroup exactOnly gates the wildcard stage', () => { grpc: false, thrift: false, topics: false, - shared_libs: false, includes: false, workspace_deps: false, }, - matching: { max_candidates_per_step: 3 }, + matching: {}, }; /** diff --git a/gitnexus/test/unit/group/sync-partial-extraction.test.ts b/gitnexus/test/unit/group/sync-partial-extraction.test.ts index 23d2b2a93..06b3bf450 100644 --- a/gitnexus/test/unit/group/sync-partial-extraction.test.ts +++ b/gitnexus/test/unit/group/sync-partial-extraction.test.ts @@ -92,11 +92,10 @@ const config = (): GroupConfig => ({ grpc: true, thrift: false, topics: false, - shared_libs: false, includes: false, workspace_deps: false, }, - matching: { max_candidates_per_step: 3 }, + matching: {}, }); describe('syncGroup when one extractor fails partway through a repo', () => { diff --git a/gitnexus/test/unit/group/sync-unreadable-repos.test.ts b/gitnexus/test/unit/group/sync-unreadable-repos.test.ts index 162cf20d4..0aab17922 100644 --- a/gitnexus/test/unit/group/sync-unreadable-repos.test.ts +++ b/gitnexus/test/unit/group/sync-unreadable-repos.test.ts @@ -169,11 +169,10 @@ const makeConfig = (repos: Record): GroupConfig => ({ grpc: false, thrift: false, topics: false, - shared_libs: false, includes: false, workspace_deps: false, }, - matching: { max_candidates_per_step: 3 }, + matching: {}, }); /** diff --git a/gitnexus/test/unit/group/sync-windowed-resolution.test.ts b/gitnexus/test/unit/group/sync-windowed-resolution.test.ts index 2cd3932a1..aaf361b87 100644 --- a/gitnexus/test/unit/group/sync-windowed-resolution.test.ts +++ b/gitnexus/test/unit/group/sync-windowed-resolution.test.ts @@ -213,11 +213,10 @@ describe('syncGroup windowed resolution bounds pool residency (real pool, #2189) grpc: false, thrift: false, topics: false, - shared_libs: false, includes: false, workspace_deps: false, }, - matching: { max_candidates_per_step: 3 }, + matching: {}, }; await syncGroup(config, { diff --git a/gitnexus/test/unit/group/sync.test.ts b/gitnexus/test/unit/group/sync.test.ts index 9ea3900ba..68fe13c50 100644 --- a/gitnexus/test/unit/group/sync.test.ts +++ b/gitnexus/test/unit/group/sync.test.ts @@ -26,11 +26,10 @@ describe('syncGroup', () => { grpc: false, thrift: false, topics: false, - shared_libs: false, includes: false, workspace_deps: false, }, - matching: { max_candidates_per_step: 3 }, + matching: {}, }); it('returns SyncResult with contracts and cross-links', async () => { @@ -222,11 +221,10 @@ describe('syncGroup', () => { grpc: false, thrift: false, topics: false, - shared_libs: false, includes: false, workspace_deps: false, }, - matching: { max_candidates_per_step: 3 }, + matching: {}, }; const result = await syncGroup(config, { @@ -676,11 +674,10 @@ service OrderService { grpc: false, thrift: false, topics: false, - shared_libs: false, includes: false, workspace_deps: false, }, - matching: { max_candidates_per_step: 3 }, + matching: {}, }; const cap = _captureLogger(); @@ -746,11 +743,10 @@ service OrderService { grpc: false, thrift: false, topics: false, - shared_libs: false, includes: false, workspace_deps: workspaceDeps, }, - matching: { max_candidates_per_step: 3 }, + matching: {}, }; } @@ -903,11 +899,10 @@ service OrderService { grpc: false, thrift: false, topics: false, - shared_libs: false, includes: false, workspace_deps: true, }, - matching: { max_candidates_per_step: 3 }, + matching: {}, }; const result = await syncGroup(config, { @@ -994,11 +989,10 @@ service OrderService { grpc: false, thrift: false, topics: false, - shared_libs: false, includes: false, workspace_deps: false, }, - matching: { max_candidates_per_step: 3 }, + matching: {}, }; const poolAdapter = await import('../../../src/core/lbug/pool-adapter.js'); @@ -1078,11 +1072,10 @@ service OrderService { grpc: false, thrift: false, topics: false, - shared_libs: false, includes: false, workspace_deps: false, }, - matching: { max_candidates_per_step: 3 }, + matching: {}, }; const result = await syncGroup(config, { @@ -1122,11 +1115,10 @@ describe('syncGroup windowed manifest resolution (issue #2189 / PR #2191 review) grpc: false, thrift: false, topics: false, - shared_libs: false, includes: false, workspace_deps: false, }, - matching: { max_candidates_per_step: 3 }, + matching: {}, }; }; diff --git a/gitnexus/test/unit/group/types.test.ts b/gitnexus/test/unit/group/types.test.ts index 463175afa..cfa9abba2 100644 --- a/gitnexus/test/unit/group/types.test.ts +++ b/gitnexus/test/unit/group/types.test.ts @@ -23,11 +23,10 @@ describe('Group types', () => { grpc: true, thrift: true, topics: true, - shared_libs: true, includes: true, workspace_deps: true, }, - matching: { max_candidates_per_step: 3 }, + matching: {}, }; expect(config.version).toBe(1); expect(config.name).toBe('company'); @@ -92,11 +91,10 @@ describe('Group types', () => { grpc: true, thrift: true, topics: true, - shared_libs: true, includes: true, workspace_deps: true, }, - matching: { max_candidates_per_step: 3 }, + matching: {}, }; expect(config.detect.thrift).toBe(true); }); diff --git a/gitnexus/test/unit/repo-manager-registry-strict-read.test.ts b/gitnexus/test/unit/repo-manager-registry-strict-read.test.ts index 814952b31..76d79e295 100644 --- a/gitnexus/test/unit/repo-manager-registry-strict-read.test.ts +++ b/gitnexus/test/unit/repo-manager-registry-strict-read.test.ts @@ -68,11 +68,10 @@ describe('readRegistryStrict', () => { grpc: false, thrift: false, topics: false, - shared_libs: false, includes: false, workspace_deps: false, }, - matching: { max_candidates_per_step: 3 }, + matching: {}, }); beforeEach(async () => {