Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/1884-p255-task-done-median.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
The Project board's task-completion median now counts a card the way the task store finishes it — `status: 'accepted'`, timed from the last terminal `actors[]` entry — instead of the `completedAt` field the store never writes, so the number stops reading `—` on a real deployment ([#1884](https://github.com/ThinkFlowLab/FlyBot/issues/1884)).
26 changes: 21 additions & 5 deletions core/src/host/project-metrics.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
// `unavailable` and every value reads `null`, never `0`: a zero claims "none today" and the board
// must not turn "cannot read" into a number. The reads are injected, so a test counts calls without
// a network, and the day boundary is the reader's own time zone, not the host's UTC clock.
import { columnOf } from '../shared/task-board.js'

export const METRICS_TTL_MS = 5 * 60 * 1000
export const METRIC_KEYS = [
'prsOpenedToday', 'prsMergedToday', 'issuesOpenedToday', 'issuesClosedToday',
Expand Down Expand Up @@ -67,13 +69,27 @@ const durationInWindow = (items, from, to, now) => items.reduce((values, item) =
// as a map, so a caller handing the whole store keeps its old answer.
const taskList = tasks => (Array.isArray(tasks) ? tasks : Object.values(tasks ?? {}))

// The task half: a card that reached `done` inside the window. `completedAt` is the store's own
// stamp; a record written before the stamp reads `updatedAt` and is not counted as a duration the
// store never recorded.
// The task half: a card that really completed. The board's own reading decides that column
// (`columnOf`, so no second status table exists here), and `accepted` is the one column value that
// means "finished": a cancelled or failed card is in the done column but has no completion moment,
// so timing it from its cancellation would only stretch the median. A card finishes with
// `transition(task, 'accepted')`, which writes `status` and pushes a terminal entry onto `actors`;
// `completedAt` is not a field that path ever writes, so it is only a fallback for older records,
// after the last terminal actor's `at`.
const finished = task => columnOf(task) === 'done' && task.status === 'accepted'
const finishedAt = task => {
const actors = Array.isArray(task.actors) ? task.actors : []
for (let index = actors.length - 1; index >= 0; index -= 1) {
const entry = actors[index]
if (entry !== null && typeof entry === 'object' && finished(entry) && Number.isFinite(entry.at)) return entry.at
}
if (Number.isFinite(task.completedAt)) return task.completedAt
return Number.isFinite(task.updatedAt) ? task.updatedAt : null
}
const doneTaskDurations = (tasks, now) => taskList(tasks).reduce((values, task) => {
if (task === null || typeof task !== 'object' || task.status !== 'done') return values
if (task === null || typeof task !== 'object' || !finished(task)) return values
const start = Number.isFinite(task.createdAt) ? task.createdAt : null
const end = Number.isFinite(task.completedAt) ? task.completedAt : null
const end = finishedAt(task)
if (start === null || end === null || end < start) return values
if (now - end > RECENT_MS) return values
values.push(end - start)
Expand Down
4 changes: 2 additions & 2 deletions core/test/project-metrics-scope.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ const seed = () => ({
tasks: {
// This board: one card in flight, one finished an hour after it started.
'task-group': { id: 'task-group', kind: 'group', status: 'claimed', claimedBy: 'Writer', claimedAt: NOW - 2 * HOUR, origin: { roomId: 'r1' }, ownerBotId: 'a', assignee: 'Writer', title: 'Ship it', createdAt: 5, updatedAt: NOW - HOUR },
'task-done-a': { id: 'task-done-a', kind: 'group', status: 'done', origin: { roomId: 'r1' }, ownerBotId: 'a', title: 'Closed on p1', createdAt: NOW - 2 * HOUR, completedAt: NOW - HOUR, updatedAt: NOW - HOUR },
'task-done-a': { id: 'task-done-a', kind: 'group', status: 'accepted', origin: { roomId: 'r1' }, ownerBotId: 'a', title: 'Closed on p1', createdAt: NOW - 2 * HOUR, completedAt: NOW - HOUR, updatedAt: NOW - HOUR },
// The other Project: same shapes, and nine hours from start to done.
'task-foreign': { id: 'task-foreign', kind: 'group', status: 'claimed', claimedBy: 'Foreigner', claimedAt: NOW - 2 * HOUR, origin: { roomId: 'r2' }, ownerBotId: 'foreign', title: 'Secret salary review', createdAt: 5, updatedAt: NOW - HOUR },
'task-foreign-done': { id: 'task-foreign-done', kind: 'group', status: 'done', origin: { roomId: 'r2' }, ownerBotId: 'foreign', title: 'Foreign closed', createdAt: NOW - 10 * HOUR, completedAt: NOW - HOUR, updatedAt: NOW - HOUR },
'task-foreign-done': { id: 'task-foreign-done', kind: 'group', status: 'accepted', origin: { roomId: 'r2' }, ownerBotId: 'foreign', title: 'Foreign closed', createdAt: NOW - 10 * HOUR, completedAt: NOW - HOUR, updatedAt: NOW - HOUR },
},
})

Expand Down
46 changes: 42 additions & 4 deletions core/test/project-metrics.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// a failed read answers `null` (painted `—`), never `0`.
import test from 'node:test'
import assert from 'node:assert/strict'
import { METRICS_TTL_MS, dayKey, median, readMetrics, runningOf, summarizeMetrics } from '../src/host/project-metrics.js'
import { METRIC_KEYS, METRICS_TTL_MS, dayKey, median, readMetrics, runningOf, summarizeMetrics } from '../src/host/project-metrics.js'

const HOUR = 60 * 60 * 1000
const NOW = Date.parse('2026-10-30T12:00:00Z')
Expand Down Expand Up @@ -116,9 +116,9 @@ test('a running row carries who, how long and where from', () => {

test('the task median reads the store stamps only', () => {
const tasks = {
a: { status: 'done', createdAt: NOW - 10 * HOUR, completedAt: NOW - 8 * HOUR },
b: { status: 'done', createdAt: NOW - 6 * HOUR, completedAt: NOW - 4 * HOUR },
c: { status: 'done', createdAt: NOW - 6 * HOUR },
a: { status: 'accepted', createdAt: NOW - 10 * HOUR, completedAt: NOW - 8 * HOUR },
b: { status: 'accepted', createdAt: NOW - 6 * HOUR, completedAt: NOW - 4 * HOUR },
c: { status: 'accepted', createdAt: NOW - 6 * HOUR },
d: { status: 'open', createdAt: NOW - 6 * HOUR },
}
const out = summarizeMetrics({ now: NOW, timeZone: 'UTC', source: { ok: true, pullRequests: [], issues: [] }, tasks })
Expand Down Expand Up @@ -184,3 +184,41 @@ test('#1849: a valid zone keeps the day boundary and the answer it had', () => {
// A zone `Intl` accepts but the host would never guess stays as sent, and UTC still reads one day.
assert.equal(summarizeMetrics({ now: NOW, timeZone: 'UTC', source }).metrics.prsOpenedToday, 1)
})

// #1884: a card finishes by `transition(task, 'accepted')` (core/src/host/tasks.js:269-279), which
// writes `status`, `updatedAt` and one `actors[]` row — never `completedAt`. So the duration is
// `createdAt` → the last terminal `actors[].at` (9h here), not a field the store never writes.
const finished = (id, createdAt, acceptedAt) => ({
id,
title: id,
status: 'accepted',
createdAt,
updatedAt: acceptedAt,
actors: [
{ status: 'open', at: createdAt },
{ status: 'claimed', at: createdAt + HOUR },
{ status: 'accepted', at: acceptedAt },
],
})

test('a card that finished the way the host finishes cards is a done duration', () => {
const tasks = [
finished('t-9h', NOW - 10 * HOUR, NOW - 1 * HOUR),
{ id: 't-cancelled', status: 'cancelled', createdAt: NOW - 10 * HOUR, updatedAt: NOW - 2 * HOUR, actors: [{ status: 'cancelled', at: NOW - 2 * HOUR }] },
]
const out = summarizeMetrics({ now: NOW, timeZone: 'UTC', source: { ok: true, pullRequests: [], issues: [] }, tasks })
assert.equal(out.metrics.taskDoneMedianMs, 9 * HOUR)
assert.deepEqual(Object.keys(out.metrics).sort(), [...METRIC_KEYS].sort())
})

// A board whose only finished cards were abandoned was never completed on: `—`, never `0`. This one
// passes on main today only because no card is counted at all, so it is the non-regression half of
// the pair — it starts guarding the `accepted`-only rule the moment the first test goes green.
test('only cancelled and failed cards is null, never zero', () => {
const tasks = [
{ id: 't-cancelled', status: 'cancelled', createdAt: NOW - 10 * HOUR, updatedAt: NOW - 2 * HOUR, actors: [{ status: 'cancelled', at: NOW - 2 * HOUR }] },
{ id: 't-failed', status: 'failed', createdAt: NOW - 8 * HOUR, updatedAt: NOW - 3 * HOUR, actors: [{ status: 'failed', at: NOW - 3 * HOUR }] },
]
const out = summarizeMetrics({ now: NOW, timeZone: 'UTC', source: { ok: true, pullRequests: [], issues: [] }, tasks })
assert.equal(out.metrics.taskDoneMedianMs, null)
})
2 changes: 1 addition & 1 deletion docs/subsystems/core-host.md
Original file line number Diff line number Diff line change
Expand Up @@ -790,7 +790,7 @@ the host side only, so the tier stays T2 and `core/src/shared/` is untouched.

## Project metrics (#1829)

`core/src/host/project-metrics.js` answers the Project board's seven numbers: PRs opened and merged today, issues opened and closed today, and the seven-day medians of PR merge, issue close and task completion durations. The four day counts compare `created_at` / `merged_at` / `closed_at` against the reader's own time zone (`Intl.DateTimeFormat`), not the host clock — the zone arrives in the request body, so `resolveTimeZone` validates it once and an unknown name answers the named `UTC` instead of throwing out of the whole board read, rows, queues and decisions with it (#1849) — and the local task half reads `createdAt` / `completedAt` from `state.tasks`. The running table and the task-done median read one board's own cards, not the deployment's task store: `project-board.js` hands `readMetrics` the same `mine` list its rows were built from (#1848), so a card of another Project, or one this reader is refused, appears in neither, and every branch (no credential, no fetch, a failed read, a cache hit, a success) reproduces both from that same list. The five-minute cache is shared by every Project on the repository, so an entry holds only the repository-level numbers (`ok`, the time zone, the stamp and the six GitHub counts and medians); `running` and `taskDoneMedianMs` are re-projected from the caller's cards on a hit exactly as on a miss (#1880). The numbers ride on the existing `project-board` endpoint (`store.js` carries the `metrics` section beside the rows), which passes the host secret store's `GITHUB_TOKEN` and the reader's time zone from the request, requires the Project, and answers `ok: false` with `null` values when the credential is missing or a read fails — the client paints `—`, never `0`. One five-minute in-process cache keys on repository, time zone and window, so a hit spends no source call; a failed read is not cached. `runningOf` projects a claimed card into the running table's row (who, claim age, source, last activity).
`core/src/host/project-metrics.js` answers the Project board's seven numbers: PRs opened and merged today, issues opened and closed today, and the seven-day medians of PR merge, issue close and task completion durations. The four day counts compare `created_at` / `merged_at` / `closed_at` against the reader's own time zone (`Intl.DateTimeFormat`), not the host clock — the zone arrives in the request body, so `resolveTimeZone` validates it once and an unknown name answers the named `UTC` instead of throwing out of the whole board read, rows, queues and decisions with it (#1849) — and the local task half reads `createdAt` and the last terminal `actors[].at` (falling back to `completedAt`, then `updatedAt`) and counts only `accepted` cards. The running table and the task-done median read one board's own cards, not the deployment's task store: `project-board.js` hands `readMetrics` the same `mine` list its rows were built from (#1848), so a card of another Project, or one this reader is refused, appears in neither, and every branch (no credential, no fetch, a failed read, a cache hit, a success) reproduces both from that same list. The five-minute cache is shared by every Project on the repository, so an entry holds only the repository-level numbers (`ok`, the time zone, the stamp and the six GitHub counts and medians); `running` and `taskDoneMedianMs` are re-projected from the caller's cards on a hit exactly as on a miss (#1880). The numbers ride on the existing `project-board` endpoint (`store.js` carries the `metrics` section beside the rows), which passes the host secret store's `GITHUB_TOKEN` and the reader's time zone from the request, requires the Project, and answers `ok: false` with `null` values when the credential is missing or a read fails — the client paints `—`, never `0`. One five-minute in-process cache keys on repository, time zone and window, so a hit spends no source call; a failed read is not cached. `runningOf` projects a claimed card into the running table's row (who, claim age, source, last activity).

## The Project run seam (#1856)

Expand Down
26 changes: 21 additions & 5 deletions patches/ds-bot/files/src/host/project-metrics.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
// `unavailable` and every value reads `null`, never `0`: a zero claims "none today" and the board
// must not turn "cannot read" into a number. The reads are injected, so a test counts calls without
// a network, and the day boundary is the reader's own time zone, not the host's UTC clock.
import { columnOf } from '../shared/task-board.js'

export const METRICS_TTL_MS = 5 * 60 * 1000
export const METRIC_KEYS = [
'prsOpenedToday', 'prsMergedToday', 'issuesOpenedToday', 'issuesClosedToday',
Expand Down Expand Up @@ -67,13 +69,27 @@ const durationInWindow = (items, from, to, now) => items.reduce((values, item) =
// as a map, so a caller handing the whole store keeps its old answer.
const taskList = tasks => (Array.isArray(tasks) ? tasks : Object.values(tasks ?? {}))

// The task half: a card that reached `done` inside the window. `completedAt` is the store's own
// stamp; a record written before the stamp reads `updatedAt` and is not counted as a duration the
// store never recorded.
// The task half: a card that really completed. The board's own reading decides that column
// (`columnOf`, so no second status table exists here), and `accepted` is the one column value that
// means "finished": a cancelled or failed card is in the done column but has no completion moment,
// so timing it from its cancellation would only stretch the median. A card finishes with
// `transition(task, 'accepted')`, which writes `status` and pushes a terminal entry onto `actors`;
// `completedAt` is not a field that path ever writes, so it is only a fallback for older records,
// after the last terminal actor's `at`.
const finished = task => columnOf(task) === 'done' && task.status === 'accepted'
const finishedAt = task => {
const actors = Array.isArray(task.actors) ? task.actors : []
for (let index = actors.length - 1; index >= 0; index -= 1) {
const entry = actors[index]
if (entry !== null && typeof entry === 'object' && finished(entry) && Number.isFinite(entry.at)) return entry.at
}
if (Number.isFinite(task.completedAt)) return task.completedAt
return Number.isFinite(task.updatedAt) ? task.updatedAt : null
}
const doneTaskDurations = (tasks, now) => taskList(tasks).reduce((values, task) => {
if (task === null || typeof task !== 'object' || task.status !== 'done') return values
if (task === null || typeof task !== 'object' || !finished(task)) return values
const start = Number.isFinite(task.createdAt) ? task.createdAt : null
const end = Number.isFinite(task.completedAt) ? task.completedAt : null
const end = finishedAt(task)
if (start === null || end === null || end < start) return values
if (now - end > RECENT_MS) return values
values.push(end - start)
Expand Down
4 changes: 2 additions & 2 deletions patches/ds-bot/files/test/project-metrics-scope.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ const seed = () => ({
tasks: {
// This board: one card in flight, one finished an hour after it started.
'task-group': { id: 'task-group', kind: 'group', status: 'claimed', claimedBy: 'Writer', claimedAt: NOW - 2 * HOUR, origin: { roomId: 'r1' }, ownerBotId: 'a', assignee: 'Writer', title: 'Ship it', createdAt: 5, updatedAt: NOW - HOUR },
'task-done-a': { id: 'task-done-a', kind: 'group', status: 'done', origin: { roomId: 'r1' }, ownerBotId: 'a', title: 'Closed on p1', createdAt: NOW - 2 * HOUR, completedAt: NOW - HOUR, updatedAt: NOW - HOUR },
'task-done-a': { id: 'task-done-a', kind: 'group', status: 'accepted', origin: { roomId: 'r1' }, ownerBotId: 'a', title: 'Closed on p1', createdAt: NOW - 2 * HOUR, completedAt: NOW - HOUR, updatedAt: NOW - HOUR },
// The other Project: same shapes, and nine hours from start to done.
'task-foreign': { id: 'task-foreign', kind: 'group', status: 'claimed', claimedBy: 'Foreigner', claimedAt: NOW - 2 * HOUR, origin: { roomId: 'r2' }, ownerBotId: 'foreign', title: 'Secret salary review', createdAt: 5, updatedAt: NOW - HOUR },
'task-foreign-done': { id: 'task-foreign-done', kind: 'group', status: 'done', origin: { roomId: 'r2' }, ownerBotId: 'foreign', title: 'Foreign closed', createdAt: NOW - 10 * HOUR, completedAt: NOW - HOUR, updatedAt: NOW - HOUR },
'task-foreign-done': { id: 'task-foreign-done', kind: 'group', status: 'accepted', origin: { roomId: 'r2' }, ownerBotId: 'foreign', title: 'Foreign closed', createdAt: NOW - 10 * HOUR, completedAt: NOW - HOUR, updatedAt: NOW - HOUR },
},
})

Expand Down
Loading
Loading