From dcae50bfe01f672c27608a16c538dc8f1c9342f9 Mon Sep 17 00:00:00 2001 From: test Date: Mon, 10 Aug 2026 23:53:18 +0000 Subject: [PATCH 1/5] Carry PR #701's stranded review fixes: skip the wasted resettle scan, MaintenanceReport symmetry, span visibility, LLP gloss PR #701 was squash-merged at head 287b67b, before the round-2 review fixes in cc82d6f were pushed, so four verified fixes never reached master. - Hoist a cheap `compactionDue` check above the `hasResettleCandidate` row scan and gate the scan on `!compactionDue`. Recognition of a foreign sorted `replace` outranks the resettle check, so the first tick after each foreign replace paid a complete single-column scan of the day purely to discard the answer. - Add `totalRebaselined` to `MaintenanceReport` beside `totalCompacted`, and let `query.js` read it instead of re-deriving the count. - Tag the enclosing `maintenance.partition` span with `rebaselined`; the `hyp_rebaselines` counter carries only the dataset, not the partition. - Give LLP 0199's bare `Extended-by: LLP 0207` the corpus's linked and glossed form. Co-Authored-By: Claude --- ...tenance-compaction-convergence.decision.md | 2 +- src/core/cache/maintenance.js | 26 ++++++++++++++++--- src/core/cache/types.d.ts | 1 + src/core/commands/query.js | 3 +-- 4 files changed, 26 insertions(+), 6 deletions(-) diff --git a/llp/0199-maintenance-compaction-convergence.decision.md b/llp/0199-maintenance-compaction-convergence.decision.md index a0232ef9d..606b4240e 100644 --- a/llp/0199-maintenance-compaction-convergence.decision.md +++ b/llp/0199-maintenance-compaction-convergence.decision.md @@ -6,7 +6,7 @@ **Author:** Kenny / Claude **Date:** 2026-08-07 **Related:** LLP 0027 -**Extended-by:** LLP 0207 +**Extended-by:** [LLP 0207](./0207-foreign-sorted-replace-convergence.decision.md) (a second convergence source: a foreign sorted `replace` re-baselines the gate instead of triggering a rewrite) > Maintenance stops re-flagging already-compacted partitions: a partition is > only compaction-due when its live data-file count has moved off the count diff --git a/src/core/cache/maintenance.js b/src/core/cache/maintenance.js index f1d97137b..4525ceaa1 100644 --- a/src/core/cache/maintenance.js +++ b/src/core/cache/maintenance.js @@ -10,7 +10,7 @@ import { loadLatestFileCatalogMetadata, } from 'icebird' -import { Attr, getMeter, withSpan } from '../observability/index.js' +import { Attr, getActiveSpan, getMeter, withSpan } from '../observability/index.js' import { inferColumnType } from './migrate.js' import { discoverCachePartitions, readCursorSync, tryReadCursorSync, writeCursor } from './partition.js' import { datasetsRoot } from './paths.js' @@ -117,6 +117,7 @@ export async function maintainCache(opts) { const reports = [] let totalSnapshotsExpired = 0 let totalCompacted = 0 + let totalRebaselined = 0 for (const part of partitions) { // Always work one partition before the budget can cut the tick short: @@ -157,6 +158,7 @@ export async function maintainCache(opts) { reports.push(report) totalSnapshotsExpired += report.snapshotsExpired if (report.compacted) totalCompacted++ + if (report.rebaselined) totalRebaselined++ } if (!opts.dryRun) { @@ -167,6 +169,7 @@ export async function maintainCache(opts) { partitions: reports, totalSnapshotsExpired, totalCompacted, + totalRebaselined, dryRun: opts.dryRun ?? false, elapsedMs: Date.now() - startMs, } @@ -275,6 +278,13 @@ async function maintainGeneration(r, cursor, cfg, opts, settle, snapshotsExpired // compact_avg_file_bytes), and the tick budget is burned rewriting the // same partitions while the rest of the walk starves. const grewSinceCompaction = dataFilesBefore !== resettleBaselineFiles(cursor) + // Cheap dueness check first: file-count and byte-size heuristics only, + // no metadata load and no row scan. A foreign sorted replace almost + // always lands here (its baseline mismatch alone doesn't imply the + // size heuristics fire), so gating the expensive re-settle scan behind + // this check means the common "recognized, nothing to scan for" tick + // never pays for one. + const compactionDue = opts.force || (grewSinceCompaction && needsCompaction(liveDir, cfg)) // @ref LLP 0027#re-settle-sweep: a partition holding a committed // fallback row may carry a split twin pair the flush-time settle // never collapsed; force a rewrite so the sweep can re-settle it even @@ -283,10 +293,15 @@ async function maintainGeneration(r, cursor, cfg, opts, settle, snapshotsExpired // line never lands (harness aux, wire-only reminders) - from forcing a // full rewrite every tick, and skips the attributes scan entirely when // nothing new has flushed. - const hasResettle = settle + // @ref LLP 0207#foreign-replace [constrained-by]: when the cheap check + // above already made compaction due, the scan's answer can never + // change the outcome (recognition, tested below, still outranks it), + // so skip it: only run the scan when it might be the sole reason to + // compact. + const hasResettle = !compactionDue && settle ? grewSinceCompaction && await hasResettleCandidate(liveDir) : false - const shouldCompact = opts.force || hasResettle || (grewSinceCompaction && needsCompaction(liveDir, cfg)) + const shouldCompact = compactionDue || hasResettle if (shouldCompact) { const tableInfo = await loadCompactionTableInfo(liveDir) // @ref LLP 0207#foreign-replace [implements]: a baseline mismatch whose @@ -299,6 +314,11 @@ async function maintainGeneration(r, cursor, cfg, opts, settle, snapshotsExpired // the sorted layout every night. An explicit --force still rewrites. if (!opts.force && foreignSortedReplace(tableInfo)) { r.rebaselined = true + // @ref LLP 0207#re-baseline: the counter proves a rebaseline happened + // at all, but it carries only the dataset; tagging the enclosing + // maintenance.partition span names the partition, so a trace query + // finds which day re-baselined without cross-referencing the counter. + getActiveSpan()?.setAttribute('rebaselined', true) if (!opts.dryRun) { await writeCursor(r.path, rebaselineCursor(cursor, dataFilesBefore)) rebaselinesCounter.add(1, { [Attr.DATASET]: r.dataset }) diff --git a/src/core/cache/types.d.ts b/src/core/cache/types.d.ts index 45084d72f..00d529d69 100644 --- a/src/core/cache/types.d.ts +++ b/src/core/cache/types.d.ts @@ -232,6 +232,7 @@ export interface MaintenanceReport { partitions: MaintenancePartitionReport[] totalSnapshotsExpired: number totalCompacted: number + totalRebaselined: number dryRun: boolean elapsedMs: number } diff --git a/src/core/commands/query.js b/src/core/commands/query.js index 1cfac29d2..b22afa21d 100644 --- a/src/core/commands/query.js +++ b/src/core/commands/query.js @@ -327,8 +327,7 @@ export async function runQueryMaintain(argv, ctx) { ctx.stdout.write(` ${label}: ${actions.join(', ')}\n`) } } - const rebaselined = report.partitions.filter((p) => p.rebaselined).length - const rebaselineNote = rebaselined > 0 ? `, ${rebaselined} rebaselined` : '' + const rebaselineNote = report.totalRebaselined > 0 ? `, ${report.totalRebaselined} rebaselined` : '' ctx.stdout.write(`maintenance: ${report.totalSnapshotsExpired} snapshots expired, ${report.totalCompacted} partitions compacted${rebaselineNote} (${report.elapsedMs}ms)\n`) return 0 } From abdb17de486bbe65e1856dcab18b6fb9ddde4727 Mon Sep 17 00:00:00 2001 From: test Date: Tue, 11 Aug 2026 00:44:55 +0000 Subject: [PATCH 2/5] Pin PR #706's carried fixes with tests; sharpen two @ref citations Round-1 review of #706 approved the carry but flagged that none of the four stranded fixes was pinned by a committed test, plus two @ref nits. - Add three tests to test/core/cache-retention-maintenance.test.js, in the foreign-sorted-replace block: - `totalRebaselined === 1` on a re-baselining run, and `=== 0` once converged (pins the MaintenanceReport symmetry fix). - a capturing TracerProvider asserting the maintenance.partition span carries `rebaselined: true` (pins the span-attribute fix). - a partition already due for compaction: assert the resettle candidate's data file is read once, not twice, by spying on `fs.readFileSync` (pins the scan-skip fix). hasResettleCandidate's return value is otherwise unobservable once compactionDue is true (it's discarded via `||`), so this is the cheapest honest signal available; each new test was verified to fail when its corresponding fix is reverted. - Retarget the scan-skip gate's @ref from LLP 0207#foreign-replace to #outranks-resettle: the gloss ("recognition ... still outranks it") is that anchor's actual subject, not the recognition test's. - Drop the @ref on the span-attribute comment: #re-baseline settles the cursor-write shape, not telemetry, so citing it was close to mechanical. The prose rationale stays; it's the useful part. - Amend the PR body's claim about item 1: `needsCompaction` (pure, read-only) is now evaluated unconditionally in the re-settle path where the old `||` short-circuited past it, so more moved than "only the scan's side effect is skipped." Co-Authored-By: Claude --- src/core/cache/maintenance.js | 10 +- test/core/cache-retention-maintenance.test.js | 91 +++++++++++++++++++ 2 files changed, 96 insertions(+), 5 deletions(-) diff --git a/src/core/cache/maintenance.js b/src/core/cache/maintenance.js index 4525ceaa1..7080c0fe8 100644 --- a/src/core/cache/maintenance.js +++ b/src/core/cache/maintenance.js @@ -293,7 +293,7 @@ async function maintainGeneration(r, cursor, cfg, opts, settle, snapshotsExpired // line never lands (harness aux, wire-only reminders) - from forcing a // full rewrite every tick, and skips the attributes scan entirely when // nothing new has flushed. - // @ref LLP 0207#foreign-replace [constrained-by]: when the cheap check + // @ref LLP 0207#outranks-resettle [constrained-by]: when the cheap check // above already made compaction due, the scan's answer can never // change the outcome (recognition, tested below, still outranks it), // so skip it: only run the scan when it might be the sole reason to @@ -314,10 +314,10 @@ async function maintainGeneration(r, cursor, cfg, opts, settle, snapshotsExpired // the sorted layout every night. An explicit --force still rewrites. if (!opts.force && foreignSortedReplace(tableInfo)) { r.rebaselined = true - // @ref LLP 0207#re-baseline: the counter proves a rebaseline happened - // at all, but it carries only the dataset; tagging the enclosing - // maintenance.partition span names the partition, so a trace query - // finds which day re-baselined without cross-referencing the counter. + // The counter proves a rebaseline happened at all, but it carries + // only the dataset; tagging the enclosing maintenance.partition span + // names the partition, so a trace query finds which day re-baselined + // without cross-referencing the counter. getActiveSpan()?.setAttribute('rebaselined', true) if (!opts.dryRun) { await writeCursor(r.path, rebaselineCursor(cursor, dataFilesBefore)) diff --git a/test/core/cache-retention-maintenance.test.js b/test/core/cache-retention-maintenance.test.js index c8947519a..9ca203c2c 100644 --- a/test/core/cache-retention-maintenance.test.js +++ b/test/core/cache-retention-maintenance.test.js @@ -3,6 +3,7 @@ import test from 'node:test' import assert from 'node:assert/strict' import fs from 'node:fs/promises' +import fsSync from 'node:fs' import os from 'node:os' import path from 'node:path' @@ -11,11 +12,13 @@ import { maintainCache, cacheStatus, normalizeMaintenanceConfig } from '../../sr import { appendRowsToSourceTable, readCursorSync, writeCursor } from '../../src/core/cache/partition.js' import { appendRowsToTable, currentPartitionSpec, currentSchema, readRowsFromTable, sortColumnsFromMetadata, tableExists } from '../../src/core/cache/iceberg/store.js' import { createLocalIcebergIO, tableUrlForDir } from '../../src/core/cache/iceberg/resolver.js' +import { TracerProvider } from '../../src/core/observability/runtime.js' import { fileCatalog, icebergRewrite, loadLatestFileCatalogMetadata } from 'icebird' /** * @import { ColumnSpec } from '../../hypaware-plugin-kernel-types.js' * @import { CachePartitioningDeclaration } from '../../src/core/cache/types.js' + * @import { Span } from '../../src/core/observability/runtime.js' */ /** @param {string} prefix */ @@ -853,6 +856,7 @@ test('a foreign sorted replace re-baselines the cursor instead of being rewritte // A dry run predicts the recognition without writing anything. const preview = await maintainCache({ cacheRoot, compactOnly: true, dryRun: true }) assert.equal(preview.totalCompacted, 0) + assert.equal(preview.totalRebaselined, 1, 'the report-level rebaseline count mirrors totalCompacted') assert.equal(preview.partitions[0].rebaselined, true) assert.equal( /** @type {{ resettleBaselineFiles: number }} */ (readCursorSync(partDir).compaction).resettleBaselineFiles, @@ -862,6 +866,7 @@ test('a foreign sorted replace re-baselines the cursor instead of being rewritte const first = await maintainCache({ cacheRoot, compactOnly: true }) assert.equal(first.totalCompacted, 0) + assert.equal(first.totalRebaselined, 1, 're-baselining one partition must be reflected in the report total') assert.equal(first.partitions[0].rebaselined, true) const cursor = readCursorSync(partDir) assert.equal(cursor.epoch, 0, 'no rewrite: the generation must not advance') @@ -873,6 +878,7 @@ test('a foreign sorted replace re-baselines the cursor instead of being rewritte // Converged: the baseline gate now blocks before any metadata load. const second = await maintainCache({ cacheRoot, compactOnly: true }) assert.equal(second.totalCompacted, 0) + assert.equal(second.totalRebaselined, 0, 'converged: no rebaseline happened this tick') assert.notEqual(second.partitions[0].rebaselined, true) // A late append flips the current snapshot off `replace` and moves the @@ -998,6 +1004,91 @@ test('force still rewrites a foreign sorted replace', async () => { } }) +test('a foreign sorted replace tags the maintenance.partition span with rebaselined', async () => { + const cacheRoot = await makeTmpDir('maint-foreign-span') + /** @type {Span[]} */ + const captured = [] + const provider = new TracerProvider({ + resource: { attributes: {} }, + exporters: [{ exportBatch(/** @type {Span[]} */ spans) { captured.push(...spans) } }], + }) + provider.register() + try { + const partDir = path.join(cacheRoot, 'datasets', 'ds1', 'date=2026-08-08') + const epoch0 = path.join(partDir, 'epoch=0') + for (let i = 0; i < 3; i++) { + await appendRowsToTable(epoch0, COLUMNS, [ + { id: i, value: `v${i}`, timestamp: new Date().toISOString() }, + ], { sortOrder: [{ column: 'id', direction: 'asc' }] }) + } + await commitForeignReplace(epoch0) + await writeCursor(partDir, { + epoch: 0, + rowCount: 3, + layout: 'epoch', + compaction: { compactedAt: '2026-08-08T00:00:00.000Z', resettleBaselineFiles: 99 }, + }) + + const report = await maintainCache({ cacheRoot, compactOnly: true }) + assert.equal(report.partitions[0].rebaselined, true, 'sanity: this tick recognized the foreign replace') + + const partitionSpan = captured.find((span) => span.name === 'maintenance.partition') + assert.ok(partitionSpan, 'maintenance.partition span must be exported') + assert.equal( + partitionSpan?.attributes.rebaselined, + true, + 'the span, not just the hyp_rebaselines counter, must name which partition re-baselined' + ) + } finally { + await provider.shutdown() + await fs.rm(cacheRoot, { recursive: true, force: true }) + } +}) + +test('a partition already due for compaction skips the resettle-candidate row scan', async (t) => { + // @ref LLP 0207#outranks-resettle [tests]: once the cheap file-count/size + // check alone makes compaction due, the resettle scan's answer cannot + // change `shouldCompact`, so it must not run at all. Observed here by + // counting `readFileSync` calls against the partition's one data file: + // unskipped, the resettle scan and the compaction rewrite each open it + // once (two reads); skipped, only the rewrite does (one read). + const cacheRoot = await makeTmpDir('maint-scan-skip') + try { + const partDir = path.join(cacheRoot, 'datasets', 'ds1', 'date=2026-08-08') + const epoch0 = path.join(partDir, 'epoch=0') + await appendRowsToTable(epoch0, COLUMNS, [ + { id: 1, value: 'v1', timestamp: new Date().toISOString() }, + ]) + // Never compacted: `grewSinceCompaction` is true unconditionally, so the + // only thing standing between the old code and a resettle scan is the + // `compactionDue` gate under test. + await writeCursor(partDir, { epoch: 0, rowCount: 1, compaction: null, layout: 'epoch' }) + + const spy = t.mock.method(fsSync, 'readFileSync') + + const report = await maintainCache({ + cacheRoot, + compactOnly: true, + // compact_file_count: 0 makes `needsCompaction` (and so + // `compactionDue`) true on file count alone, with no size heuristic + // involved: dueness is settled before the resettle scan would run. + config: { compact_file_count: 0 }, + storage: /** @type {any} */ ({}), + getSettleHook: () => async (rows) => rows, + }) + assert.equal(report.totalCompacted, 1, 'sanity: compaction actually ran') + + const dataFileReads = spy.mock.calls.filter((call) => String(call.arguments[0]).endsWith('.parquet')).length + assert.equal( + dataFileReads, + 1, + 'the data file must be read once, by the rewrite; a resettle scan would read it a second time first' + ) + } finally { + await fs.rm(cacheRoot, { recursive: true, force: true }) + } +}) + test('maintenance walks partitions neediest-first, not directory order', async () => { const cacheRoot = await makeTmpDir('maint-order') try { From 47fd4a4e97380c11969dca07ffac1996027ed107 Mon Sep 17 00:00:00 2001 From: test Date: Tue, 11 Aug 2026 01:49:10 +0000 Subject: [PATCH 3/5] Sharpen scan-skip test to stack attribution; fix PR #706 body citations - test/core/cache-retention-maintenance.test.js: the resettle-scan-skip test asserted a global .parquet readFileSync count of 1 for the whole maintainCache tick, not just reads the gate governs. Any future second legitimate read elsewhere in the tick would break it with a message that misdirects the next reader. Switch to capturing a stack trace per .parquet read and asserting none pass through hasResettleCandidate, attributing each read to its caller instead of counting tick-wide. Verified both directions: passes at this head (3x, no flake), and fails with the expected message when the !compactionDue && gate in src/core/cache/maintenance.js is reverted, showing hasResettleCandidate in the offending stack. - PR body: item 2's exemplar list cited llp/0012:9, 0017:9, 0036:9, 0041:8, 0191:9 as using the linked-and-glossed Extended-by form, but only 0191 actually carries a link; the other four are gloss-only. Replaced with docs confirmed to use the linked form: llp/0106:9, 0129:9, 0158:9, 0180:9, 0182:9, 0188:9, 0190:9, 0191:9. (0201, also suggested, only has a backward "Extends" pointer, not "Extended-by", so it was excluded.) Co-Authored-By: Claude --- test/core/cache-retention-maintenance.test.js | 31 +++++++++++++------ 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/test/core/cache-retention-maintenance.test.js b/test/core/cache-retention-maintenance.test.js index 9ca203c2c..f55fa62de 100644 --- a/test/core/cache-retention-maintenance.test.js +++ b/test/core/cache-retention-maintenance.test.js @@ -1048,10 +1048,15 @@ test('a foreign sorted replace tags the maintenance.partition span with rebaseli test('a partition already due for compaction skips the resettle-candidate row scan', async (t) => { // @ref LLP 0207#outranks-resettle [tests]: once the cheap file-count/size // check alone makes compaction due, the resettle scan's answer cannot - // change `shouldCompact`, so it must not run at all. Observed here by - // counting `readFileSync` calls against the partition's one data file: - // unskipped, the resettle scan and the compaction rewrite each open it - // once (two reads); skipped, only the rewrite does (one read). + // change `shouldCompact`, so it must not run at all. `hasResettleCandidate` + // is module-private and its `scanRowsFromTable` is an unpatchable ESM named + // import, so there is no direct call-count hook to assert against; instead + // this mocks `readFileSync` and captures a stack trace per `.parquet` read, + // then asserts none of those stacks pass through `hasResettleCandidate`. + // That frame name survives the async boundary between the scan and the + // read, so it attributes each read to its caller instead of only counting + // reads tick-wide (a second legitimate read elsewhere in the tick, e.g. a + // footer-stats probe, would not falsely implicate the scan). const cacheRoot = await makeTmpDir('maint-scan-skip') try { const partDir = path.join(cacheRoot, 'datasets', 'ds1', 'date=2026-08-08') @@ -1064,7 +1069,13 @@ test('a partition already due for compaction skips the resettle-candidate row sc // `compactionDue` gate under test. await writeCursor(partDir, { epoch: 0, rowCount: 1, compaction: null, layout: 'epoch' }) - const spy = t.mock.method(fsSync, 'readFileSync') + /** @type {string[]} */ + const stacks = [] + const original = fsSync.readFileSync + t.mock.method(fsSync, 'readFileSync', function (p, ...rest) { + if (String(p).endsWith('.parquet')) stacks.push(new Error().stack ?? '') + return original.call(this, p, ...rest) + }) const report = await maintainCache({ cacheRoot, @@ -1078,11 +1089,11 @@ test('a partition already due for compaction skips the resettle-candidate row sc }) assert.equal(report.totalCompacted, 1, 'sanity: compaction actually ran') - const dataFileReads = spy.mock.calls.filter((call) => String(call.arguments[0]).endsWith('.parquet')).length - assert.equal( - dataFileReads, - 1, - 'the data file must be read once, by the rewrite; a resettle scan would read it a second time first' + assert.ok(stacks.length > 0, 'sanity: the data file was read at all') + assert.deepEqual( + stacks.filter((s) => s.includes('hasResettleCandidate')), + [], + 'the resettle scan must not read the data file' ) } finally { await fs.rm(cacheRoot, { recursive: true, force: true }) From 4045b6325045aef88b1eb83417c4d5e3342f4a7c Mon Sep 17 00:00:00 2001 From: test Date: Tue, 11 Aug 2026 02:28:34 +0000 Subject: [PATCH 4/5] Scan-skip test goes blind if the call chain grows 3 frames (#707) `a partition already due for compaction skips the resettle-candidate row scan` attributes each `.parquet` read to its caller by capturing `new Error().stack` inside a `readFileSync` mock and asserting no stack passes through `hasResettleCandidate`. The discriminating frame sits at frame 8 of 10, and `Error.stackTraceLimit` defaults to 10. Three more frames anywhere between the mock and the caller (an icebird refactor, extra node:test mock internals, a wrapper in `resolver.js`) drop that frame, and a negative "no stack mentions X" assertion then passes vacuously: the test stays green even with the `!compactionDue &&` gate reverted. The `stacks.length > 0` sanity check only proved a read happened, not that the stacks were attributable, and the `new Error().stack ?? ''` fallback would store an unattributable empty string without complaint. Demonstrated by running with `Error.stackTraceLimit = 7` (standing in for the three extra frames) and the gate reverted: the old test passed. Two guards, because they cover different routes to the same blindness: 1. Raise `Error.stackTraceLimit` to 50 while the mock is installed, restoring the previous value in the existing `finally` so it is put back even if the test throws. This removes the practical hazard. 2. Assert positively that some captured stack names `compactGeneration`, the legitimate reader. It calls `scanRowsFromTable` from exactly the same depth as `hasResettleCandidate` does, so any truncation deep enough to hide the frame the negative assertion hunts for also hides this one and fails the test loudly. Asserting on `scanRowsFromTable` itself would not work: it sits one frame shallower and survives truncation that has already blinded the real check. This guard also catches the empty-string fallback. Under the same truncation with the gate still reverted, the hardened test fails; with the gate restored it passes, truncated or not. No production code changed. Co-Authored-By: Claude --- test/core/cache-retention-maintenance.test.js | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test/core/cache-retention-maintenance.test.js b/test/core/cache-retention-maintenance.test.js index f55fa62de..9b45b1a0c 100644 --- a/test/core/cache-retention-maintenance.test.js +++ b/test/core/cache-retention-maintenance.test.js @@ -1057,7 +1057,27 @@ test('a partition already due for compaction skips the resettle-candidate row sc // read, so it attributes each read to its caller instead of only counting // reads tick-wide (a second legitimate read elsewhere in the tick, e.g. a // footer-stats probe, would not falsely implicate the scan). + // + // A stack-based observation can go blind: the deciding frame sits ~8 frames + // below the mock, and V8's default `Error.stackTraceLimit` of 10 leaves only + // two frames of headroom. Three more frames anywhere between the mock and + // the caller (an icebird refactor, extra node:test mock internals, a wrapper + // in `resolver.js`) would drop it, and a negative "no stack mentions + // `hasResettleCandidate`" assertion passes vacuously on truncated stacks. + // Two guards keep that from happening silently: + // 1. raise `Error.stackTraceLimit` while the mock is installed (restored + // below even if the test throws), which removes the hazard outright; + // 2. assert positively that some stack names `compactGeneration`, the + // legitimate reader. It calls `scanRowsFromTable` from exactly the same + // depth as `hasResettleCandidate` does, so any truncation deep enough + // to hide the frame the negative assertion hunts for also hides this + // one, and the test fails loudly instead of going quiet. Asserting on + // `scanRowsFromTable` itself would not do: it sits one frame shallower + // and survives truncation that has already blinded the real check. + // Guard 2 also catches the `?? ''` fallback below storing an unattributable + // empty string. const cacheRoot = await makeTmpDir('maint-scan-skip') + const originalStackTraceLimit = Error.stackTraceLimit try { const partDir = path.join(cacheRoot, 'datasets', 'ds1', 'date=2026-08-08') const epoch0 = path.join(partDir, 'epoch=0') @@ -1072,6 +1092,7 @@ test('a partition already due for compaction skips the resettle-candidate row sc /** @type {string[]} */ const stacks = [] const original = fsSync.readFileSync + Error.stackTraceLimit = 50 t.mock.method(fsSync, 'readFileSync', function (p, ...rest) { if (String(p).endsWith('.parquet')) stacks.push(new Error().stack ?? '') return original.call(this, p, ...rest) @@ -1090,12 +1111,17 @@ test('a partition already due for compaction skips the resettle-candidate row sc assert.equal(report.totalCompacted, 1, 'sanity: compaction actually ran') assert.ok(stacks.length > 0, 'sanity: the data file was read at all') + assert.ok( + stacks.some((s) => s.includes('compactGeneration')), + 'sanity: captured stacks must be deep enough to name the reader, or the assertion below passes vacuously' + ) assert.deepEqual( stacks.filter((s) => s.includes('hasResettleCandidate')), [], 'the resettle scan must not read the data file' ) } finally { + Error.stackTraceLimit = originalStackTraceLimit await fs.rm(cacheRoot, { recursive: true, force: true }) } }) From 8e9a2b7828ed4af62f3025c7de5e42a767380610 Mon Sep 17 00:00:00 2001 From: neutral Date: Tue, 11 Aug 2026 18:11:27 +0000 Subject: [PATCH 5/5] LLP 0199: keep master's landed 0207 gloss, add only the link The conflict resolution replaced the 0207 gloss that landed on master with this branch's alternative wording. Both describe LLP 0207 accurately, but rewording a gloss already on an Accepted doc is a content edit, where adding the link is the permitted mechanical one. Keeping master's text also matches how PR #706 resolved the same line, so the two open PRs no longer diverge and whichever merges second will not re-conflict here. --- llp/0199-maintenance-compaction-convergence.decision.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llp/0199-maintenance-compaction-convergence.decision.md b/llp/0199-maintenance-compaction-convergence.decision.md index f416ab641..d23d855ce 100644 --- a/llp/0199-maintenance-compaction-convergence.decision.md +++ b/llp/0199-maintenance-compaction-convergence.decision.md @@ -6,7 +6,7 @@ **Author:** Kenny / Claude **Date:** 2026-08-07 **Related:** LLP 0027 -**Extended-by:** [LLP 0207](./0207-foreign-sorted-replace-convergence.decision.md) (a second convergence source: a foreign sorted `replace` re-baselines the gate instead of triggering a rewrite); [LLP 0209](./0209-compaction-file-size.decision.md) (the rewrite the baseline gate protects now sizes its output files by bytes written rather than by the in-memory batch estimate, so a compacted generation actually reaches `target_file_bytes`) +**Extended-by:** [LLP 0207](./0207-foreign-sorted-replace-convergence.decision.md) (a baseline mismatch whose current snapshot is a sorted `replace` is a foreign rewrite, not growth: recognize it and re-baseline instead of compacting); [LLP 0209](./0209-compaction-file-size.decision.md) (the rewrite the baseline gate protects now sizes its output files by bytes written rather than by the in-memory batch estimate, so a compacted generation actually reaches `target_file_bytes`) > Maintenance stops re-flagging already-compacted partitions: a partition is > only compaction-due when its live data-file count has moved off the count