From 65e484403bf597ae5c7c469d31d29898d256cded Mon Sep 17 00:00:00 2001 From: test Date: Thu, 6 Aug 2026 22:39:31 +0000 Subject: [PATCH] Overview planner charges for the sections it will actually run (#665) `rowsAffordable` divided the time budget by `OVERVIEW_SECTIONS.length`, always four, while `collectOverview` runs only the sections it was asked for. A subset caller was therefore charged for work nobody would do and got a window narrowed in proportion, with nothing to show for it: a window bound by time reports no reason, so the shortfall is invisible. LLP 0135 #window already states the divisor as the sections planned. `missingSections` had the same seam in reverse: it filtered the full section list, so a subset run reported the sections nobody asked for as sections that did not finish. The wizard prints that list verbatim ("the repos and tools sections did not finish"), which would have been a false claim about work never started. `collectOverview` now stamps the plan on the result, before any await, so an abandoned run still carries what it meant to do. Co-Authored-By: Claude --- src/core/query/overview.js | 54 +++++++++++++++++---- src/core/query/types.d.ts | 8 ++++ test/core/query-overview.test.js | 81 ++++++++++++++++++++++++++++++++ 3 files changed, 135 insertions(+), 8 deletions(-) diff --git a/src/core/query/overview.js b/src/core/query/overview.js index e4ad99d9..e873aa90 100644 --- a/src/core/query/overview.js +++ b/src/core/query/overview.js @@ -222,16 +222,37 @@ const SECTION_COST_VS_PROBE = 1.9 * Without a probe timing there is nothing to infer from, so the caller * falls back to the row cap alone (`Infinity` here defers to it). * - * @param {{ budgetMs?: number, probeMs?: number, totalRows: number }} args + * `sections` is what is about to run, not the list of sections that exist. + * `collectOverview` executes only the sections it was asked for, so + * charging a two-section caller for four halves the window it gets in + * exchange for work nobody will do - and the narrowing is invisible to it, + * since a window bound by time reports no reason. LLP 0135 #window puts the + * divisor as `perRowMs x 1.9 x sections`, which is the count planned. + * + * An empty section list divides by zero and yields `Infinity`, which is the + * honest answer: no sections means no section cost, and the row cap decides + * a window nothing will scan. + * + * @ref LLP 0135#window [implements]: the cost model's section term is the sections being planned for + * @param {{ + * budgetMs?: number, + * probeMs?: number, + * totalRows: number, + * sections?: readonly ('models'|'daily'|'repos'|'tools')[], + * }} args * @returns {number} */ -function rowsAffordable({ budgetMs = OVERVIEW_TIME_BUDGET_MS, probeMs, totalRows }) { +function rowsAffordable({ + budgetMs = OVERVIEW_TIME_BUDGET_MS, + probeMs, + totalRows, + sections = OVERVIEW_SECTIONS, +}) { if (probeMs === undefined || totalRows <= 0) return Infinity // A probe too fast to time is not evidence of infinite speed; floor it at // 1ms so the estimate stays finite and the row cap keeps its say. const perRowMs = Math.max(probeMs, 1) / totalRows - const sectionCount = OVERVIEW_SECTIONS.length - return Math.floor(budgetMs / (perRowMs * SECTION_COST_VS_PROBE * sectionCount)) + return Math.floor(budgetMs / (perRowMs * SECTION_COST_VS_PROBE * sections.length)) } /** Rows shown per section before the remainder is folded into a count line. */ @@ -367,9 +388,11 @@ export function overviewRunnerFromCtx(ctx, onNotice, opts = {}) { * days?: number, * budgetMs?: number, * probeMs?: number, + * sections?: readonly ('models'|'daily'|'repos'|'tools')[], * }} [opts] `days` pins an explicit window (the user asked for it) and * skips both caps; `probeMs` is how long the probe took over every row, - * which calibrates the time cap to this machine + * which calibrates the time cap to this machine; `sections` is the work + * the time cap is being asked to pay for, defaulting to all four * @returns {OverviewWindow | null} null when nothing has been recorded * @ref LLP 0135#window [implements]: the affordable-window plan, measured on the machine it runs on */ @@ -392,6 +415,7 @@ export function chooseOverviewWindow(probeRows, opts = {}) { const timeCap = rowsAffordable({ budgetMs: sectionBudgetMs, ...(opts.probeMs !== undefined ? { probeMs: opts.probeMs } : {}), + ...(opts.sections !== undefined ? { sections: opts.sections } : {}), totalRows, }) const cap = Math.min(rowCap, timeCap) @@ -444,9 +468,18 @@ export function hasRenderableOverview(rows) { } /** - * Which of the requested sections have landed. Lets a caller that stopped - * early say what is missing rather than presenting a short block as whole. + * Which of the requested sections have not landed. Lets a caller that + * stopped early say what is missing rather than presenting a short block as + * whole. + * + * Requested, not existing: the wizard prints this list as "the repos and + * tools sections did not finish", and a section nobody asked for did not + * fail to finish - it was never started. Naming it would be the same false + * claim as calling an unfinished section empty, in the other direction. + * `collectOverview` stamps the plan on the result, so a subset run carries + * what it meant to do; a result assembled by hand falls back to all four. * + * @ref LLP 0135#overrun [implements]: unfinished sections are named as unfinished, which only the requested ones can be * @param {OverviewRows} rows * @returns {('models'|'daily'|'repos'|'tools')[]} */ @@ -458,7 +491,7 @@ export function missingSections(rows) { repos: rows.repoRows, tools: rows.toolRows, } - return OVERVIEW_SECTIONS.filter((s) => byName[s].length === 0) + return (rows.sections ?? OVERVIEW_SECTIONS).filter((s) => byName[s].length === 0) } /** @@ -492,6 +525,10 @@ export async function collectOverview(runner, opts = {}) { const sections = opts.sections ?? OVERVIEW_SECTIONS const clock = opts.clock ?? Date.now const out = opts.into ?? emptyOverview() + // Recorded on the result, before any await, because after an abandoned + // run the result object is all the caller still holds - and `missing` + // means "asked for and did not land", which needs the plan to read. + out.sections = sections // Timing the probe is what makes the plan an observation of this machine // rather than an assumption about it: the probe reads every row, so its @@ -502,6 +539,7 @@ export async function collectOverview(runner, opts = {}) { const probeMs = Math.max(0, clock() - probeStart) const window = chooseOverviewWindow(probe.rows, { probeMs, + sections, ...(opts.targetRows !== undefined ? { targetRows: opts.targetRows } : {}), ...(opts.budgetMs !== undefined ? { budgetMs: opts.budgetMs } : {}), ...(opts.days !== undefined ? { days: opts.days } : {}), diff --git a/src/core/query/types.d.ts b/src/core/query/types.d.ts index 68b9869c..2a10eea1 100644 --- a/src/core/query/types.d.ts +++ b/src/core/query/types.d.ts @@ -156,6 +156,14 @@ export interface OverviewRows { toolRows: Record[] window?: OverviewWindow sql?: { models: string; daily: string; repos: string; tools: string } + /** + * The sections this run set out to fill, stamped by `collectOverview` + * before it runs anything. It is what tells an empty section that was + * never requested from one that did not finish, which is the difference + * between the two sentences LLP 0135 #overrun insists on. Absent on a + * result nobody collected into, where all four are assumed. + */ + sections?: readonly ('models' | 'daily' | 'repos' | 'tools')[] } /** diff --git a/test/core/query-overview.test.js b/test/core/query-overview.test.js index 2a3172ad..a138badf 100644 --- a/test/core/query-overview.test.js +++ b/test/core/query-overview.test.js @@ -15,7 +15,9 @@ import { chooseOverviewWindow, collectOverview, describeWindow, + emptyOverview, formatCount, + missingSections, overviewRunnerFromCtx, renderDailyActivity, renderOverview, @@ -524,6 +526,35 @@ test('chooseOverviewWindow: with no probe timing, the row cap decides alone', () assert.equal(win?.boundBy, 'rows') }) +/** 20 days x 10k rows, newest first, on dates that sort as strings. */ +function twentyDayProbe() { + return Array.from({ length: 20 }, (_, i) => ({ + date: `2026-07-${String(20 - i).padStart(2, '0')}`, + n: 10_000, + })) +} + +test('chooseOverviewWindow: the plan charges for the sections it will run, not all four', () => { + // LLP 0135 #window states the estimate as `remaining / (perRowMs x 1.9 x + // sections)`, and `collectOverview` runs only the sections it was asked + // for. Half the work is half the cost, so a two-section caller can afford + // roughly twice the rows on the same machine and the same budget. + const probe = twentyDayProbe() + const opts = { targetRows: 1e9, budgetMs: 5000, probeMs: 2000 } + + const four = chooseOverviewWindow(probe, opts) + const two = chooseOverviewWindow(probe, { ...opts, sections: ['models', 'daily'] }) + + assert.ok(four && two) + assert.equal(four.boundBy, 'time') + assert.equal(two.boundBy, 'time') + // 3000ms left after the probe, at 0.01ms/row: 39,473 rows for four + // sections, 78,947 for two - three days against seven. + assert.equal(four.days, 3) + assert.equal(two.days, 7) + assert.ok(two.rows > four.rows * 2, `${two.rows} should be over twice ${four.rows}`) +}) + test('chooseOverviewWindow: an explicit --days request outranks both caps', () => { const probe = Array.from({ length: 10 }, (_, i) => ({ date: `2026-07-${20 - i}`, n: 1_000_000 })) // Row cap tiny, machine measured as glacial: the user asked anyway. @@ -603,6 +634,56 @@ test('collectOverview: probes first, then runs only the requested sections', asy assert.deepEqual(rows.toolRows, []) }) +/** A runner that answers the probe with `probe` and every section one row. */ +function answeringRunner(probe) { + return { + hasDataset: () => true, + /** @param {string} sql */ + async run(sql) { + return { columns: [], rows: sql === OVERVIEW_PROBE_SQL ? probe : [{ n: 1 }] } + }, + } +} + +/** A clock whose second reading is `ms` later, so the probe times exactly. */ +function clockSpending(ms) { + let calls = 0 + return () => (calls++ === 0 ? 0 : ms) +} + +test('collectOverview: the window is planned for the sections actually requested', async () => { + // The same machine, the same budget, the same cache - only the section + // list differs, and the subset caller is not charged for work it will + // never do. + const probe = twentyDayProbe() + const four = await collectOverview(answeringRunner(probe), { + targetRows: 1e9, + clock: clockSpending(2000), + }) + const two = await collectOverview(answeringRunner(probe), { + sections: ['models', 'daily'], + targetRows: 1e9, + clock: clockSpending(2000), + }) + assert.equal(four.window?.days, 3) + assert.equal(two.window?.days, 7) +}) + +test('missingSections: a section nobody asked for is not reported as unfinished', async () => { + // "The repos section did not finish" and "you did not ask for repos" are + // different claims, and the wizard prints the first one verbatim + // (LLP 0135 #overrun). A subset run has nothing missing. + const probe = [{ date: '2026-07-24', n: 10 }] + const two = await collectOverview(answeringRunner(probe), { sections: ['models', 'daily'] }) + assert.deepEqual(missingSections(two), []) + + // And a section that was asked for and did not land is still named. + const { runner } = probingRunner(probe) + const none = await collectOverview(runner, { sections: ['models', 'repos'] }) + assert.deepEqual(missingSections(none), ['models', 'repos']) + assert.deepEqual(missingSections(emptyOverview()), ['models', 'daily', 'repos', 'tools']) +}) + test('collectOverview: runs all four sections by default, in display order', async () => { const { seen, runner } = probingRunner([{ date: '2026-07-24', n: 10 }]) const rows = await collectOverview(runner)