mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-10 03:27:59 +00:00
fix(group): record the matching stages a sync was told to skip (U2)
An `--exact-only` sync wrote a contracts.json and bridge with fewer cross-links
and nothing recorded that the wildcard stage had been suppressed by request.
`group_impact` and cross-repo `trace` read that registry as authoritative, so a
narrowed graph was indistinguishable from a complete one -- and because
`group_sync` is MCP-exposed, one agent call durably narrowed the shared answer
for every later reader with no signal at all.
Add `suppressedMatchStages` to ContractRegistry and SyncResult, following the
`unreadableRepos` tri-state end to end: absent means a registry written before
the field existed, `[]` is the measurement "this run suppressed nothing", and a
populated list names the stages. The writer always emits it, because omitting
the empty case is what made "measured, none" unreachable for `unreadableRepos`.
Two properties that are easy to get backwards, and are why the split matters:
- SyncResult carries the marker on EVERY outcome. The sync genuinely did skip
the stage whatever happened to the file afterwards, and the CLI summary (U3)
renders from this rather than re-deriving it from the caller's options.
- The PERSISTED registry stamps it only on the `written` outcome. The preserve
path re-writes `{ ...prior }`, so a carried-forward registry keeps the marker
of the sync that actually produced its contracts instead of being relabelled
with this run's request. That holds by construction: the registry literal
carrying the field is only reachable on the written path.
`loadContractRegistryResilient` gains an explicit line, because it rebuilds the
envelope field by field with no spread of the parsed root -- a new on-disk field
is silently dropped unless named there.
Its reader is `recordedMatchStages`, not the existing `recordedRepoList`: that
one validates `string[]`, which is right for repo names and one notch too weak
here. This repo has already retired MatchType members ('bm25', 'embedding'), so
a stale value on disk is a real shape, and dropping non-members keeps an unknown
stage name from reaching a caller typed as a live one.
Surfaced on `group_contracts` and on `group_sync`'s own return -- deliberately
kept separate from the truncated/truncationReason/riskEpistemic triple. That
triple reports limits a run hit by accident, whose remedy is to fix the repo; a
suppressed stage was asked for, and its remedy is to re-sync without the flag.
Conflating them would tell an agent to retry something that returns identically.
tsc clean; 1043/1043 group unit tests pass.
This commit is contained in:
parent
fd21d2de7b
commit
2c8a266d51
5 changed files with 199 additions and 0 deletions
|
|
@ -33,6 +33,7 @@ import type {
|
|||
CrossLink,
|
||||
GroupConfig,
|
||||
GroupContextResult,
|
||||
MatchType,
|
||||
StoredContract,
|
||||
} from './types.js';
|
||||
|
||||
|
|
@ -327,6 +328,7 @@ async function loadContractRegistryResilient(
|
|||
|
||||
// Bound once: the gate is a full array scan and the ternary below used it twice.
|
||||
const recordedUnreadable = recordedRepoList(base.unreadableRepos);
|
||||
const recordedSuppressed = recordedMatchStages(base.suppressedMatchStages);
|
||||
const registry: ContractRegistry = {
|
||||
version: typeof base.version === 'number' ? base.version : 0,
|
||||
generatedAt: typeof base.generatedAt === 'string' ? base.generatedAt : '',
|
||||
|
|
@ -348,6 +350,10 @@ async function loadContractRegistryResilient(
|
|||
// rendered as a clean result, which is the same conflation this whole
|
||||
// change removes.
|
||||
...(recordedUnreadable ? { unreadableRepos: recordedUnreadable } : {}),
|
||||
// Same omit-when-unrecorded rule. This reader rebuilds the envelope field
|
||||
// by field with no spread of `base`, so a new on-disk field is dropped
|
||||
// unless it is named here.
|
||||
...(recordedSuppressed ? { suppressedMatchStages: recordedSuppressed } : {}),
|
||||
contracts,
|
||||
crossLinks,
|
||||
};
|
||||
|
|
@ -355,6 +361,21 @@ async function loadContractRegistryResilient(
|
|||
return { ok: true, registry, skippedCorrupt };
|
||||
}
|
||||
|
||||
/**
|
||||
* Read a persisted `suppressedMatchStages` list.
|
||||
*
|
||||
* Deliberately NOT `recordedRepoList`: that validates `string[]`, which is
|
||||
* right for repo names and one notch too weak here. This repo has already
|
||||
* retired MatchType members ('bm25', 'embedding'), so a stale value on disk is
|
||||
* a real shape — dropping non-members keeps an unknown stage name from
|
||||
* reaching a caller typed as a live one. Absence stays absence (tri-state).
|
||||
*/
|
||||
function recordedMatchStages(value: unknown): MatchType[] | undefined {
|
||||
if (!Array.isArray(value)) return undefined;
|
||||
const known: MatchType[] = ['exact', 'manifest', 'wildcard'];
|
||||
return value.filter((v): v is MatchType => known.includes(v as MatchType));
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate a boolean MCP parameter — reject, never coerce.
|
||||
*
|
||||
|
|
@ -470,6 +491,10 @@ export class GroupService {
|
|||
unmatched: result.unmatched.length,
|
||||
missingRepos: result.missingRepos,
|
||||
unreadableRepos: result.unreadableRepos,
|
||||
// The agent-facing half of the skipped-stage signal. A human sees it in
|
||||
// the CLI summary; without this an agent would have to issue a second
|
||||
// `group_contracts` call to discover its own sync was narrowed.
|
||||
suppressedMatchStages: result.suppressedMatchStages,
|
||||
// An agent that calls group_sync and then group_contracts a moment later
|
||||
// can otherwise see contract counts that disagree with this payload, with
|
||||
// nothing here explaining why the write was skipped.
|
||||
|
|
@ -529,6 +554,13 @@ export class GroupService {
|
|||
// convention `skippedCorrupt` follows below, and the difference between
|
||||
// "the sync measured zero unreadable repos" and "the sync never said".
|
||||
...(unreadableRepos ? { unreadableRepos } : {}),
|
||||
// Same omit-when-unrecorded rule, and deliberately NOT folded into the
|
||||
// truncation triple below: that triple reports limits a run hit by
|
||||
// accident, whose remedy is to fix the repo. A suppressed stage was
|
||||
// asked for, and its remedy is to re-sync without that flag.
|
||||
...(registry.suppressedMatchStages
|
||||
? { suppressedMatchStages: registry.suppressedMatchStages }
|
||||
: {}),
|
||||
// The structured triple, verbatim from the impact surface (KTD10):
|
||||
// `truncated` always, `truncationReason` + `riskEpistemic` with it.
|
||||
...truncation,
|
||||
|
|
|
|||
|
|
@ -19,6 +19,7 @@ import type {
|
|||
StoredContract,
|
||||
CrossLink,
|
||||
GroupManifestLink,
|
||||
MatchType,
|
||||
} from './types.js';
|
||||
import { HttpRouteExtractor } from './extractors/http-route-extractor.js';
|
||||
import { GrpcExtractor } from './extractors/grpc-extractor.js';
|
||||
|
|
@ -94,6 +95,14 @@ export interface SyncResult {
|
|||
*/
|
||||
unreadableRepos: string[];
|
||||
repoSnapshots: Record<string, RepoSnapshot>;
|
||||
/**
|
||||
* Matching stages this run was asked to skip. Populated on EVERY outcome,
|
||||
* not just `written`: the sync genuinely did skip the stage whatever
|
||||
* happened to the file afterwards, and the CLI summary renders from this
|
||||
* rather than re-deriving it from the caller's options. Only the persisted
|
||||
* registry stamps it conditionally — see the write below.
|
||||
*/
|
||||
suppressedMatchStages: MatchType[];
|
||||
/**
|
||||
* What this sync did to `contracts.json`. Callers must not announce a write
|
||||
* they did not get: without this, `group sync` printed "Wrote contracts.json
|
||||
|
|
@ -572,6 +581,9 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis
|
|||
const wildcard: WildcardMatchResult = opts?.exactOnly
|
||||
? { matched: [], remaining: unmatched }
|
||||
: runWildcardMatch(unmatched, providerIndex);
|
||||
// Measured, not assumed: `[]` says this run suppressed nothing, which is a
|
||||
// different statement from a registry that never recorded the field at all.
|
||||
const suppressedMatchStages: MatchType[] = opts?.exactOnly ? ['wildcard'] : [];
|
||||
|
||||
// Dedupe cross-links. Manifest contracts participate in runExactMatch, so a
|
||||
// manifest-declared link can also emit a matchType:'exact' CrossLink with the
|
||||
|
|
@ -593,6 +605,11 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis
|
|||
// not recorded, telling the operator to re-run the sync that had just
|
||||
// succeeded. The tri-state only works if the writer commits to it.
|
||||
unreadableRepos,
|
||||
// Stamped only on the path that writes THIS run's contracts. The preserve
|
||||
// path below re-writes `{ ...prior }`, so a carried-forward registry keeps
|
||||
// the marker of the sync that actually produced its contracts instead of
|
||||
// being relabelled with this run's request.
|
||||
suppressedMatchStages,
|
||||
contracts: allContracts,
|
||||
crossLinks,
|
||||
};
|
||||
|
|
@ -802,6 +819,7 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis
|
|||
return {
|
||||
contracts: allContracts,
|
||||
crossLinks,
|
||||
suppressedMatchStages,
|
||||
unmatched: wildcard.remaining,
|
||||
missingRepos,
|
||||
unreadableRepos,
|
||||
|
|
|
|||
|
|
@ -111,6 +111,20 @@ export interface ContractRegistry {
|
|||
* absent means "not recorded", not "none".
|
||||
*/
|
||||
unreadableRepos?: string[];
|
||||
/**
|
||||
* Matching stages this sync was ASKED to skip, so a later reader can tell a
|
||||
* short cross-link list from a complete one. `--exact-only` / `exactOnly`
|
||||
* suppresses the wildcard stage, and the registry it writes is otherwise
|
||||
* indistinguishable from one where that stage ran and matched nothing.
|
||||
*
|
||||
* Same tri-state as `unreadableRepos` and for the same reason: absent means
|
||||
* "not recorded" (written before this field existed), `[]` means "measured,
|
||||
* nothing was suppressed", and a populated list names the stages. Distinct
|
||||
* from `truncated` / `truncationReason`, which report limits this run hit by
|
||||
* accident — a suppressed stage is a deliberate request, and its remedy is
|
||||
* "re-sync without exactOnly", not "fix the unreadable repo".
|
||||
*/
|
||||
suppressedMatchStages?: MatchType[];
|
||||
contracts: StoredContract[];
|
||||
crossLinks: CrossLink[];
|
||||
}
|
||||
|
|
|
|||
132
gitnexus/test/unit/group/registry-suppressed-stages.test.ts
Normal file
132
gitnexus/test/unit/group/registry-suppressed-stages.test.ts
Normal file
|
|
@ -0,0 +1,132 @@
|
|||
/**
|
||||
* `suppressedMatchStages` — what a sync says about the stages it was told to skip.
|
||||
*
|
||||
* `--exact-only` / `exactOnly` suppresses the wildcard matching stage. The
|
||||
* registry it writes is otherwise indistinguishable from one where that stage
|
||||
* ran and matched nothing, and `group_impact` / cross-repo `trace` read that
|
||||
* registry as authoritative. So the sync has to say so.
|
||||
*
|
||||
* The tri-state is the same one `unreadableRepos` uses, and the reason is the
|
||||
* same: ABSENT means a registry written before the field existed and therefore
|
||||
* has no opinion; EMPTY is a measurement — this run suppressed nothing;
|
||||
* POPULATED names the stages. Normalizing absent to `[]` would report an
|
||||
* unmeasured registry as a clean one.
|
||||
*
|
||||
* Two properties here are easy to get wrong and are pinned deliberately:
|
||||
* the returned result carries the marker on EVERY outcome (the sync really did
|
||||
* skip the stage whatever happened to the file), while the persisted registry
|
||||
* stamps it only on the outcome that writes this run's contracts.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import fs from 'node:fs';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
import { syncGroup } from '../../../src/core/group/sync.js';
|
||||
import type {
|
||||
GroupConfig,
|
||||
StoredContract,
|
||||
ContractRegistry,
|
||||
} from '../../../src/core/group/types.js';
|
||||
|
||||
const config: GroupConfig = {
|
||||
version: 1,
|
||||
name: 'suppressed',
|
||||
description: '',
|
||||
repos: { 'app/provider': 'provider-repo', 'app/consumer': 'consumer-repo' },
|
||||
links: [],
|
||||
packages: {},
|
||||
detect: {
|
||||
http: true,
|
||||
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,
|
||||
},
|
||||
};
|
||||
|
||||
/** The one shape `runWildcardMatch` fires on: a thrift service wildcard consumer. */
|
||||
const provider: StoredContract = {
|
||||
contractId: 'thrift::billing.v1.OrderService/PlaceOrder',
|
||||
type: 'thrift',
|
||||
role: 'provider',
|
||||
symbolUid: 'uid-provider',
|
||||
symbolRef: { filePath: 'src/provider.ts', name: 'OrderService.PlaceOrder' },
|
||||
symbolName: 'OrderService.PlaceOrder',
|
||||
confidence: 0.9,
|
||||
meta: {},
|
||||
repo: 'app/provider',
|
||||
};
|
||||
|
||||
const consumer: StoredContract = {
|
||||
contractId: 'thrift::OrderService/*',
|
||||
type: 'thrift',
|
||||
role: 'consumer',
|
||||
symbolUid: 'uid-consumer',
|
||||
symbolRef: { filePath: 'src/consumer.ts', name: 'callOrderService' },
|
||||
symbolName: 'callOrderService',
|
||||
confidence: 0.9,
|
||||
meta: {},
|
||||
repo: 'app/consumer',
|
||||
};
|
||||
|
||||
let groupDir: string;
|
||||
|
||||
beforeEach(() => {
|
||||
groupDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-suppressed-'));
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllEnvs();
|
||||
fs.rmSync(groupDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
const run = (exactOnly: boolean, opts: { write: boolean } = { write: false }) =>
|
||||
syncGroup(config, {
|
||||
extractorOverride: async () => [provider, consumer],
|
||||
exactOnly,
|
||||
...(opts.write ? { groupDir } : { skipWrite: true }),
|
||||
});
|
||||
|
||||
const readRegistry = (): ContractRegistry =>
|
||||
JSON.parse(fs.readFileSync(path.join(groupDir, 'contracts.json'), 'utf8')) as ContractRegistry;
|
||||
|
||||
describe('a sync records the matching stages it was told to skip', () => {
|
||||
it('names the wildcard stage when exactOnly suppressed it', async () => {
|
||||
const result = await run(true);
|
||||
|
||||
expect(result.suppressedMatchStages).toEqual(['wildcard']);
|
||||
expect(result.crossLinks).toEqual([]);
|
||||
});
|
||||
|
||||
// control: the marker tracks the request, not a constant. Without this, a
|
||||
// hardcoded `['wildcard']` would pass the case above.
|
||||
it('control: measures an empty list when no stage was suppressed', async () => {
|
||||
const result = await run(false);
|
||||
|
||||
expect(result.suppressedMatchStages).toEqual([]);
|
||||
expect(result.crossLinks).toHaveLength(1);
|
||||
expect(result.crossLinks[0].matchType).toBe('wildcard');
|
||||
});
|
||||
|
||||
it('persists the marker into contracts.json on a written sync', async () => {
|
||||
await run(true, { write: true });
|
||||
|
||||
expect(readRegistry().suppressedMatchStages).toEqual(['wildcard']);
|
||||
});
|
||||
|
||||
it('persists an empty measurement, not an absent key, on an unsuppressed sync', async () => {
|
||||
await run(false, { write: true });
|
||||
|
||||
const registry = readRegistry();
|
||||
expect(registry.suppressedMatchStages).toEqual([]);
|
||||
// The distinction the tri-state exists for: a measured zero is not silence.
|
||||
expect(registry).toHaveProperty('suppressedMatchStages');
|
||||
});
|
||||
});
|
||||
|
|
@ -82,6 +82,7 @@ const syncResult = (overrides: Partial<SyncResult> = {}): SyncResult => ({
|
|||
missingRepos: [],
|
||||
unreadableRepos: [],
|
||||
repoSnapshots: {},
|
||||
suppressedMatchStages: [],
|
||||
registryOutcome: 'written',
|
||||
...overrides,
|
||||
});
|
||||
|
|
@ -172,6 +173,7 @@ describe('group_sync forwards what the sync learned about the repos and the file
|
|||
unmatched: 1,
|
||||
missingRepos: ['app/frontend'],
|
||||
unreadableRepos: ['app/backend'],
|
||||
suppressedMatchStages: [],
|
||||
registryOutcome: 'preserved',
|
||||
});
|
||||
});
|
||||
|
|
@ -190,6 +192,7 @@ describe('group_sync forwards what the sync learned about the repos and the file
|
|||
unmatched: 0,
|
||||
missingRepos: [],
|
||||
unreadableRepos: [],
|
||||
suppressedMatchStages: [],
|
||||
registryOutcome: 'written',
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue