From 1eeeb816fdc76879833141d1f0cbfff06b4a7bd1 Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Mon, 15 Jun 2026 18:05:08 +0000 Subject: [PATCH] docs(lbug): scope byte-identity to quote-free ids + lock the quote-in-id divergence (#2215 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 'byte-identical' claim was unconditional, but the router derives labels from the raw id while the retained splitRelCsvByLabelPair oracle re-derives them via a regex over the escaped row — so for an id containing a double-quote they diverge (the router is the more-correct path). Soften the wording in rel-pair-routing.ts, the bench README, and the differential-test comment to document the exception, and add a differential test asserting the intended divergence (router routes the quote-in-id edge; oracle drops it) so a future change can't silently revert to the buggy regex semantics. Co-Authored-By: Claude Opus 4.8 (1M context) --- gitnexus/bench/emit-persistence/README.md | 4 +- gitnexus/src/core/lbug/rel-pair-routing.ts | 14 ++-- .../test/integration/csv-pipeline.test.ts | 65 +++++++++++++++++-- 3 files changed, 73 insertions(+), 10 deletions(-) diff --git a/gitnexus/bench/emit-persistence/README.md b/gitnexus/bench/emit-persistence/README.md index 01f16de92..0a690f40b 100644 --- a/gitnexus/bench/emit-persistence/README.md +++ b/gitnexus/bench/emit-persistence/README.md @@ -25,7 +25,9 @@ scales: CSVs + per-FROM→TO-label-pair rel CSVs). This is the **byte-identity gate**: the U2 (direct per-pair routing) and U3 (per-row microtask elimination) optimisations must not change graph content, and any future change that does - fails `--check`. + fails `--check`. Byte-identity holds for all quote-free ids; for an id + containing a `"` the router intentionally diverges from — and is more correct + than — the legacy regex oracle (see `src/core/lbug/rel-pair-routing.ts`). ## What it does NOT measure diff --git a/gitnexus/src/core/lbug/rel-pair-routing.ts b/gitnexus/src/core/lbug/rel-pair-routing.ts index 453ffe0b1..a10a817fa 100644 --- a/gitnexus/src/core/lbug/rel-pair-routing.ts +++ b/gitnexus/src/core/lbug/rel-pair-routing.ts @@ -9,11 +9,15 @@ * * This router lets the single emit pass route each edge to its per-pair file * directly, so the monolithic write + re-read + per-edge regex are all gone. - * The label-derivation + validTables filtering + per-pair-file format here are - * the SAME ones the legacy `splitRelCsvByLabelPair` applies, so the per-pair - * files are byte-identical — see the differential test in - * `test/integration/csv-pipeline.test.ts`. `splitRelCsvByLabelPair` is retained - * as the differential oracle. + * The label-derivation + validTables filtering + per-pair-file format here match + * the legacy `splitRelCsvByLabelPair`, so the per-pair files are byte-identical + * for all quote-free ids — see the differential test in + * `test/integration/csv-pipeline.test.ts`. ONE intentional divergence: this + * router derives the label from the RAW id, while the oracle re-derives it via a + * regex over the ESCAPED row — so for an id containing a `"` the router is the + * more-correct path (it routes the edge to the right pair; the oracle's regex + * mis-buckets or drops it). `splitRelCsvByLabelPair` is retained as the + * differential oracle (the quote-in-id divergence is asserted explicitly). * * Backpressure: at most one stream is awaited at a time (the caller routes * edges sequentially and awaits the returned drain promise before the next), diff --git a/gitnexus/test/integration/csv-pipeline.test.ts b/gitnexus/test/integration/csv-pipeline.test.ts index 74b1abf1f..16ed577fb 100644 --- a/gitnexus/test/integration/csv-pipeline.test.ts +++ b/gitnexus/test/integration/csv-pipeline.test.ts @@ -344,10 +344,13 @@ describe('streamAllCSVsToDisk — deterministic output ordering', () => { }); /** - * #2203 U2 byte-identity: the direct per-pair emit must produce per-pair files - * byte-for-byte identical to the legacy splitRelCsvByLabelPair oracle run over - * an equivalent monolithic relations.csv from the same graph. This is the - * load-bearing guard for "byte-identical graph content" (issue acceptance). + * #2203 U2 byte-identity: for all quote-free ids the direct per-pair emit must + * produce per-pair files byte-for-byte identical to the legacy + * splitRelCsvByLabelPair oracle run over an equivalent monolithic relations.csv + * from the same graph. This is the load-bearing guard for "byte-identical graph + * content" (issue acceptance). The ONE intentional divergence — ids containing a + * double-quote, where the router (raw-id label) is more correct than the oracle + * (regex over the escaped row) — is asserted explicitly in its own test below. */ describe('streamAllCSVsToDisk — direct per-pair emit matches the split oracle', () => { // The oracle always emits in graph.iterRelationships() (unsorted) order; the @@ -426,4 +429,58 @@ describe('streamAllCSVsToDisk — direct per-pair emit matches the split oracle' expect(directContent, `pair ${key}`).toBe(oracleContent); } }); + + it('quote-in-id edge: router routes it (raw-id label) while the oracle drops it — intended divergence', async () => { + // A node id with an embedded double-quote (legal in a POSIX filePath). The + // router derives the label from the RAW id (`File`), so it routes the edge; + // the oracle re-derives the label via /"([^"]*)","([^"]*)"/ over the ESCAPED + // row (`"File:a""b.ts",...`), mis-reads the field, and drops it. This locks + // the intended divergence so a future change can't silently revert the + // router to the buggy regex semantics. + const graph = buildTestGraph( + [ + { id: 'File:clean.ts', label: 'File', name: 'clean.ts', filePath: 'clean.ts' }, + { id: 'File:a"b.ts', label: 'File', name: 'a"b.ts', filePath: 'a"b.ts' }, + { id: 'Function:a.ts:f:1', label: 'Function', name: 'f', filePath: 'a.ts' }, + ], + [ + { sourceId: 'File:clean.ts', targetId: 'Function:a.ts:f:1', type: 'CONTAINS' }, + { sourceId: 'File:a"b.ts', targetId: 'Function:a.ts:f:1', type: 'CONTAINS' }, + ], + ); + + const directDir = path.join(csvDir, 'qd-direct'); + const oracleDir = path.join(csvDir, 'qd-oracle'); + await fs.mkdir(oracleDir, { recursive: true }); + + const direct = await streamAllCSVsToDisk(graph, repoDir, directDir); + + const relCsv = path.join(oracleDir, 'relations.csv'); + const lines = [REL_CSV_HEADER]; + for (const rel of graph.iterRelationships()) lines.push(buildRelRow(rel)); + await fs.writeFile(relCsv, lines.join('\n') + '\n', 'utf-8'); + const split = await splitRelCsvByLabelPair( + relCsv, + oracleDir, + new Set(NODE_TABLES), + getNodeLabel, + ); + await Promise.all( + Array.from(split.pairWriteStreams.values()).map(async (ws) => { + ws.end(); + await finished(ws); + }), + ); + + // Router routes BOTH edges — the raw-id label `File` is valid for both. + expect(direct.totalValidRels).toBe(2); + expect(direct.skippedRels).toBe(0); + expect(direct.relsByPair.get('File|Function')!.rows).toBe(2); + + // Oracle DIVERGES: its regex mis-reads the quote-in-id row and drops that + // edge, so it routes strictly fewer edges. Asserted robustly — we do NOT + // pin the oracle's exact mis-derived label. + expect(split.totalValidRels).toBeLessThan(direct.totalValidRels); + expect(split.skippedRels).toBeGreaterThan(direct.skippedRels); + }); });