scriptoriumadmin/GitNexus
1
0
Fork 0
mirror of https://github.com/abhigyanpatwari/GitNexus.git synced 2026-08-28 05:25:25 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 9

refactor(group): apply simplify-pass findings

Browse source 
Four cleanup agents over the last five commits. Efficiency was clean and traced
why: the containment helper is failure-path only, the reason ternary sits after
the fan-out loop, and the tri-state readers run once per artifact read.

The strongest finding was one the diff itself proved. `refreshPreservedBridgeMeta`
enforced the never-persisted rule for `repoListsUnreadable` and
`pairedWithDatabase` with two deletes in its own body, under a comment noting it
was the only code that read metadata and wrote it back. That held exactly as
long as there was one such caller. `markBridgeProvenanceUnknown` made it two,
and inherited nothing. The strip now lives in `writeBridgeMeta`, so every writer
gets it and no future one can forget; `pairedWithDatabase` is the dangerous one,
because persisted it tells every later reader the pair was verified when nothing
verified it.

`group impact` still printed "fan-out stopped early" whenever `truncatedRepos`
was non-empty — but the bridge's incomplete repos are unioned into that list
even when zero crossings were attempted, so a structural gap was reported as a
runtime one, with the only working remedy omitted. That is the same false-cause
shape the contract listing was re-gated for one commit ago, left live one
command over because the new reason was bolted in front of the old branch rather
than replacing the thing it branched on. Now keyed on the reason.

The `?? 'incomplete-sync'` arm in cross-impact was unreachable: reaching it
needs `truncated` true with all three of its inputs false, which
`truncated = runtimeTruncated || bridge.truncated` forbids. Flattened.

Also: a `recordedMatchStages` insert had split `crossRepoCompleteness` from its
own JSDoc; one new test was a strict subset of another; and the bridge-failure
warning interleaved concatenation with a mid-chain ternary.

The new invariant assertion was caught being VACUOUS by mutation before it
shipped — seeded with a valid repo list, `readBridgeMeta` never sets the
reader-only field, so it passed with or without the strip. The fixture now seeds
an unreadable list, and both it and the pre-existing assertion go red when the
strip is removed.

Deliberately skipped, with reasons: a shared `firstTruncated` fold over
`TruncationFields` (the right altitude, but it changes cross-trace's return
assembly and that surface separately documents a 'timeout' rung it cannot emit —
a behavior change, not a cleanup); a reason-keyed `explainFloor` helper across
all four CLI renderers (real, but a four-site refactor); narrowing the persisted
stage vocabulary to a `SuppressibleStage` alias (would be undone by the very
extension the field was modelled as a list to allow); moving `verbose` to
`logger.debug` and deleting `SyncOptions.verbose` (the maintainer explicitly
directed keeping both); and merging the two tri-state readers behind a predicate
(they are adjacent in one file now, so a tightening applies to both by
inspection — the duplication the comment warned about was cross-FILE).

tsc clean; eslint 0 errors; 1164/1164.
This commit is contained in:
Gergo Magyar 2026-08-27 16:41:52 +00:00
parent b1665d11e4
commit 2d2ef8c47f
7 changed files with 88 additions and 61 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

32
gitnexus/src/cli/group.ts
View file

