fix(core): reject a fractional chunk size, and stop the truncation note overclaiming (#2915)

All five from the gitnexus-check bot's pass on the previous push; two were
introduced by the cleanup round that preceded it.

`chunk` guarded with `Number.isFinite`, which admits a fractional size — and
that one does not fail, it DUPLICATES. `slice` truncates its indices while `i`
does not, so size 1.5 yields slice(0, 1.5) = items 0-1 then slice(1.5, 3) =
items 1-2, putting item 1 in two batches; a caller batching a query would send
it twice. A size is a count, so `Number.isInteger`. Unreachable today (every
caller passes a constant) but the guard existed precisely for the unreachable
case, and the NaN half of it was already there.

`mapConcurrent`'s per-item degradation contract had a hole: `onError` is
caller-supplied and was invoked outside a try, so a throwing reporter rejected
`settle`, rejected the whole `Promise.all` wave, and discarded the neighbouring
successes the function exists to preserve. Reporting a failure must not become
one.

The CLI truncation note asserted "the counts and risk level still cover all of
them", which is true only when `truncated` fires alone — with `partial` the
counts are summed from the batches that succeeded. It now varies: a distinct
string when both flags are set, saying the counts are a lower bound. This is the
same claim already corrected in the tool description; the CLI text still had the
old one.

The di-extractors contract docstring claimed the barrel re-exports everything
from it. That stopped being true when the re-export was trimmed to what is
actually imported, one commit earlier.

The real-engine wiki test claimed to prepare "every exported query" and omitted
`getInterModuleEdgesForOverview`, which `generateOverview` calls. Added — it
aggregates in JS over `getInterFileCallEdges` rather than issuing its own
Cypher, so the note says why it is in a prepare test.

Verified and NOT changed: the bot's other error, that detect_changes compares
1-based hunks against 0-based graph lines. `bounds` is built from
`coalesceHunksByPath`, which converts both ends at the grouping boundary
(storage/git.ts), and two tests pin an edit landing on a symbol's last line.
The remaining seven findings are changed-symbol heads-ups with no signature
change; their callers' suites are green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0139ycbqnAorkJGQaZQXUNuv

gitnexus/src/cli/detect-changes-format.ts

@@ -46,7 +46,13 @@ export function formatDetectChangesResult(result: unknown): string {
   // Both lead the output — a caveat printed after the summary is read too late.
   const notes: string[] = [];
   if (payload.partial) notes.push(t('tool.detectChanges.partial'));
-  if (payload.truncated) notes.push(t('tool.detectChanges.truncated'));
+  // The plain truncation note reassures that the counts are whole. That is only
+  // true when the run did NOT also degrade — `changed_count` sums the batches
+  // that succeeded — so the two flags together get a different sentence.
+  if (payload.truncated)
+    notes.push(
+      t(payload.partial ? 'tool.detectChanges.truncatedDegraded' : 'tool.detectChanges.truncated'),
+    );
 
   if ((summary.changed_count ?? 0) === 0) {
     return [...notes, t('tool.detectChanges.noChanges')].join('\n');

gitnexus/src/cli/i18n/en.ts

@@ -69,6 +69,10 @@ export const en = {
     'PARTIAL RESULT: a graph query failed, so changed symbols may be missing. Do not read this as a clean pre-commit check.',
   'tool.detectChanges.truncated':
     'LISTING CAPPED: the changed-symbol list was capped, so it does not name every changed symbol. The counts and risk level still cover all of them.',
+  // The reassurance above is only true on its own. When the run also degraded,
+  // `changed_count` was summed from the batches that SUCCEEDED, so it is a floor.
+  'tool.detectChanges.truncatedDegraded':
+    'LISTING CAPPED: the changed-symbol list was capped. The run also degraded, so the counts are a lower bound, not a total.',
   'tool.detectChanges.changesSummary': 'Changes: {{files}} files, {{symbols}} symbols',
   'tool.detectChanges.affectedProcesses': 'Affected processes: {{count}}',
   'tool.detectChanges.riskLevel': 'Risk level: {{risk}}',

gitnexus/src/cli/i18n/zh-CN.ts

@@ -73,6 +73,8 @@ export const zhCN = {
     '结果不完整：图查询失败，可能遗漏已变更符号。请勿将其视为通过的提交前检查。',
   'tool.detectChanges.truncated':
     '列表已截断：已变更符号列表被截断，未列出全部变更符号。计数与风险等级仍涵盖全部符号。',
+  'tool.detectChanges.truncatedDegraded':
+    '列表已截断：已变更符号列表被截断。本次运行同时不完整，因此计数为下限而非总数。',
   'tool.detectChanges.changesSummary': '变更：{{files}} 个文件，{{symbols}} 个符号',
   'tool.detectChanges.affectedProcesses': '受影响流程：{{count}}',
   'tool.detectChanges.riskLevel': '风险等级：{{risk}}',

gitnexus/src/core/ingestion/di-extractors/types.ts

@@ -5,8 +5,10 @@
  * A leaf module by design: it imports nothing from this directory, so the
  * barrel (`./index.ts`, which aggregates the resolver *implementations*) and
  * each implementation (`./spring.ts`) can both depend on the contract without
- * depending on each other. `./index.ts` re-exports everything here, so
- * consumers may keep importing these types from the barrel.
+ * depending on each other. The barrel re-exports the two MATCH types, because
+ * consumers of the registry read them off its results; `DiResolver` is not
+ * re-exported, since only implementations need it and they import it from here
+ * directly.
  *
  * Mirrors the `import-resolvers/types.ts` split of contract from registry.
  */

gitnexus/src/lib/utils.ts

@@ -135,12 +135,17 @@ export const stripWindowsLongPathPrefix = (
  */
 export function chunk<T>(items: readonly T[], size: number): T[][] {
   // `NaN` fails every comparison, so a bare `size < 1` lets it through and
-  // `i += NaN` produces exactly one empty slice — the shape the docstring above
-  // promises never to return. `mapConcurrent`'s `Math.max(1, …)` propagates a
-  // NaN concurrency the same way, so the guard has to cover non-finite sizes
-  // rather than just small ones.
-  if (!Number.isFinite(size) || size < 1) {
-    throw new RangeError(`chunk size must be >= 1, got ${size}`);
+  // A size is a COUNT, so it has to be a positive integer — `Number.isInteger`
+  // rather than `Number.isFinite`, because a fractional size silently DUPLICATES
+  // items rather than failing: `slice` truncates its indices while `i` does not,
+  // so size 1.5 yields slice(0, 1.5) = items 0-1 and then slice(1.5, 3) = items
+  // 1-2, and item 1 lands in two batches. `NaN` is the other shape this rejects —
+  // it fails every comparison, so a bare `size < 1` lets it through and `i += NaN`
+  // produces exactly one empty slice, which the docstring promises never to
+  // return. `mapConcurrent`'s `Math.max(1, …)` propagates a NaN concurrency the
+  // same way.
+  if (!Number.isInteger(size) || size < 1) {
+    throw new RangeError(`chunk size must be a positive integer, got ${size}`);
   }
   const batches: T[][] = [];
   for (let i = 0; i < items.length; i += size) batches.push(items.slice(i, i + size));
@@ -171,7 +176,15 @@ export async function mapConcurrent<T, R>(
     try {
       return await run(item);
     } catch (error) {
-      options.onError?.(error);
+      // Reporting a failure must not become a failure. `onError` is caller-supplied
+      // — a logger with a bad format string is enough — and an uncaught throw here
+      // rejects `settle`, which rejects the whole `Promise.all` wave and discards
+      // the neighbouring successes this function exists to preserve.
+      try {
+        options.onError?.(error);
+      } catch {
+        /* empty */
+      }
       return undefined;
     }
   };

gitnexus/test/integration/wiki-graph-queries-engine.test.ts

@@ -29,6 +29,7 @@ import {
   getFilesWithExports,
   getInterFileCallEdges,
   getInterModuleCallEdges,
+  getInterModuleEdgesForOverview,
   getIntraModuleCallEdges,
   getProcessesForFiles,
   initWikiDb,
@@ -259,6 +260,10 @@ withTestLbugDB(
             getInterModuleCallEdges(MODULE_FILES),
             getProcessesForFiles(MODULE_FILES),
             getAllProcesses(),
+            // Aggregates in JS over `getInterFileCallEdges`, so it issues no
+            // Cypher of its own — included anyway because this test claims to
+            // cover every exported query, and `generateOverview` calls it.
+            getInterModuleEdgesForOverview({ mod: MODULE_FILES, other: [GAMMA] }),
           ]),
         ).resolves.toBeDefined();
       });

gitnexus/test/unit/eval-formatters.test.ts

@@ -597,6 +597,12 @@ describe('formatDetectChangesResult', () => {
     expect(result.indexOf('PARTIAL RESULT')).toBe(0);
     expect(result.indexOf('LISTING CAPPED')).toBeGreaterThan(0);
     expect(result.indexOf('LISTING CAPPED')).toBeLessThan(result.indexOf('Changes: 40 files'));
+    // And it must NOT keep the truncated-only reassurance that the counts are
+    // whole: `changed_count` was summed from the batches that succeeded, so with
+    // `partial` it is a floor. Claiming otherwise here contradicts the note above
+    // it and the tool description.
+    expect(result).toContain('lower bound');
+    expect(result).not.toContain('still cover all of them');
   });
 
   it('flags a capped listing that found nothing, alongside the no-changes line', () => {

gitnexus/test/unit/query-batch.test.ts

@@ -47,4 +47,13 @@ describe('chunk', () => {
     // promises never to return, and one a caller reads as "nothing to query".
     expect(() => chunk([1], Number.NaN)).toThrow(RangeError);
   });
+
+  it('rejects a fractional size, which would DUPLICATE an item rather than fail', () => {
+    // The nastier sibling of the NaN case, because it produces a plausible-looking
+    // result instead of an empty one. `slice` truncates its indices but `i` does
+    // not, so size 1.5 gives slice(0, 1.5) = items 0-1 then slice(1.5, 3) = items
+    // 1-2: 'b' is in two batches, and a caller batching a query would send it
+    // twice. `Number.isFinite` admits this; only `Number.isInteger` rejects it.
+    expect(() => chunk(['a', 'b', 'c'], 1.5)).toThrow(RangeError);
+  });
 });