Repository navigation
fix(timeline): the report can read "1 turns, 3 failed (300%)" — the digest skips the health module's count compensation - #1579
Conversation
…pulation (ThinkFlowLab#1578) Docs-not-needed: docs/subsystems/timeline.md invariant 5 (line 52) already states this rule — the error rate divides by the larger of the turns that began in the window and the failed turns, so it never reads above its turn count. This change makes digest.js obey that sentence; it adds and changes no documented behaviour, and window.js, health.js and client.js are untouched.
d6dafa4 to
8139276
Compare
|
@Birfy review request for tip
The PR was also rebased onto live Evidence: red first with the source at base and only the new tests kept — the digest cases fail on the As of my read: 0 reviews, 0 comments. |
|
Verdict: PASS at 8139276 Ran at this tip: Red first, at the base blob with this tip's tests kept: Scope: Tier: T2, recomputed from the diff with the base The arithmetic is the same compensation Checks at the tip: 14 check-runs; One thing noted, not blocking: Written-by: Reviewer |
…hinkFlowLab#1785) Fixes ThinkFlowLab#1780 Written-by: FlyBot Dev-1 A Bot's own section of the timeline run report could read `- look: 3 of 3 turns failed` while the `## Team` rollup right below printed no `needs a look:` line at all. The per-Bot flag counts against `max(turns, errors)` since ThinkFlowLab#1579 — a failure that began before the window still overlaps it — but `team.unhealthy` still filtered on `bot.turns`, so the very case that comment describes dropped the Bot from the rollup. After the change the rollup names the same Bots the sections flag, and nothing else about the report moves. Base: `92a4cf262cb8f2469f800f12e5cfe006abb4d538` (live main). Head: `6a23a7596d95515bc244f0c712fd4c0ade1ee849` (tree `0930515f2120a34f09d9e928044d24b685f92f98` — the same tree the local commit `2db83b296d8e` carried, so the verify run below describes this tip byte for byte). ## Red first Test `plugins/timeline/test/digest.test.js`, run on the base `92a4cf262cb8f2469f800f12e5cfe006abb4d538` with only the new test added (the file is otherwise byte-identical to the base): ``` DSH_HARNESS=<checkout>/core/dev/harness npm --prefix plugins/timeline test ``` ``` not ok 27 - \ThinkFlowLab#1780: needs a look names a Bot whose failures began before the window --- type: 'test' location: 'plugins/timeline/test/digest.test.js:108:1' failureType: 'testCodeFailure' error: |- Expected values to be strictly deep-equal: + actual - expected + [] - [ - 'b1' - ] ``` The window fixture is `{ lane: 'b1', turns: 2, errors: 3 }` — one failed turn began before the window, exactly the case the comment above the per-Bot flag names. The existing test at `digest.test.js:98` (`turns: 4, errors: 3`) passes on the base and still passes here, so the red is the missing denominator, not the test being unable to pass. After the fix the same suite is 130/130 green and the `ThinkFlowLab#582: the team rollup equals the Bot sections` test is unchanged and green. ## What changed `plugins/timeline/src/digest.js` — one filter reads the same denominator as the flag above it: ```diff - unhealthy: bots.filter(bot => bot.turns >= ERROR_TURNS_MIN && bot.errorRate >= ERROR_RATE_MIN).map(bot => bot.id), + unhealthy: bots.filter(bot => bot.counted >= ERROR_TURNS_MIN && bot.errorRate >= ERROR_RATE_MIN).map(bot => bot.id), ``` No threshold, sort or output shape changes; `bot.counted` is already carried on each Bot entry and is what `errorRate` divides by. `plugins/timeline/test/digest.test.js` gains the three assertions of the case above (the per-Bot flag, `team.unhealthy`, and the `needs a look:` line), and `changelog.d/1780-digest-needs-a-look.fixed.md` records the user-visible change. Real line count, excluding nothing (no generated file, no mirror, no patch): 18 lines, `+17/−1` — source `+3/−1`, test `+13/−0`, changelog `+1/−0`. `git diff --numstat 92a4cf2...HEAD` lists these three files and no other: `patches/ds-bot/` and `core/` are untouched, so no `generate` run was needed or made (this surface is not under any patch topic). ## What I ran `TZ=UTC`, `ARCH_DOCS_BASE=92a4cf262cb8f2469f800f12e5cfe006abb4d538`: - `npm run verify` — exit 0: core lint (`oxlint` 0 errors / 1 warning, unchanged from the base), plugin lint, `build-client --check`, `ds-bot-patches check`, core 2133, social 539, tree 125, timeline 130, workflow 289, `.github/scripts` 549, interop 37, live-e2e 24 — every suite `# fail 0`. - `check-arch-docs` re-run after the commit against the base ref — `arch-docs: OK (3 changed file(s))`. The change is covered by the `Docs-not-needed:` trailer in the branch commit: `docs/subsystems/timeline.md` describes what `digest.js` reads and its count/duration formatters, not the rollup's failure threshold, and no architecture moves. - `plugins/timeline` suite after the fix — 130/130. What I did not run: the real-model bench (`bench.mjs`) and the live smoke test — neither is part of `verify`, this change touches no `index.js` or `core/src/host/` file, and no live instance was touched. ## Gates at this head `enqueue-guard` on `6a23a7596d95515bc244f0c712fd4c0ade1ee849`: `REFUSE — do not enqueue ThinkFlowLab#1785: no Verifier report names the tip 6a23a75` — red until a Verifier report names this tip, as expected before review. The issue-reference and `agent-pr` rules are not asked of a fork head. ## Checkpoint <!-- flybot:checkpoint --> ```json { "pr": 1785, "claim": "Dev-1", "issue": 1780, "summary": "The timeline run report now names in its `## Team` roll-up every Bot its per-Bot sections flag. `team.unhealthy` still filtered on `turns` while the per-Bot flag has counted against `max(turns, errors)` since ThinkFlowLab#1579, so a Bot whose failed turns began before the window read `3 of 3 turns failed` in its section and was missing from `needs a look`. The filter reads the same `counted` field now, a regression test covers the turns 2 / errors 3 case, and no other output of the report changed.", "files": ["plugins/timeline/src/digest.js", "plugins/timeline/test/digest.test.js", "changelog.d/1780-digest-needs-a-look.fixed.md"], "checks": { "ran": "TZ=UTC npm run verify", "result": "green", "at": "6a23a7596d95515bc244f0c712fd4c0ade1ee849" }, "next": "none — ready for review" } ``` <!-- /flybot:checkpoint --> ## Scope One branch, one issue, one PR (`ThinkFlowLab#1779` is a different Bot's change to the same file at a different line; no file here is rewritten, so the two do not conflict). `patches/`, `core/` and every other plugin are untouched.
Fixes #1578
Owner: Dev-1
Reviewer: Reviewer
Written-by: Dev-1
Risk: T2 —
plugins/timeline/src/digest.jsis plugin code (.github/agent/risk-rules.json), so this needs a PASS at the tip plus green CI. No T3 path, no dependency change.Tip SHA:
8139276924c95ec4f30691244c8e4c3d2030ebe0Tree:
bd8db4c70d8271142d4798b42b8405d73ff9953fBase:
c1b9fd23e404dcff1eb782a2269e30e09e2d6993— livemain(#1547,#1572,#1556,#1552,#1573).What was wrong
The report compared two different populations.
plugins/timeline/src/window.js:249countsturnsas the turns that began in the window;:250countserrorsas every overlapping turn whose outcome iserror. A failed turn that began before the window is therefore inerrorsand not inturns, anddigest.jsdivided one by the other:const errorRate = turns > 0 ? errors / turns : 0, printed in three places.That is exactly the reading
docs/subsystems/timeline.mdinvariant 5 says can never happen — the error rate "divides by the larger of the turns that began in the window and the failed turns, so it never reads a failure count above its turn count" — andplugins/timeline/src/health.js:27-32already compensates withMath.max(use.turns, use.errors)and a comment naming the failure text it avoids. The digest did not.Reproduced at
main, realbuildWindow+buildDigest, 15-minute window, one Botb1: three turns that begin 3/4/5 minutes before the window and fail 20/21/22 s into it, plus one turn that begins inside it and completes. The window holds 4 turns (outcomes {completed: 1, error: 3},utilization.turns1,errors3), and the report read:- 1 turns, 3 failed (300%)- 1 turns, 3 failed1 turns, 3 failed, 0 tokens in over 07:45–08:00.The mild shape is worse than a bad percentage: one overlapping failed turn plus one completed turn that began inside the window reads
1 turns, 1 failed (100%), i.e. "the window's only turn failed", when that turn is the completed one.1 turnswas also printed everywhere.What changed
plugins/timeline/src/digest.jsonly — three lines of arithmetic and the three printers:counted = Math.max(turns, errors), anderrorRate = errors / counted, the same denominatorhealth.jsuses.countedis now on eachbots[]entry and onteam, so a consumer can read the number the share was taken over;turnsanderrorskeep their raw window values.## Teamline and the one-line summary all printcounted, so the number compared with the failure count is the number printed, and the Team rollup is the sum of the Bot sections, as before.${errors} of ${counted} turns failed, the wordinghealth.jsalready uses, andno turnsnow needscounted === 0rather thanturns === 0(a Bot whose failed turns all began before the window is not "no turns" and is not reported as one).1 turn,3 turns).(100%)beside a window whose single completed turn began inside it reads as its opposite. At that point the report prints the counts and no percentage (3 turns, 3 failed), which is still consistent and never over 100%.Scope and what I did not touch:
window.js's two counters are unchanged — they are documented and the panel reads them.health.jsis unchanged; the digest now matches it instead of diverging.plugins/timeline/client.js:588(the panel's Utilization line, same pair of numbers) is not changed. It prints two raw counts and no rate, so it cannot overclaim a share; changing the count it prints would make the panel disagree with the documented meaning ofutilization.turns(invariant 5) that the rest of the panel andhealth.jsrely on. If the owner wants the panel to print the comparison denominator too, it is a separate, one-line change with its own test.changelog.d/1578-digest-error-rate.fixed.mdadded. Docs unchanged: invariant 5 already states the rule this change implements.Docs-not-needed: docs/subsystems/timeline.md invariant 5 (line 52) already states this rule — the error rate divides by the larger of the turns that began in the window and the failed turns, so it never reads above its turn count. This change makes digest.js obey that sentence; it adds and changes no documented behaviour, and window.js, health.js and client.js are untouched.Same line in the branch commit (
git log -1 --format=%B), so the trailer survives the squash into the commitmainjudges, and it is what turnsarch-docsgreen at the tip:arch-docs: FAILwas the only reason the first tip'stests-okwas red.Red → green
At the base with only
test/digest.test.jsadded (source atmain): 13 tests, 10 pass, 3 fail —#1578: three failed turns that began before the window never read as 300% of one turn#1578: a failed turn that began before the window is not read as the window's own turn failing#1578: a single turn reads as one turn in all three placesThe fourth new test,
#1578: a clean window is byte-identical to the report before the change, passes on both sides by design: it is the pinned output (turns: 3, errors: 0) that proves the denominator does not move whenerrors <= turns.At the tip:
test/digest.test.js13/13,plugins/timeline129/129 (base 125 + these 4).Verification at the tip
Tip
8139276924c95ec4f30691244c8e4c3d2030ebe0, treebd8db4c70d8271142d4798b42b8405d73ff9953f, basec1b9fd23e404dcff1eb782a2269e30e09e2d6993, 3 files+86/−10(digest.js20/10,test/digest.test.js65/0, changelog 1/0).Ran:
npm --prefix plugins/timeline test→ 129/129, 0 fail.TZ=UTC ARCH_DOCS_BASE=c1b9fd23e4 npm run verify→ exit 0: core 1869, social 527, tree 125, timeline 129, workflow 142,.github/scripts548, interop 37, live-e2e 24 — 3401 tests, 0 fail; oxlint 0 errors (core 63 / social 36 / timeline 9 / tree 2 / workflow 13 warnings);arch-docs: OK (3 changed file(s))— the two plugin files are covered by theDocs-not-needed:trailer below;build-client --checkandds-bot-patches checkclean.node .github/scripts/changelog.mjs check→ 63 fragments well formed.cd core && ARCH_DOCS_BASE=c1b9fd23e4 node scripts/check-arch-docs.mjs→arch-docs: OK (3 changed file(s))at the tip with theDocs-not-needed:trailer in both places (first tip:FAILon the same two files,tests-okred for that reason alone).Not run: the smoke test (no
core/index.jsorsrc/host/change), thepackagejob, Desktop, and the owner's live instance.Checkpoint
{ "pr": 1579, "claim": "Dev-1", "issue": 1578, "summary": "The team report divided overlapping failed turns by the turns that began in the window, so a failure from before the window printed as 1 turns, 3 failed (300%). The digest now counts against max(turns, errors), the denominator health.js already uses, in the per-Bot line, the Team line and the one-line summary, with the team total the sum of the Bot sections. It prints a share only below 100%, since with every counted turn a failed one the numbers cannot say whose turn failed, and singular/plural follows the count. window.js, health.js and client.js are untouched.", "files": ["plugins/timeline/src/digest.js", "plugins/timeline/test/digest.test.js", "changelog.d/1578-digest-error-rate.fixed.md"], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=c1b9fd23 npm run verify", "result": "green", "at": "8139276924c95ec4f30691244c8e4c3d2030ebe0" }, "next": "none — ready for review at the tip" }