@ -435,17 +435,27 @@ export function registerGroupCommands(program: Command): void {
// same way #2787's totals did.
const dropped = (raw as { truncatedRepos?: string[] })?.truncatedRepos ?? [];
const reason = (raw as { truncationReason?: string })?.truncationReason;
// A floor caused by the flag is not a floor caused by the walk.
// Reported together, an operator re-runs the query or repairs a
// repo and gets the identical answer, because neither is the fix.
console.log(
reason === 'suppressed-stage'
? ' risk is a LOWER BOUND — the last sync skipped a matching stage (--exact-only);' +
' re-run `gitnexus group sync` without it for the complete graph'
: dropped.length > 0
? ` risk is a LOWER BOUND — fan-out stopped early; crossings to ${dropped.length} repo(s) not traversed: ${dropped.join(', ')}`
: ' risk is a LOWER BOUND — the local impact walk did not complete (every bridge crossing was traversed)',
);
// Keyed on the REASON, not on which incidental fact happens to be
// non-empty. `truncatedRepos` is populated for a structural gap too
// — the bridge's incomplete repos are unioned into it even when ZERO
// crossings were attempted — so branching on its length first
// reported "fan-out stopped early" for a run where nothing stopped
// early, and omitted the only remedy that works. Same false-cause
// shape the contract listing was just re-gated for, one command over.
const floorReason = (): string => {
if (reason === 'suppressed-stage') {
return 'the last sync skipped a matching stage (--exact-only); re-run `gitnexus group sync` without it for the complete graph';
}
if (reason === 'incomplete-sync') {
return dropped.length > 0
? `the last sync could not account for ${dropped.join(', ')}; their contracts are absent from every query against this bridge — re-run \`gitnexus group sync\``
: 'the last sync could not say which repos it read — re-run `gitnexus group sync`';
}
return dropped.length > 0
? `fan-out stopped early; crossings to ${dropped.length} repo(s) not traversed: ${dropped.join(', ')}`
: 'the local impact walk did not complete (every bridge crossing was traversed)';
};
console.log(` risk is a LOWER BOUND — ${floorReason()}`);
}
}
} finally {

16
gitnexus/src/core/group/bridge-db.ts
View file

@ -656,7 +656,15 @@ export async function closeBridgeDb(handle: BridgeHandle): Promise<void> {
/* ------------------------------------------------------------------ */
export async function writeBridgeMeta(groupDir: string, meta: BridgeMeta): Promise<void> {
await writeFileAtomic(path.join(groupDir, 'meta.json'), JSON.stringify(meta, null, 2));
// Strip the reader-only fields HERE rather than at each writer. `readBridgeMeta`
// sets both on what it returns, so any caller that reads-modifies-writes would
// round-trip them to disk — and `pairedWithDatabase` is the poisonous one:
// persisted, it tells every future reader the pair was verified when nothing
// verified it. That rule used to live in the body of the only such caller,
// which held exactly as long as there was one. There are now three writers and
// two of them read first. Enforced at the boundary, no writer can get it wrong.
const { repoListsUnreadable: _reader1, pairedWithDatabase: _reader2, ...persisted } = meta;
await writeFileAtomic(path.join(groupDir, 'meta.json'), JSON.stringify(persisted, null, 2));
}
/**
@ -938,10 +946,8 @@ export async function refreshPreservedBridgeMeta(
const refreshed: BridgeMeta = { ...existing, ...diagnostics };
// NEVER PERSISTED (see `BridgeMeta`): both are things a READER computes ABOUT
// a file, and this is the first code in the repo that reads metadata and
// writes it back. `pairedWithDatabase` is the poisonous one — persisted, it
// would tell every future reader that the pair had been verified.
delete refreshed.repoListsUnreadable;
delete refreshed.pairedWithDatabase;
// writes it back. The strip itself now lives in `writeBridgeMeta`, so every
// writer inherits it rather than each remembering.
if (paired) {
const stat = await fsp.stat(dbPath).catch(() => null);

28
gitnexus/src/core/group/completeness.ts
View file

@ -99,20 +99,6 @@ export type CrossRepoCompleteness = TruncationFields & {
incompleteRepos: string[];
};
/**
* The ONE computation of "is this cross-repo answer complete?" (KTD10).
*
* Three surfaces can return a partial cross-repo answer — impact, trace, and
* the contract listing — and each used to decide for itself, in its own
* vocabulary, which is how two of them ended up saying it in prose only. The
* answer is the same structured triple `GroupImpactResult` already carries, so
* an agent reading any of them learns "complete" vs "floor" the same way.
*
* `truncationFields` derives `riskEpistemic` from `truncated` mechanically, and
* is reused here rather than re-implemented for the same reason it exists: the
* marker that says "this is a floor, not a verdict" may never drift away from
* the flag that says the answer was cut short (#2787).
*/
/**
* Read a persisted `suppressedMatchStages` list.
*
@ -130,6 +116,20 @@ export function recordedMatchStages(value: unknown): MatchType[] | undefined {
return value.every((v): v is MatchType => known.includes(v as MatchType)) ? value : undefined;
}
/**
* The ONE computation of "is this cross-repo answer complete?" (KTD10).
*
* Three surfaces can return a partial cross-repo answer — impact, trace, and
* the contract listing — and each used to decide for itself, in its own
* vocabulary, which is how two of them ended up saying it in prose only. The
* answer is the same structured triple `GroupImpactResult` already carries, so
* an agent reading any of them learns "complete" vs "floor" the same way.
*
* `truncationFields` derives `riskEpistemic` from `truncated` mechanically, and
* is reused here rather than re-implemented for the same reason it exists: the
* marker that says "this is a floor, not a verdict" may never drift away from
* the flag that says the answer was cut short (#2787).
*/
export function crossRepoCompleteness(input: CrossRepoCompletenessInput): CrossRepoCompleteness {
const incompleteRepos = [
...new Set([...(input.unreadableRepos ?? []), ...(input.missingRepos ?? [])]),

4
gitnexus/src/core/group/cross-impact.ts
View file

@ -893,7 +893,9 @@ export async function runGroupImpact(
? 'timeout'
: runtimeTruncated
? 'partial'
: ((bridge.truncated ? bridge.truncationReason : undefined) ?? 'incomplete-sync'),
: bridge.truncated
? bridge.truncationReason
: 'incomplete-sync',
),
truncatedRepos: [...new Set([...truncatedRepos, ...bridge.incompleteRepos])],
summary: {

12
gitnexus/src/core/group/sync.ts
View file

@ -816,16 +816,16 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis
// withdraws the claim without touching the database — the previous
// graph stays queryable, it just stops being called complete.
const withdrawn = await markBridgeProvenanceUnknown(groupDir);
const provenanceNote = withdrawn
? 'Its metadata has been marked provenance-unknown, so those answers now report as ' +
'a lower bound rather than as complete.'
: 'Its metadata could NOT be marked provenance-unknown, so those answers may still ' +
'report as complete despite describing an older sync.';
logger.warn(
{ err: msg, groupDir, bridgeProvenanceWithdrawn: withdrawn },
'⚠️ writeBridge failed; contracts.json is intact and is the canonical copy, ' +
'but bridge.lbug was not replaced: cross-repo queries may still answer from ' +
"the previous sync's contracts. " +
(withdrawn
? 'Its metadata has been marked provenance-unknown, so those answers now ' +
'report as a lower bound rather than as complete. '
: 'Its metadata could NOT be marked provenance-unknown, so those answers may ' +
'still report as complete despite describing an older sync. ') +
`the previous sync's contracts. ${provenanceNote} ` +
'Re-run `gitnexus group sync` to retry.',
);
}

24
gitnexus/test/unit/group/registry-suppressed-stages.test.ts
View file

@ -162,27 +162,15 @@ describe('cross-repo completeness reflects a suppressed stage', () => {
});
/**
* The reason has to be REACHABLE, not just documented.
* The reason must stay a DECLARED member of the union agents branch on.
*
* A guard in `tools.test.ts` asserts every truncation reason is explained in
* the impact tool description. That check passed while `runGroupImpact`
* hardcoded its fallback and could never emit `'suppressed-stage'` — a
* documented value no surface could produce. These pin the emitting side.
* That it is reachable from real code is already pinned above, by a case that
* drives `crossRepoCompleteness` and gets the value back. What that cannot see
* is the union itself shrinking: a guard in `tools.test.ts` asserts every
* member is documented, so dropping a member keeps that guard green while every
* consumer silently loses the value.
*/
describe('the suppressed-stage reason is reachable, not just documented', () => {
it('is the reason when a stage was suppressed and every repo read fine', () => {
const out = crossRepoCompleteness({
unreadableRepos: [],
missingRepos: [],
suppressedMatchStages: ['wildcard'],
provenanceUnknown: false,
inScope: () => true,
});
expect(out.truncated).toBe(true);
expect(out.truncationReason).toBe('suppressed-stage');
});
it('is a declared member of the reason union agents branch on', () => {
// If a future change drops it from the union, the tool description guard
// would still pass while every consumer lost the value.

33
gitnexus/test/unit/group/sync-unreadable-repos.test.ts
View file

@ -978,18 +978,39 @@ describe('the warning after a failed bridge write', () => {
// reports the new registry — two public surfaces, contradictory claims,
// out of one sync. Marking provenance unknown withdraws the completeness
// claim without deleting a graph still useful as a floor.
await writeBridgeMeta(groupDir, {
version: 1,
generatedAt: '2026-01-01T00:00:00.000Z',
missingRepos: [],
unreadableRepos: [],
});
// `unreadableRepos` is deliberately UNREADABLE here, not merely empty: that
// is what makes `readBridgeMeta` set the reader-only `repoListsUnreadable`
// on what it returns, so the assertion below can actually catch a
// read-modify-write writer round-tripping it back to disk. Seeded with a
// valid list the check passes whether or not the strip exists.
fs.writeFileSync(
path.join(groupDir, 'meta.json'),
JSON.stringify({
version: 1,
generatedAt: '2026-01-01T00:00:00.000Z',
missingRepos: [],
unreadableRepos: 'not-a-list',
}),
);
expect((await readBridgeMeta(groupDir)).repoListsUnreadable).toBe(true);
expect((await readBridgeMeta(groupDir)).provenanceUnknown).toBeUndefined();
writeBridgeFailure = new Error('ENOSPC: no space left on device');
await runSync();
expect((await readBridgeMeta(groupDir)).provenanceUnknown).toBe(true);
// ...and the withdrawal must not persist the reader-only fields.
// `readBridgeMeta` sets both on what it returns, so a read-modify-write
// writer round-trips them unless the write boundary strips them.
// `pairedWithDatabase` is the poisonous one: persisted, it would tell every
// later reader the pair was verified when nothing verified it.
const raw = JSON.parse(fs.readFileSync(path.join(groupDir, 'meta.json'), 'utf8')) as Record<
string,
unknown
>;
expect(raw).not.toHaveProperty('pairedWithDatabase');
expect(raw).not.toHaveProperty('repoListsUnreadable');
});
// control: a sync whose bridge write SUCCEEDS must not withdraw provenance —