diff --git a/changelog.d/1222-daily-metrics.added.md b/changelog.d/1222-daily-metrics.added.md new file mode 100644 index 000000000..7f6ebcdce --- /dev/null +++ b/changelog.d/1222-daily-metrics.added.md @@ -0,0 +1 @@ +The delivery board's numbers now come with the seven days behind them: `metrics.daily` carries one entry per Asia/Shanghai day, oldest first and today last, each with the issues opened and closed, the pull requests opened and merged, and the CI share of the runs that actually counted that day. A day the host could not read is `null` rather than `0`, so a gap stays a gap instead of reading as a quiet day, and a week that could not be read at all leaves the whole series `null` instead of drawing seven zeros. The seven-day CI share is read over the same week its own row covers — it used to be a week out of step, because the CI windows were walked backwards from the start of the week. ([#1222](https://github.com/ThinkFlowLab/FlyBot/issues/1222)) diff --git a/core/src/host/delivery-board.js b/core/src/host/delivery-board.js index 4f709892d..0a51f5f8a 100644 --- a/core/src/host/delivery-board.js +++ b/core/src/host/delivery-board.js @@ -36,6 +36,48 @@ export const BACKOFF_MS = [10_000, 30_000, 120_000] export const startOfDay = (now = Date.now()) => Math.floor((now + SHANGHAI_OFFSET_MS) / DAY_MS) * DAY_MS - SHANGHAI_OFFSET_MS +// `YYYY-MM-DD` for a day boundary, read in the zone the day is cut in — the date the owner sees, +// not the UTC one, which is a day behind for eight hours of every Shanghai day (#1222). +export const shanghaiDate = (start) => new Date(start + SHANGHAI_OFFSET_MS).toISOString().slice(0, 10) + +// The seven-day series the chart draws (#1222): one entry per Asia/Shanghai day, oldest first, the +// last one today and marked `partial`. It is assembled from the two windows the round already read +// — the week's seven days and today's own — so it costs no request. A window that could not be +// read, or a day nobody counted, makes the whole series `null`: the view says it has no data +// instead of drawing seven quiet zero days. +export function dailySeries(week, todayMetrics, today) { + const buckets = new Map() + for (const day of [...(week?.daily ?? []), ...(todayMetrics?.daily ?? [])]) buckets.set(day.start, day) + const series = [] + for (let offset = 6; offset >= 0; offset -= 1) { + const start = today - offset * DAY_MS + const bucket = buckets.get(start) + if (bucket === undefined) return null + series.push({ + date: shanghaiDate(start), + ...(offset === 0 ? { partial: true } : {}), + newIssues: bucket.newIssues, + closedIssues: bucket.closedIssues, + newPullRequests: bucket.newPullRequests, + mergedPullRequests: bucket.mergedPullRequests, + ci: bucket.ci, + }) + } + return series +} + +// The per-day buckets are the series' raw material; the payload carries the assembled series only. +const withoutBuckets = (metrics) => { + if (!metrics) return metrics ?? null + const { daily: _buckets, ...rest } = metrics + return rest +} +const withoutCiDays = (ci) => { + if (!ci) return ci ?? null + const { days: _days, ...rest } = ci + return rest +} + export function median(values) { const list = (values ?? []).filter(value => Number.isFinite(value)).sort((a, b) => a - b) if (list.length === 0) return null @@ -276,16 +318,26 @@ export function createDeliveryBoard({ repo, token, request = fetch, now = Date.n return body } - async function pages(path, { max = MAX_PAGES, stop = null } = {}) { + // A walk that ends on its last allowed page with that page full, and with no row the `stop` + // predicate wanted, left a tail nobody read: the caller must say "unknown", not "fewer" + // (#1211, #1222). A full page alone is not that answer — the stop may have matched on it, which + // means the window was read whole and only the list ended (#1240). + async function walk(path, { max = MAX_PAGES, stop = null } = {}) { const rows = [] + let truncated = false for (let page = 1; page <= max; page += 1) { const separator = path.includes('?') ? '&' : '?' const chunk = await get(`${base}${path}${separator}per_page=${PAGE}&page=${page}`) if (!Array.isArray(chunk)) break rows.push(...chunk) if (chunk.length < PAGE || (stop !== null && chunk.some(stop))) break + if (page === max) truncated = true } - return rows + return { rows, truncated } + } + + async function pages(path, options) { + return (await walk(path, options)).rows } // Every page, not only the first (#1189). Past 100 comments — ordinary on this repository @@ -328,37 +380,53 @@ export function createDeliveryBoard({ repo, token, request = fetch, now = Date.n // The CI half of a window, kept apart from the issue and pull counts: `/actions/runs` missing a // permission is one unknown number in the header, not an empty board (#1211). async function ciFor(from, ciDays) { - const ci = { failed: 0, superseded: 0, counted: 0, total: 0, unknownDays: [] } + const ci = { failed: 0, superseded: 0, counted: 0, total: 0, unknownDays: [], days: [] } for (let day = 0; day < ciDays; day += 1) { - const window = windowOf(from - day * DAY_MS) + // Forward from the window's start, the way every other count in this module reads its + // window: walking back from `from` described the week before the one the other numbers + // cover, so a seven-day CI share was a week out of step with its own row (#1222). + const window = windowOf(from + day * DAY_MS) const runs = await runsOfDay(window.from, window.to) if (runs.truncated) { // The tail was not read, so this day's number is unknown: it is named and left out rather // than added as a partial truth (#1211). note('ci', failureOf(`the workflow runs for ${new Date(window.from).toISOString()} were truncated at ${MAX_PAGES * PAGE}`, { kind: 'truncated', scope: 'actions', endpoint: '/actions/runs', status: null })) ci.unknownDays.push(new Date(window.from).toISOString()) + ci.days.push({ start: window.from, unknown: true }) continue } ci.failed += runs.failed ci.superseded += runs.superseded ci.counted += runs.counted ci.total += runs.total + // The per-day block the chart reads (#1222). A cancelled run never judged the code, so it is + // left out of the denominator; the header keeps its own aggregate untouched. + ci.days.push({ start: window.from, failed: runs.failed, counted: Math.max(0, runs.counted - runs.superseded), total: runs.total }) } return ci } - async function metricsFor(from, { days = 1, ciDays = 1 } = {}) { + // A list that stopped at the page cap has a tail nobody read, so the counts built from it are + // unknown rather than smaller numbers (#1222). The flag is the walk's own answer, not the row + // count: a page four that is full because `stop` matched an old row there was read whole (#1240). + const capped = (listing) => listing.truncated === true + + async function metricsFor(from, { days = 1, ciDays = 1, buckets = false } = {}) { const to = from + days * DAY_MS - const [issues, pulls] = await Promise.all([ - pages(`/issues?state=all&since=${new Date(from - DAY_MS).toISOString()}`, { max: 4, stop: row => row.created_at !== undefined && Date.parse(row.created_at) < from - DAY_MS }), - pages('/pulls?state=all&sort=created&direction=desc', { max: 4, stop: row => Date.parse(row.created_at ?? 0) < from }), + const [issueList, pullList] = await Promise.all([ + walk(`/issues?state=all&since=${new Date(from - DAY_MS).toISOString()}`, { max: 4, stop: row => row.created_at !== undefined && Date.parse(row.created_at) < from - DAY_MS }), + walk('/pulls?state=all&sort=created&direction=desc', { max: 4, stop: row => Date.parse(row.created_at ?? 0) < from }), ]) + const issues = issueList.rows + const pulls = pullList.rows const plainIssues = issues.filter(row => !isPull(row)) const inWindow = (value) => { const at = Date.parse(value ?? ''); return Number.isFinite(at) && at >= from && at < to } const newIssues = plainIssues.filter(row => inWindow(row.created_at)).length const closedIssues = plainIssues.filter(row => inWindow(row.closed_at)) const newPrs = pulls.filter(row => inWindow(row.created_at)).length const merged = pulls.filter(row => inWindow(row.merged_at)) + const cappedIssues = capped(issueList) + const cappedPulls = capped(pullList) // A CI window that cannot be read leaves `ci: null` — the header says unknown, never zero. let ci = null if (ciDays > 0) { @@ -367,6 +435,27 @@ export function createDeliveryBoard({ repo, token, request = fetch, now = Date.n note('ci', error) } } + // One bucket per day of the window, for the seven-day series (#1222). A bucket is built from + // the rows this round already read, so it costs no request; a day whose list or run query was + // truncated, or whose CI could not be read at all, says `null` instead of a smaller number. + const dayBuckets = buckets + ? Array.from({ length: days }, (_, index) => { + const start = from + index * DAY_MS + const end = start + DAY_MS + const inDay = (value) => { const at = Date.parse(value ?? ''); return Number.isFinite(at) && at >= start && at < end } + const ciDay = ci?.days?.find(day => day.start === start) ?? null + return { + start, + newIssues: cappedIssues ? null : plainIssues.filter(row => inDay(row.created_at)).length, + closedIssues: cappedIssues ? null : plainIssues.filter(row => inDay(row.closed_at)).length, + newPullRequests: cappedPulls ? null : pulls.filter(row => inDay(row.created_at)).length, + mergedPullRequests: cappedPulls ? null : pulls.filter(row => inDay(row.merged_at)).length, + ci: ciDay === null || ciDay.unknown === true + ? null + : { failed: ciDay.failed, counted: ciDay.counted, total: ciDay.total }, + } + }) + : undefined return { newIssues, closedIssues: closedIssues.length, @@ -379,7 +468,8 @@ export function createDeliveryBoard({ repo, token, request = fetch, now = Date.n mergedPullRequests: merged.length, medianIssueCloseMs: median(closedIssues.map(row => Date.parse(row.closed_at) - Date.parse(row.created_at))), medianPullMergeMs: median(merged.map(row => Date.parse(row.merged_at) - Date.parse(row.created_at))), - ci, + ci: withoutCiDays(ci), + ...(dayBuckets ? { daily: dayBuckets } : {}), } } @@ -451,8 +541,8 @@ export function createDeliveryBoard({ repo, token, request = fetch, now = Date.n ]) // Two windows, settled separately: a week that cannot be read must not take today with it. const metricsResults = await Promise.allSettled([ - metricsFor(today), - metricsFor(weekStart, { days: 7, ciDays: 7 }), + metricsFor(today, { buckets: true }), + metricsFor(weekStart, { days: 7, ciDays: 7, buckets: true }), ]) const settle = (result, name) => { if (result.status === 'fulfilled') return result.value @@ -546,8 +636,9 @@ export function createDeliveryBoard({ repo, token, request = fetch, now = Date.n metrics: { dayStart: new Date(today).toISOString(), timeZone: 'Asia/Shanghai', - today: todayMetrics, - week: weekMetrics, + today: withoutBuckets(todayMetrics), + week: withoutBuckets(weekMetrics), + daily: dailySeries(weekMetrics, todayMetrics, today), open: { issues: issues.length, pullRequests: openPrs, diff --git a/core/test/delivery-board.test.js b/core/test/delivery-board.test.js index 5aba1ffca..17b305712 100644 --- a/core/test/delivery-board.test.js +++ b/core/test/delivery-board.test.js @@ -8,7 +8,7 @@ import assert from 'node:assert/strict' import { test } from 'node:test' import { readFileSync } from 'node:fs' -import { createDeliveryBoard, DELIVERY_COLUMNS, describeResponse, install, median, probeGitHub, startOfDay, BUILD_BUDGET_MS, BACKOFF_MS } from '../src/host/delivery-board.js' +import { createDeliveryBoard, dailySeries, DELIVERY_COLUMNS, describeResponse, install, median, probeGitHub, startOfDay, BUILD_BUDGET_MS, BACKOFF_MS } from '../src/host/delivery-board.js' import { install as installAuthz } from '../src/host/authz.js' import { install as installChannel } from '../src/host/channel.js' @@ -23,7 +23,7 @@ const respond = (body, status = 200, headerMap = {}) => ({ json: async () => body, }) -function fakeGitHub({ comments = {}, checks = {}, closedExtra = [], mergedExtra = [], openPullsExtra = [], failures = {}, hang = false } = {}) { +function fakeGitHub({ comments = {}, checks = {}, closedExtra = [], mergedExtra = [], openPullsExtra = [], failures = {}, hang = false, runsTruncated = false, listsCapped = false, listsStale = false, listsStopLate = false } = {}) { const calls = [] const request = async (url, init = {}) => { const parsed = new URL(url) @@ -48,6 +48,32 @@ function fakeGitHub({ comments = {}, checks = {}, closedExtra = [], mergedExtra { number: 13, title: 'Needs a human', html_url: 'https://x/13', labels: [{ name: 'needs-info' }], created_at: '2026-09-01T00:00:00Z', updated_at: '2026-10-01T00:00:00Z' }, ] : []) } + // The two list calls that feed the counts (#1222), when the page cap is what ends them: + // `listsCapped` answers four full pages of rows dated inside the week the board asks for, so a + // window's walk only stops at the cap; `listsStale` answers full pages of rows older than every + // window, so the walk ends on its first page and nothing was in fact left unread. + // `listsStopLate` is the same full walk with the `stop` row on page four: four full pages, and + // the only old row is the last one, so the walk ends because the window was read whole, not + // because it ran out of pages. + if ((path === '/issues' || path === '/pulls') && parsed.searchParams.get('state') === 'all' && listsStopLate) { + return respond(Array.from({ length: 100 }, (_, index) => ({ + number: 700 + page * 100 + index, + title: 'a row on a full page', + created_at: page === 4 && index === 99 ? '2026-10-01T00:00:00Z' : '2026-10-20T00:00:00Z', + closed_at: null, + ...(path === '/pulls' ? { merged_at: page === 4 && index === 99 ? '2026-10-01T00:00:00Z' : '2026-10-20T01:00:00Z' } : {}), + }))) + } + if ((path === '/issues' || path === '/pulls') && parsed.searchParams.get('state') === 'all' && (listsCapped || listsStale)) { + const created = listsStale ? '2026-10-01T00:00:00Z' : '2026-10-20T00:00:00Z' + return respond(Array.from({ length: 100 }, (_, index) => ({ + number: 900 + page * 100 + index, + title: 'a row on a full page', + created_at: created, + closed_at: null, + ...(path === '/pulls' ? { merged_at: listsStale ? '2026-10-01T01:00:00Z' : '2026-10-20T01:00:00Z' } : {}), + }))) + } if (path === '/pulls' && (parsed.searchParams.get('state') ?? 'open') === 'open') { return respond(page === 1 ? [ { number: 40, title: 'Pending verdict', html_url: 'https://x/40', body: 'Owner: Dev', head: { sha: 'aaaaaaa1' }, mergeable_state: 'unstable', created_at: '2026-10-24T00:00:00Z', updated_at: '2026-10-24T10:00:00Z', labels: [] }, @@ -87,6 +113,10 @@ function fakeGitHub({ comments = {}, checks = {}, closedExtra = [], mergedExtra ] : []) } if (path.startsWith('/actions/runs')) { + // A day whose runs outnumber the page cap: every page is full, so the tail is never read. + if (runsTruncated) { + return respond({ total_count: 1000, workflow_runs: Array.from({ length: 100 }, () => ({ conclusion: 'success' })) }) + } return respond({ total_count: 3, workflow_runs: [ @@ -515,6 +545,171 @@ test('metrics use the Shanghai day boundary, medians, and the CI counted/total c ])) }) +test('metrics.daily is seven Shanghai days ending today, and only today is partial (#1222)', async () => { + const { board, github } = build() + const snapshot = await settle(board) + const daily = snapshot.metrics.daily + // Seven days, oldest first, the last one today: the window the chart draws is the same one the + // header's Today column sits at the end of. + assert.equal(daily.length, 7) + assert.deepEqual([...daily.map(day => day.date)], + ['2026-10-19', '2026-10-20', '2026-10-21', '2026-10-22', '2026-10-23', '2026-10-24', '2026-10-25']) + assert.equal(daily[6].partial, true) + assert.equal(daily.slice(0, 6).some(day => 'partial' in day), false, 'only today is a part of a day') + const byDate = Object.fromEntries(daily.map(day => [day.date, day])) + // A day nobody counted is 0, and 0 is a measurement: 10-22 opened and closed nothing. + assert.deepEqual({ ...byDate['2026-10-22'] }, { + date: '2026-10-22', + newIssues: 0, + closedIssues: 0, + newPullRequests: 1, + mergedPullRequests: 0, + ci: { failed: 1, counted: 2, total: 3 }, + }) + assert.equal(byDate['2026-10-19'].newIssues, 1) + assert.equal(byDate['2026-10-20'].newIssues, 2) + assert.equal(byDate['2026-10-21'].newIssues, 2) + assert.equal(byDate['2026-10-24'].closedIssues, 1) + assert.equal(byDate['2026-10-24'].mergedPullRequests, 1) + // Today is the same day the Today column counts, not a day behind it. + assert.equal(byDate['2026-10-25'].closedIssues, snapshot.metrics.today.closedIssues) + assert.equal(byDate['2026-10-25'].mergedPullRequests, snapshot.metrics.today.mergedPullRequests) + // A cancelled run never judged the code, so it is out of the denominator; the header keeps its + // own aggregate, superseded included. + assert.deepEqual(byDate['2026-10-25'].ci, { failed: 1, counted: 2, total: 3 }) + assert.deepEqual(snapshot.metrics.today.ci, { failed: 1, superseded: 1, counted: 3, total: 3, unknownDays: [] }) + // The series costs no request: it is built from the two windows the round already reads. + assert.ok(github.calls.every(call => call.headers.Authorization === 'Bearer SECRET-TOKEN')) + // The assembled series is the only thing the payload carries — no per-day buckets anywhere else. + assert.equal('daily' in snapshot.metrics.today, false) + assert.equal('daily' in snapshot.metrics.week, false) +}) + +test('the day boundary is Shanghai, not UTC: an evening in UTC is already tomorrow (#1222)', async () => { + // 2026-10-24T17:00Z is 2026-10-25 01:00 in Shanghai, so the newest day is the 25th. + const { board } = build({}, { now: () => AT('2026-10-24T17:00:00Z') }) + const snapshot = await settle(board) + assert.equal(snapshot.metrics.dayStart, '2026-10-24T16:00:00.000Z') + const daily = snapshot.metrics.daily + assert.equal(daily[6].date, '2026-10-25') + assert.equal(daily[6].partial, true) + assert.equal(daily[0].date, '2026-10-19') + assert.equal(daily.length, 7) +}) + +test('the seven-day CI share is read over the week its counts cover (#1222)', async () => { + // The week window is [2026-10-17T16:00Z, 2026-10-24T16:00Z). Walking backwards from its start + // read the seven days *before* it, so the CI row described a week the other numbers never saw. + const { board, github } = build() + await settle(board) + const ranges = github.calls + .filter(call => call.path.startsWith('/actions/runs')) + .map(call => new URLSearchParams(call.query).get('created')) + const starts = ranges.map(range => Date.parse(range.split('..')[0])) + assert.equal(starts.length, 8, 'seven days for the week, one for today') + assert.ok(starts.every(at => at >= AT('2026-10-17T16:00:00Z')), 'no window reaches back before the week it describes') + assert.equal(starts.filter(at => at === AT('2026-10-17T16:00:00Z')).length, 1, 'the first day of the week is read') + assert.equal(starts.filter(at => at === AT('2026-10-23T16:00:00Z')).length, 1, 'the week runs up to the day before today') + assert.equal(starts.filter(at => at === AT('2026-10-24T16:00:00Z')).length, 1, 'today is its own window') +}) + +test('a day whose runs were truncated is unknown, and unknown is not zero (#1222)', async () => { + const { board } = build({ runsTruncated: true }) + const snapshot = await settle(board) + const daily = snapshot.metrics.daily + assert.equal(daily.length, 7) + // The tail of every day was unread, so no day may claim a CI share — including a quiet-looking 0%. + assert.equal(daily.every(day => day.ci === null), true) + // The counts are still numbers: a CI read that failed does not erase them. + assert.equal(daily[0].newIssues, 1) + assert.ok(snapshot.missing.some(item => item.part === 'ci')) +}) + +test('a count built from a list that stopped at the page cap is unknown, not smaller (#1222)', async () => { + // Both lists answer four full pages, so the walk ends on the cap with a tail nobody read. The + // week's window is capped; today's own window stops on its first page because these rows are + // already older than it, so today keeps its measurements and the cap reaches only the six days + // whose list it truncated. + const { board, github } = build({ listsCapped: true }) + const snapshot = await settle(board) + const daily = snapshot.metrics.daily + assert.equal(daily.length, 7) + for (const day of daily.slice(0, 6)) { + assert.equal(day.newIssues, null, `${day.date} sits in a list that stopped at the page cap`) + assert.equal(day.closedIssues, null, `${day.date} has no issue count to show`) + assert.equal(day.newPullRequests, null, `${day.date} has no pull count to show`) + assert.equal(day.mergedPullRequests, null) + } + // Today read its own window whole: an unknown day never costs a day that was measured. + assert.equal(typeof daily[6].newIssues, 'number') + assert.equal(typeof daily[6].closedIssues, 'number') + assert.equal(typeof daily[6].newPullRequests, 'number') + assert.equal(typeof daily[6].mergedPullRequests, 'number') + // The header counts are built from the same lists but are not per-day: the cap nulls the series + // entry, never the number a reader compares across the week. + assert.equal(typeof snapshot.metrics.today.newIssues, 'number') + assert.equal(typeof snapshot.metrics.week.newIssues, 'number') + assert.equal(daily[6].newIssues, snapshot.metrics.today.newIssues) + const issueCalls = github.calls.filter(call => call.path === '/issues' && new URLSearchParams(call.query).get('state') === 'all') + assert.equal(issueCalls.filter(call => new URLSearchParams(call.query).get('page') === '4').length >= 1, true, 'the week really walked into the page cap') +}) + +test('a window that stopped early on an old row is complete, not unknown (#1222)', async () => { + // A full first page of rows older than the window ends the walk with nothing left unread. The + // page-count heuristic reads that as "the list had more to give"; it must not, or a quiet week + // would drift into permanent unknowns. + const { board, github } = build({ listsStale: true }) + const snapshot = await settle(board) + const daily = snapshot.metrics.daily + assert.equal(daily.length, 7) + // Nothing was opened or merged inside the seven days, and that is a measurement: 0, never null. + for (const day of daily) { + assert.equal(day.newIssues, 0, `${day.date} was read whole`) + assert.equal(day.closedIssues, 0) + assert.equal(day.newPullRequests, 0) + assert.equal(day.mergedPullRequests, 0) + } + // One page per window, not four: the walk stopped on the old row rather than at the cap. + const issueCalls = github.calls.filter(call => call.path === '/issues' && new URLSearchParams(call.query).get('state') === 'all') + assert.equal(issueCalls.length, 2, 'week plus today, one page each') + assert.equal(issueCalls.some(call => new URLSearchParams(call.query).get('page') === '2'), false) + assert.equal(snapshot.missing.some(item => item.part === 'issues' || item.part === 'pulls'), false) +}) + +test('a full page four that stopped on an old row is a complete window, not a capped one (#1240)', async () => { + // The page count cannot tell these apart: four full pages either hit the cap or ended because the + // stop row was on page four. Only the walk knows, so the flag has to come from it. + const { board } = build({ listsStopLate: true }) + const snapshot = await settle(board) + const daily = snapshot.metrics.daily + assert.equal(daily.length, 7) + const byDate = Object.fromEntries(daily.map(day => [day.date, day])) + // 300 rows on pages one to three plus 99 on page four; the hundredth is the old row that stopped + // the walk. Every one of them was measured, so none of these days is unknown. + assert.equal(byDate['2026-10-20'].newIssues, 399) + assert.equal(byDate['2026-10-20'].newPullRequests, 399) + assert.equal(daily.every(day => day.newIssues !== null && day.newPullRequests !== null), true) + assert.equal(typeof snapshot.metrics.week.newIssues, 'number') +}) + +test('CI that cannot be read leaves every day at null, never at a zero share (#1222)', async () => { + const { board } = build({ failures: { '/actions/runs': { status: 403, headers: { 'retry-after': '30' } } } }) + const snapshot = await settle(board) + const daily = snapshot.metrics.daily + assert.equal(daily.every(day => day.ci === null), true) + assert.equal(daily[6].closedIssues, 1) +}) + +test('no week, no series: a window nobody read is null, not seven zero days (#1222)', () => { + // The series is assembled from the two windows the round read; without the week there is nothing + // to draw, and the view says so instead of drawing a flat line at zero. + assert.equal(dailySeries(null, null, AT('2026-10-24T16:00:00Z')), null) + const today = AT('2026-10-24T16:00:00Z') + const only = { daily: [{ start: today, newIssues: 1, closedIssues: 0, newPullRequests: 0, mergedPullRequests: 0, ci: null }] } + assert.equal(dailySeries(null, only, today), null, 'six days without a bucket are six unknown days') + assert.equal(dailySeries({ daily: [] }, only, today), null) +}) + test('a snapshot is cached, a stale one is served while it refreshes, and 304 reuses it (#1173)', async () => { let clock = NOW const github = fakeGitHub() diff --git a/docs/subsystems/core-host.md b/docs/subsystems/core-host.md index 610804992..f919eba65 100644 --- a/docs/subsystems/core-host.md +++ b/docs/subsystems/core-host.md @@ -38,7 +38,7 @@ The server side of the patched `ds-bot` core (`core/`): the part of [ds-bot-core | --- | --- | | `core/index.js` | Server entry. | | `core/src/host/` | Host-side helpers (non-hooks). | -| `core/src/host/delivery-board.js` | The delivery board (#1173): one read-only snapshot of the repository's GitHub issues, pull requests and CI runs, laid out by delivery stage. Every request is a `GET`; the token is read host-side (`rt.secretValue`, falling back to none) and never leaves the payload, which carries no credential. `read({ force })` caches per URL with the ETag and revalidates with `If-None-Match`, serving the old snapshot on a 304 and while a refresh runs; there is no timer — the client asks. Issues are placed by label (`agent-working` → working, `agent-ready` → ready, else original), pull requests in review, and pull requests merged, and issues or pull requests closed, since the day start in done today — a pull request closed without merging counts, so finished work never sits in no column. A PR card carries the verdict badge of a recent `Verdict:` on its own tip (an old-tip verdict does not count; CI decides) — every comment page is read, so a verdict past the first hundred comments still decides — and an issue is linked to a pull request only when a closing keyword (`close`/`fix`/`resolve` with `#n`) names it, never because a body merely mentions the number (#1189), and the header reports the Asia/Shanghai day, the 7-day window and CI as `counted` of `total`. Endpoint `delivery-board`, allowed to any signed-in member because it reads one repository rather than an owner's Bots. #1211: a read never reports a blank board for a problem it can name. The first read answers `{ state: 'loading', since, attempt }` at once and the round runs in the background on the same `inflight`, because a request parked on a whole round is cut by the reverse proxy before it answers. Then `fresh`, `stale` (an older snapshot kept, with `error: { message, endpoint, status, at }` carried into the next read rather than dropped to `null`) and `partial` (a round past `BUILD_BUDGET_MS` — 90 s — lands with the columns it has and the metrics it read). Degradation comes before retry: an unreadable `/actions/runs` makes CI `null` and the row says `unknown`, never `0`; an unreadable check-runs writes `checks-unknown`; unreadable comments leave that card's owner blank; only the two open tables (`pulls`, `issues`) fail a round, because without them there are no columns. A failed round backs off `10 s → 30 s → 2 min` (`BACKOFF_MS`) unless the client forces a read, and a 403 names `retry-after`, else `max(0, x-ratelimit-reset - now)`, and no number at all when neither is present. `probeGitHub()` walks five endpoints and returns status plus the setting to fix; `probe()` and the `delivery-probe` endpoint expose it, and `core/scripts/delivery-probe.mjs` is a thin shell over the same function. Tests: `core/test/delivery-board.test.js`. | +| `core/src/host/delivery-board.js` | The delivery board (#1173): one read-only snapshot of the repository's GitHub issues, pull requests and CI runs, laid out by delivery stage. Every request is a `GET`; the token is read host-side (`rt.secretValue`, falling back to none) and never leaves the payload, which carries no credential. `read({ force })` caches per URL with the ETag and revalidates with `If-None-Match`, serving the old snapshot on a 304 and while a refresh runs; there is no timer — the client asks. Issues are placed by label (`agent-working` → working, `agent-ready` → ready, else original), pull requests in review, and pull requests merged, and issues or pull requests closed, since the day start in done today — a pull request closed without merging counts, so finished work never sits in no column. A PR card carries the verdict badge of a recent `Verdict:` on its own tip (an old-tip verdict does not count; CI decides) — every comment page is read, so a verdict past the first hundred comments still decides — and an issue is linked to a pull request only when a closing keyword (`close`/`fix`/`resolve` with `#n`) names it, never because a body merely mentions the number (#1189), and the header reports the Asia/Shanghai day, the 7-day window and CI as `counted` of `total`. The payload also carries `metrics.daily` (#1222): the last seven Asia/Shanghai days, oldest first and today last with `partial: true`, each with the issues and pull requests opened, closed and merged and one `{ failed, counted, total }` CI block per day — a cancelled run is left out of the denominator. A day whose list or runs were truncated, or whose CI could not be read, is `null` and never `0`; a week that could not be read makes the whole series `null`, so the view says it has no data instead of drawing seven zeros. The series costs no request: it is assembled from the week and today windows the round already reads. Endpoint `delivery-board`, allowed to any signed-in member because it reads one repository rather than an owner's Bots. #1211: a read never reports a blank board for a problem it can name. The first read answers `{ state: 'loading', since, attempt }` at once and the round runs in the background on the same `inflight`, because a request parked on a whole round is cut by the reverse proxy before it answers. Then `fresh`, `stale` (an older snapshot kept, with `error: { message, endpoint, status, at }` carried into the next read rather than dropped to `null`) and `partial` (a round past `BUILD_BUDGET_MS` — 90 s — lands with the columns it has and the metrics it read). Degradation comes before retry: an unreadable `/actions/runs` makes CI `null` and the row says `unknown`, never `0`; an unreadable check-runs writes `checks-unknown`; unreadable comments leave that card's owner blank; only the two open tables (`pulls`, `issues`) fail a round, because without them there are no columns. A failed round backs off `10 s → 30 s → 2 min` (`BACKOFF_MS`) unless the client forces a read, and a 403 names `retry-after`, else `max(0, x-ratelimit-reset - now)`, and no number at all when neither is present. `probeGitHub()` walks five endpoints and returns status plus the setting to fix; `probe()` and the `delivery-probe` endpoint expose it, and `core/scripts/delivery-probe.mjs` is a thin shell over the same function. Tests: `core/test/delivery-board.test.js`. | | `core/src/host/prompt.js` | The Bot system prompt; a Main Bot's section is headed `You are the Main Bot` and uses that one term, the one the UI shows (#276). The team rules state the idle default (#699): a Bot does nothing until the user addresses it, it is @-mentioned in a group, its parent gives it a task, or unfinished tasks it was already given are still on its board. It does not invent work. A coordinator acts on a user task rather than on its own initiative — an addressed Bot answers as before. | | `core/src/host/room-membership.js` | Typed room participants (human / local bot / granted remote), authorship, membership-window history (#50); rooms with friends' Bots or other owners take granted local Bots only and evict a Bot that stops qualifying (#90). Face-to-face rooms (#83) reuse this model (`origin: 'face-to-face'`, room-only remotes without a person-grant, `upsertFaceRoom` / `syncFaceParticipants` / `applyFaceRoomLine`); a face-room local Bot is `viaCode` (brought by number), `evictUnshared` skips face rooms (membership is the server's), and a Bot that returns to the member list drops the empty local `left` line (#236). Refusing an ungranted local Bot next to a friend's Bot throws `ungrantedRefusal`: a message that names the Bot, the friend and the two ways out, plus `error.details` `{ reason: 'ungranted-local', bots, remotes }`; `channel.js` `fail` forwards a plain-object `details` (empty otherwise) and `client/api.js` puts it on the thrown error, where `client/new-chat.js` draws a card (**Copy invite link for {Bot}** via `__flybot__.social.createInvite`, **Leave {Bot} out**) read through `ungrantedLocalFix` in `shared/remote-label.js`; the same file's `copiedToClipboard` reports whether the invite link really reached the clipboard (none, or a refused write, is `false`), and the card then shows the link to copy by hand instead of saying it was copied. | | `core/src/host/face-mentions.js` | Who "@" calls in a face-to-face room (#287): every alias of a participant (`faceAliases`, kept in `shared/remote-label.js` so the bubble chips the same names, #305; bare name, the picker label `'s Chief` / ` · 's Bot`, ` · `, handle), local and friend's Bots told apart. `faceMentions().handles` is the handle of every Bot a line names (the explicit mention list a sender attaches, #293) and `mentionedByHandle` resolves a received list to my Bots (`known` counts handles the room has). | @@ -198,7 +198,7 @@ Deciding such a task leaves one comment for its Bot to post. `task-accept-result The dialog's **Delivery** view is one snapshot of this repository's GitHub work, not a Bot's task board. `src/host/delivery-board.js` reads issues, pulls and workflow runs, lays them into `raw`, `ready`, `working`, `review` and `done-today`, and caches each URL with its ETag; a stale snapshot is still served while a refresh runs, and a `304` reuses the cached body. It installs no timer: the browser asks (`delivery-board`, optional `force`) and the host answers. Since #1211 a read is a state, not a board or nothing: the first one returns `loading` at once while the round runs in the background, so no request is parked on a whole round behind the reverse proxy; a round that cannot finish writes `failed` with the endpoint, the advice and the settings link, an older snapshot that could not be refreshed is `stale` and keeps its columns with the reason beside them, and a round past the 90 s budget lands as `partial` with what it read. A cell that could not be read degrades by itself — CI `unknown` rather than `0` — and a failed round waits 10 s, then 30 s, then two minutes before the next automatic attempt. -The module is read-only by construction: every request is a `GET`, and the payload carries no token. Issues are placed by state label (`agent-working` → working, `agent-ready` → ready, otherwise raw), pull requests in review, and pull requests merged, and issues or pull requests closed, since the Asia/Shanghai day start in done today (a closed pull request counts whether or not it merged). A pull request card shows the newest `Verdict:` on its own tip — a verdict that names an older tip is ignored and CI speaks instead — plus the tip sha. The header reports today and the 7-day window, medians for issue close and pull merge, the open counts, and CI as `counted` of `total` (the list is capped, and the payload says so rather than pretending the tail does not exist). `authz.js` allows the endpoint to any signed-in member: it reads one repository, not an owner's Bots. Channel `READS` carries `delivery-board` and `delivery-probe` (#1225) for the same reason: a window that only reads the team — another dsh holds `state.lock` — answers the board and its probe instead of refusing them with `bot/read-only`, while every write in that window stays refused. `core/test/home-lock.test.js` holds both halves: the two reads answer and `set-prefs` / `secret-set` are still `bot/read-only`. +The module is read-only by construction: every request is a `GET`, and the payload carries no token. Issues are placed by state label (`agent-working` → working, `agent-ready` → ready, otherwise raw), pull requests in review, and pull requests merged, and issues or pull requests closed, since the Asia/Shanghai day start in done today (a closed pull request counts whether or not it merged). A pull request card shows the newest `Verdict:` on its own tip — a verdict that names an older tip is ignored and CI speaks instead — plus the tip sha. The header reports today and the 7-day window, medians for issue close and pull merge, the open counts, and CI as `counted` of `total` (the list is capped, and the payload says so rather than pretending the tail does not exist). That header also carries `metrics.daily` (#1222): the last seven Asia/Shanghai days, oldest first and today last as `partial`, with a per-day CI block whose denominator leaves out cancelled runs. A day that could not be read is `null`, never `0`, and a week that could not be read leaves the series `null`. `authz.js` allows the endpoint to any signed-in member: it reads one repository, not an owner's Bots. Channel `READS` carries `delivery-board` and `delivery-probe` (#1225) for the same reason: a window that only reads the team — another dsh holds `state.lock` — answers the board and its probe instead of refusing them with `bot/read-only`, while every write in that window stays refused. `core/test/home-lock.test.js` holds both halves: the two reads answer and `set-prefs` / `secret-set` are still `bot/read-only`. ### Task-driven turns (#1109) diff --git a/patches/ds-bot/files/src/host/delivery-board.js b/patches/ds-bot/files/src/host/delivery-board.js index 4f709892d..0a51f5f8a 100644 --- a/patches/ds-bot/files/src/host/delivery-board.js +++ b/patches/ds-bot/files/src/host/delivery-board.js @@ -36,6 +36,48 @@ export const BACKOFF_MS = [10_000, 30_000, 120_000] export const startOfDay = (now = Date.now()) => Math.floor((now + SHANGHAI_OFFSET_MS) / DAY_MS) * DAY_MS - SHANGHAI_OFFSET_MS +// `YYYY-MM-DD` for a day boundary, read in the zone the day is cut in — the date the owner sees, +// not the UTC one, which is a day behind for eight hours of every Shanghai day (#1222). +export const shanghaiDate = (start) => new Date(start + SHANGHAI_OFFSET_MS).toISOString().slice(0, 10) + +// The seven-day series the chart draws (#1222): one entry per Asia/Shanghai day, oldest first, the +// last one today and marked `partial`. It is assembled from the two windows the round already read +// — the week's seven days and today's own — so it costs no request. A window that could not be +// read, or a day nobody counted, makes the whole series `null`: the view says it has no data +// instead of drawing seven quiet zero days. +export function dailySeries(week, todayMetrics, today) { + const buckets = new Map() + for (const day of [...(week?.daily ?? []), ...(todayMetrics?.daily ?? [])]) buckets.set(day.start, day) + const series = [] + for (let offset = 6; offset >= 0; offset -= 1) { + const start = today - offset * DAY_MS + const bucket = buckets.get(start) + if (bucket === undefined) return null + series.push({ + date: shanghaiDate(start), + ...(offset === 0 ? { partial: true } : {}), + newIssues: bucket.newIssues, + closedIssues: bucket.closedIssues, + newPullRequests: bucket.newPullRequests, + mergedPullRequests: bucket.mergedPullRequests, + ci: bucket.ci, + }) + } + return series +} + +// The per-day buckets are the series' raw material; the payload carries the assembled series only. +const withoutBuckets = (metrics) => { + if (!metrics) return metrics ?? null + const { daily: _buckets, ...rest } = metrics + return rest +} +const withoutCiDays = (ci) => { + if (!ci) return ci ?? null + const { days: _days, ...rest } = ci + return rest +} + export function median(values) { const list = (values ?? []).filter(value => Number.isFinite(value)).sort((a, b) => a - b) if (list.length === 0) return null @@ -276,16 +318,26 @@ export function createDeliveryBoard({ repo, token, request = fetch, now = Date.n return body } - async function pages(path, { max = MAX_PAGES, stop = null } = {}) { + // A walk that ends on its last allowed page with that page full, and with no row the `stop` + // predicate wanted, left a tail nobody read: the caller must say "unknown", not "fewer" + // (#1211, #1222). A full page alone is not that answer — the stop may have matched on it, which + // means the window was read whole and only the list ended (#1240). + async function walk(path, { max = MAX_PAGES, stop = null } = {}) { const rows = [] + let truncated = false for (let page = 1; page <= max; page += 1) { const separator = path.includes('?') ? '&' : '?' const chunk = await get(`${base}${path}${separator}per_page=${PAGE}&page=${page}`) if (!Array.isArray(chunk)) break rows.push(...chunk) if (chunk.length < PAGE || (stop !== null && chunk.some(stop))) break + if (page === max) truncated = true } - return rows + return { rows, truncated } + } + + async function pages(path, options) { + return (await walk(path, options)).rows } // Every page, not only the first (#1189). Past 100 comments — ordinary on this repository @@ -328,37 +380,53 @@ export function createDeliveryBoard({ repo, token, request = fetch, now = Date.n // The CI half of a window, kept apart from the issue and pull counts: `/actions/runs` missing a // permission is one unknown number in the header, not an empty board (#1211). async function ciFor(from, ciDays) { - const ci = { failed: 0, superseded: 0, counted: 0, total: 0, unknownDays: [] } + const ci = { failed: 0, superseded: 0, counted: 0, total: 0, unknownDays: [], days: [] } for (let day = 0; day < ciDays; day += 1) { - const window = windowOf(from - day * DAY_MS) + // Forward from the window's start, the way every other count in this module reads its + // window: walking back from `from` described the week before the one the other numbers + // cover, so a seven-day CI share was a week out of step with its own row (#1222). + const window = windowOf(from + day * DAY_MS) const runs = await runsOfDay(window.from, window.to) if (runs.truncated) { // The tail was not read, so this day's number is unknown: it is named and left out rather // than added as a partial truth (#1211). note('ci', failureOf(`the workflow runs for ${new Date(window.from).toISOString()} were truncated at ${MAX_PAGES * PAGE}`, { kind: 'truncated', scope: 'actions', endpoint: '/actions/runs', status: null })) ci.unknownDays.push(new Date(window.from).toISOString()) + ci.days.push({ start: window.from, unknown: true }) continue } ci.failed += runs.failed ci.superseded += runs.superseded ci.counted += runs.counted ci.total += runs.total + // The per-day block the chart reads (#1222). A cancelled run never judged the code, so it is + // left out of the denominator; the header keeps its own aggregate untouched. + ci.days.push({ start: window.from, failed: runs.failed, counted: Math.max(0, runs.counted - runs.superseded), total: runs.total }) } return ci } - async function metricsFor(from, { days = 1, ciDays = 1 } = {}) { + // A list that stopped at the page cap has a tail nobody read, so the counts built from it are + // unknown rather than smaller numbers (#1222). The flag is the walk's own answer, not the row + // count: a page four that is full because `stop` matched an old row there was read whole (#1240). + const capped = (listing) => listing.truncated === true + + async function metricsFor(from, { days = 1, ciDays = 1, buckets = false } = {}) { const to = from + days * DAY_MS - const [issues, pulls] = await Promise.all([ - pages(`/issues?state=all&since=${new Date(from - DAY_MS).toISOString()}`, { max: 4, stop: row => row.created_at !== undefined && Date.parse(row.created_at) < from - DAY_MS }), - pages('/pulls?state=all&sort=created&direction=desc', { max: 4, stop: row => Date.parse(row.created_at ?? 0) < from }), + const [issueList, pullList] = await Promise.all([ + walk(`/issues?state=all&since=${new Date(from - DAY_MS).toISOString()}`, { max: 4, stop: row => row.created_at !== undefined && Date.parse(row.created_at) < from - DAY_MS }), + walk('/pulls?state=all&sort=created&direction=desc', { max: 4, stop: row => Date.parse(row.created_at ?? 0) < from }), ]) + const issues = issueList.rows + const pulls = pullList.rows const plainIssues = issues.filter(row => !isPull(row)) const inWindow = (value) => { const at = Date.parse(value ?? ''); return Number.isFinite(at) && at >= from && at < to } const newIssues = plainIssues.filter(row => inWindow(row.created_at)).length const closedIssues = plainIssues.filter(row => inWindow(row.closed_at)) const newPrs = pulls.filter(row => inWindow(row.created_at)).length const merged = pulls.filter(row => inWindow(row.merged_at)) + const cappedIssues = capped(issueList) + const cappedPulls = capped(pullList) // A CI window that cannot be read leaves `ci: null` — the header says unknown, never zero. let ci = null if (ciDays > 0) { @@ -367,6 +435,27 @@ export function createDeliveryBoard({ repo, token, request = fetch, now = Date.n note('ci', error) } } + // One bucket per day of the window, for the seven-day series (#1222). A bucket is built from + // the rows this round already read, so it costs no request; a day whose list or run query was + // truncated, or whose CI could not be read at all, says `null` instead of a smaller number. + const dayBuckets = buckets + ? Array.from({ length: days }, (_, index) => { + const start = from + index * DAY_MS + const end = start + DAY_MS + const inDay = (value) => { const at = Date.parse(value ?? ''); return Number.isFinite(at) && at >= start && at < end } + const ciDay = ci?.days?.find(day => day.start === start) ?? null + return { + start, + newIssues: cappedIssues ? null : plainIssues.filter(row => inDay(row.created_at)).length, + closedIssues: cappedIssues ? null : plainIssues.filter(row => inDay(row.closed_at)).length, + newPullRequests: cappedPulls ? null : pulls.filter(row => inDay(row.created_at)).length, + mergedPullRequests: cappedPulls ? null : pulls.filter(row => inDay(row.merged_at)).length, + ci: ciDay === null || ciDay.unknown === true + ? null + : { failed: ciDay.failed, counted: ciDay.counted, total: ciDay.total }, + } + }) + : undefined return { newIssues, closedIssues: closedIssues.length, @@ -379,7 +468,8 @@ export function createDeliveryBoard({ repo, token, request = fetch, now = Date.n mergedPullRequests: merged.length, medianIssueCloseMs: median(closedIssues.map(row => Date.parse(row.closed_at) - Date.parse(row.created_at))), medianPullMergeMs: median(merged.map(row => Date.parse(row.merged_at) - Date.parse(row.created_at))), - ci, + ci: withoutCiDays(ci), + ...(dayBuckets ? { daily: dayBuckets } : {}), } } @@ -451,8 +541,8 @@ export function createDeliveryBoard({ repo, token, request = fetch, now = Date.n ]) // Two windows, settled separately: a week that cannot be read must not take today with it. const metricsResults = await Promise.allSettled([ - metricsFor(today), - metricsFor(weekStart, { days: 7, ciDays: 7 }), + metricsFor(today, { buckets: true }), + metricsFor(weekStart, { days: 7, ciDays: 7, buckets: true }), ]) const settle = (result, name) => { if (result.status === 'fulfilled') return result.value @@ -546,8 +636,9 @@ export function createDeliveryBoard({ repo, token, request = fetch, now = Date.n metrics: { dayStart: new Date(today).toISOString(), timeZone: 'Asia/Shanghai', - today: todayMetrics, - week: weekMetrics, + today: withoutBuckets(todayMetrics), + week: withoutBuckets(weekMetrics), + daily: dailySeries(weekMetrics, todayMetrics, today), open: { issues: issues.length, pullRequests: openPrs, diff --git a/patches/ds-bot/files/test/delivery-board.test.js b/patches/ds-bot/files/test/delivery-board.test.js index 5aba1ffca..17b305712 100644 --- a/patches/ds-bot/files/test/delivery-board.test.js +++ b/patches/ds-bot/files/test/delivery-board.test.js @@ -8,7 +8,7 @@ import assert from 'node:assert/strict' import { test } from 'node:test' import { readFileSync } from 'node:fs' -import { createDeliveryBoard, DELIVERY_COLUMNS, describeResponse, install, median, probeGitHub, startOfDay, BUILD_BUDGET_MS, BACKOFF_MS } from '../src/host/delivery-board.js' +import { createDeliveryBoard, dailySeries, DELIVERY_COLUMNS, describeResponse, install, median, probeGitHub, startOfDay, BUILD_BUDGET_MS, BACKOFF_MS } from '../src/host/delivery-board.js' import { install as installAuthz } from '../src/host/authz.js' import { install as installChannel } from '../src/host/channel.js' @@ -23,7 +23,7 @@ const respond = (body, status = 200, headerMap = {}) => ({ json: async () => body, }) -function fakeGitHub({ comments = {}, checks = {}, closedExtra = [], mergedExtra = [], openPullsExtra = [], failures = {}, hang = false } = {}) { +function fakeGitHub({ comments = {}, checks = {}, closedExtra = [], mergedExtra = [], openPullsExtra = [], failures = {}, hang = false, runsTruncated = false, listsCapped = false, listsStale = false, listsStopLate = false } = {}) { const calls = [] const request = async (url, init = {}) => { const parsed = new URL(url) @@ -48,6 +48,32 @@ function fakeGitHub({ comments = {}, checks = {}, closedExtra = [], mergedExtra { number: 13, title: 'Needs a human', html_url: 'https://x/13', labels: [{ name: 'needs-info' }], created_at: '2026-09-01T00:00:00Z', updated_at: '2026-10-01T00:00:00Z' }, ] : []) } + // The two list calls that feed the counts (#1222), when the page cap is what ends them: + // `listsCapped` answers four full pages of rows dated inside the week the board asks for, so a + // window's walk only stops at the cap; `listsStale` answers full pages of rows older than every + // window, so the walk ends on its first page and nothing was in fact left unread. + // `listsStopLate` is the same full walk with the `stop` row on page four: four full pages, and + // the only old row is the last one, so the walk ends because the window was read whole, not + // because it ran out of pages. + if ((path === '/issues' || path === '/pulls') && parsed.searchParams.get('state') === 'all' && listsStopLate) { + return respond(Array.from({ length: 100 }, (_, index) => ({ + number: 700 + page * 100 + index, + title: 'a row on a full page', + created_at: page === 4 && index === 99 ? '2026-10-01T00:00:00Z' : '2026-10-20T00:00:00Z', + closed_at: null, + ...(path === '/pulls' ? { merged_at: page === 4 && index === 99 ? '2026-10-01T00:00:00Z' : '2026-10-20T01:00:00Z' } : {}), + }))) + } + if ((path === '/issues' || path === '/pulls') && parsed.searchParams.get('state') === 'all' && (listsCapped || listsStale)) { + const created = listsStale ? '2026-10-01T00:00:00Z' : '2026-10-20T00:00:00Z' + return respond(Array.from({ length: 100 }, (_, index) => ({ + number: 900 + page * 100 + index, + title: 'a row on a full page', + created_at: created, + closed_at: null, + ...(path === '/pulls' ? { merged_at: listsStale ? '2026-10-01T01:00:00Z' : '2026-10-20T01:00:00Z' } : {}), + }))) + } if (path === '/pulls' && (parsed.searchParams.get('state') ?? 'open') === 'open') { return respond(page === 1 ? [ { number: 40, title: 'Pending verdict', html_url: 'https://x/40', body: 'Owner: Dev', head: { sha: 'aaaaaaa1' }, mergeable_state: 'unstable', created_at: '2026-10-24T00:00:00Z', updated_at: '2026-10-24T10:00:00Z', labels: [] }, @@ -87,6 +113,10 @@ function fakeGitHub({ comments = {}, checks = {}, closedExtra = [], mergedExtra ] : []) } if (path.startsWith('/actions/runs')) { + // A day whose runs outnumber the page cap: every page is full, so the tail is never read. + if (runsTruncated) { + return respond({ total_count: 1000, workflow_runs: Array.from({ length: 100 }, () => ({ conclusion: 'success' })) }) + } return respond({ total_count: 3, workflow_runs: [ @@ -515,6 +545,171 @@ test('metrics use the Shanghai day boundary, medians, and the CI counted/total c ])) }) +test('metrics.daily is seven Shanghai days ending today, and only today is partial (#1222)', async () => { + const { board, github } = build() + const snapshot = await settle(board) + const daily = snapshot.metrics.daily + // Seven days, oldest first, the last one today: the window the chart draws is the same one the + // header's Today column sits at the end of. + assert.equal(daily.length, 7) + assert.deepEqual([...daily.map(day => day.date)], + ['2026-10-19', '2026-10-20', '2026-10-21', '2026-10-22', '2026-10-23', '2026-10-24', '2026-10-25']) + assert.equal(daily[6].partial, true) + assert.equal(daily.slice(0, 6).some(day => 'partial' in day), false, 'only today is a part of a day') + const byDate = Object.fromEntries(daily.map(day => [day.date, day])) + // A day nobody counted is 0, and 0 is a measurement: 10-22 opened and closed nothing. + assert.deepEqual({ ...byDate['2026-10-22'] }, { + date: '2026-10-22', + newIssues: 0, + closedIssues: 0, + newPullRequests: 1, + mergedPullRequests: 0, + ci: { failed: 1, counted: 2, total: 3 }, + }) + assert.equal(byDate['2026-10-19'].newIssues, 1) + assert.equal(byDate['2026-10-20'].newIssues, 2) + assert.equal(byDate['2026-10-21'].newIssues, 2) + assert.equal(byDate['2026-10-24'].closedIssues, 1) + assert.equal(byDate['2026-10-24'].mergedPullRequests, 1) + // Today is the same day the Today column counts, not a day behind it. + assert.equal(byDate['2026-10-25'].closedIssues, snapshot.metrics.today.closedIssues) + assert.equal(byDate['2026-10-25'].mergedPullRequests, snapshot.metrics.today.mergedPullRequests) + // A cancelled run never judged the code, so it is out of the denominator; the header keeps its + // own aggregate, superseded included. + assert.deepEqual(byDate['2026-10-25'].ci, { failed: 1, counted: 2, total: 3 }) + assert.deepEqual(snapshot.metrics.today.ci, { failed: 1, superseded: 1, counted: 3, total: 3, unknownDays: [] }) + // The series costs no request: it is built from the two windows the round already reads. + assert.ok(github.calls.every(call => call.headers.Authorization === 'Bearer SECRET-TOKEN')) + // The assembled series is the only thing the payload carries — no per-day buckets anywhere else. + assert.equal('daily' in snapshot.metrics.today, false) + assert.equal('daily' in snapshot.metrics.week, false) +}) + +test('the day boundary is Shanghai, not UTC: an evening in UTC is already tomorrow (#1222)', async () => { + // 2026-10-24T17:00Z is 2026-10-25 01:00 in Shanghai, so the newest day is the 25th. + const { board } = build({}, { now: () => AT('2026-10-24T17:00:00Z') }) + const snapshot = await settle(board) + assert.equal(snapshot.metrics.dayStart, '2026-10-24T16:00:00.000Z') + const daily = snapshot.metrics.daily + assert.equal(daily[6].date, '2026-10-25') + assert.equal(daily[6].partial, true) + assert.equal(daily[0].date, '2026-10-19') + assert.equal(daily.length, 7) +}) + +test('the seven-day CI share is read over the week its counts cover (#1222)', async () => { + // The week window is [2026-10-17T16:00Z, 2026-10-24T16:00Z). Walking backwards from its start + // read the seven days *before* it, so the CI row described a week the other numbers never saw. + const { board, github } = build() + await settle(board) + const ranges = github.calls + .filter(call => call.path.startsWith('/actions/runs')) + .map(call => new URLSearchParams(call.query).get('created')) + const starts = ranges.map(range => Date.parse(range.split('..')[0])) + assert.equal(starts.length, 8, 'seven days for the week, one for today') + assert.ok(starts.every(at => at >= AT('2026-10-17T16:00:00Z')), 'no window reaches back before the week it describes') + assert.equal(starts.filter(at => at === AT('2026-10-17T16:00:00Z')).length, 1, 'the first day of the week is read') + assert.equal(starts.filter(at => at === AT('2026-10-23T16:00:00Z')).length, 1, 'the week runs up to the day before today') + assert.equal(starts.filter(at => at === AT('2026-10-24T16:00:00Z')).length, 1, 'today is its own window') +}) + +test('a day whose runs were truncated is unknown, and unknown is not zero (#1222)', async () => { + const { board } = build({ runsTruncated: true }) + const snapshot = await settle(board) + const daily = snapshot.metrics.daily + assert.equal(daily.length, 7) + // The tail of every day was unread, so no day may claim a CI share — including a quiet-looking 0%. + assert.equal(daily.every(day => day.ci === null), true) + // The counts are still numbers: a CI read that failed does not erase them. + assert.equal(daily[0].newIssues, 1) + assert.ok(snapshot.missing.some(item => item.part === 'ci')) +}) + +test('a count built from a list that stopped at the page cap is unknown, not smaller (#1222)', async () => { + // Both lists answer four full pages, so the walk ends on the cap with a tail nobody read. The + // week's window is capped; today's own window stops on its first page because these rows are + // already older than it, so today keeps its measurements and the cap reaches only the six days + // whose list it truncated. + const { board, github } = build({ listsCapped: true }) + const snapshot = await settle(board) + const daily = snapshot.metrics.daily + assert.equal(daily.length, 7) + for (const day of daily.slice(0, 6)) { + assert.equal(day.newIssues, null, `${day.date} sits in a list that stopped at the page cap`) + assert.equal(day.closedIssues, null, `${day.date} has no issue count to show`) + assert.equal(day.newPullRequests, null, `${day.date} has no pull count to show`) + assert.equal(day.mergedPullRequests, null) + } + // Today read its own window whole: an unknown day never costs a day that was measured. + assert.equal(typeof daily[6].newIssues, 'number') + assert.equal(typeof daily[6].closedIssues, 'number') + assert.equal(typeof daily[6].newPullRequests, 'number') + assert.equal(typeof daily[6].mergedPullRequests, 'number') + // The header counts are built from the same lists but are not per-day: the cap nulls the series + // entry, never the number a reader compares across the week. + assert.equal(typeof snapshot.metrics.today.newIssues, 'number') + assert.equal(typeof snapshot.metrics.week.newIssues, 'number') + assert.equal(daily[6].newIssues, snapshot.metrics.today.newIssues) + const issueCalls = github.calls.filter(call => call.path === '/issues' && new URLSearchParams(call.query).get('state') === 'all') + assert.equal(issueCalls.filter(call => new URLSearchParams(call.query).get('page') === '4').length >= 1, true, 'the week really walked into the page cap') +}) + +test('a window that stopped early on an old row is complete, not unknown (#1222)', async () => { + // A full first page of rows older than the window ends the walk with nothing left unread. The + // page-count heuristic reads that as "the list had more to give"; it must not, or a quiet week + // would drift into permanent unknowns. + const { board, github } = build({ listsStale: true }) + const snapshot = await settle(board) + const daily = snapshot.metrics.daily + assert.equal(daily.length, 7) + // Nothing was opened or merged inside the seven days, and that is a measurement: 0, never null. + for (const day of daily) { + assert.equal(day.newIssues, 0, `${day.date} was read whole`) + assert.equal(day.closedIssues, 0) + assert.equal(day.newPullRequests, 0) + assert.equal(day.mergedPullRequests, 0) + } + // One page per window, not four: the walk stopped on the old row rather than at the cap. + const issueCalls = github.calls.filter(call => call.path === '/issues' && new URLSearchParams(call.query).get('state') === 'all') + assert.equal(issueCalls.length, 2, 'week plus today, one page each') + assert.equal(issueCalls.some(call => new URLSearchParams(call.query).get('page') === '2'), false) + assert.equal(snapshot.missing.some(item => item.part === 'issues' || item.part === 'pulls'), false) +}) + +test('a full page four that stopped on an old row is a complete window, not a capped one (#1240)', async () => { + // The page count cannot tell these apart: four full pages either hit the cap or ended because the + // stop row was on page four. Only the walk knows, so the flag has to come from it. + const { board } = build({ listsStopLate: true }) + const snapshot = await settle(board) + const daily = snapshot.metrics.daily + assert.equal(daily.length, 7) + const byDate = Object.fromEntries(daily.map(day => [day.date, day])) + // 300 rows on pages one to three plus 99 on page four; the hundredth is the old row that stopped + // the walk. Every one of them was measured, so none of these days is unknown. + assert.equal(byDate['2026-10-20'].newIssues, 399) + assert.equal(byDate['2026-10-20'].newPullRequests, 399) + assert.equal(daily.every(day => day.newIssues !== null && day.newPullRequests !== null), true) + assert.equal(typeof snapshot.metrics.week.newIssues, 'number') +}) + +test('CI that cannot be read leaves every day at null, never at a zero share (#1222)', async () => { + const { board } = build({ failures: { '/actions/runs': { status: 403, headers: { 'retry-after': '30' } } } }) + const snapshot = await settle(board) + const daily = snapshot.metrics.daily + assert.equal(daily.every(day => day.ci === null), true) + assert.equal(daily[6].closedIssues, 1) +}) + +test('no week, no series: a window nobody read is null, not seven zero days (#1222)', () => { + // The series is assembled from the two windows the round read; without the week there is nothing + // to draw, and the view says so instead of drawing a flat line at zero. + assert.equal(dailySeries(null, null, AT('2026-10-24T16:00:00Z')), null) + const today = AT('2026-10-24T16:00:00Z') + const only = { daily: [{ start: today, newIssues: 1, closedIssues: 0, newPullRequests: 0, mergedPullRequests: 0, ci: null }] } + assert.equal(dailySeries(null, only, today), null, 'six days without a bucket are six unknown days') + assert.equal(dailySeries({ daily: [] }, only, today), null) +}) + test('a snapshot is cached, a stale one is served while it refreshes, and 304 reuses it (#1173)', async () => { let clock = NOW const github = fakeGitHub()