Repository navigation
Conversation
|
Review (Birfy, 2026-10-20 20:07 Asia/Shanghai): PASS (code) — held for the owner's confirmation of #538 Tip SHA: c5e47a5
Findings (non-blocking, for the owner's decision):
#538 is a new feature the owner has not confirmed yet, so this is not merged regardless of tier. It shares the same |
|
@HugoXK Owner decision (2026-10-20 21:09 Asia/Shanghai): accepted. Required change: also show per-tool calls/failures in the dashboard/panel, not only in timeline/window. |
|
Waiting for approval. It needs (1) the owner's approval through the GitHub UI (a chat reply does not count) and (2) a review PASS at the exact tip SHA; any new commit voids both. Automation does not merge T2/T3 by itself and does not approve on the owner's behalf. |
Birfy
left a comment
There was a problem hiding this comment.
PASS — tip c5e47a50535afbb8f5694c80c0cd6239ca274a2c (Maintainer review, acting as Birfy)
Design check: no change to Bot authorization, to the approval path (still a UI button only) or to user-facing English text; the new surface is read-only and part of the existing answer.
Local evidence: I applied this PR's diff to my local main snapshot and ran the touched tests there (TZ=UTC), plus node scripts/ds-bot-patches.mjs check (with the new files staged, as a commit would):
plugins/timeline:node --import ./test/resolve.mjs --test test/window.test.js→ 22/22 passnode scripts/ds-bot-patches.mjs check→ ok (pin v0.1.5, 10 patches, core == ds-bot/ + patches)
The new toolUse is read-only, names come through cleanLabel(name, 60), and the list is capped at 20 per Bot.
Not run locally: the full npm run verify in one shot (this box has a 2 GiB memory cap, so I run the steps one at a time), the smoke test, Desktop, and the real-model steward.
CI: tests-ok is green at the tip SHA above (GitHub Actions ran the full suite there).
Tier: T2/T3, so this PASS does not approve the PR: the owner still has to approve it through the GitHub UI, and any new commit voids both the approval and this PASS. Not merged by automation.
c5e47a5 to
918f8dd
Compare
Birfy
left a comment
There was a problem hiding this comment.
Verdict: PASS at 918f8dd
Tip SHA: 918f8dd7752ab05b322a6609356946b7379ec7f7
The branch was rebased onto current main at 01:31Z (single commit, parent 4f6a53d7), so it is clean again — no rebuild was needed; the earlier PASS at c5e47a50 is void. Read the whole new diff: plugins/timeline/src/window.js (+18 -1, a `toolUse` list per Bot inside the existing timeline/window answer), test/window.test.js (+16), one docs/subsystems/timeline.md row, one changelog fragment.
Design: read-only and additive — `toolUse` is another field of the same answer, per Bot, counted from that Bot's non-room turns, names through cleanLabel(60), worst-first by calls then name, capped at 20 entries per Bot; no new endpoint, no config, no user-facing string, no approval path. Failed calls come from `tool.ok === false` only, so an unfinished call is not counted as a failure.
TDD: the new test reads `win.toolUse`, which does not exist at the base window.js — 0 pass / 1 fail with only window.js reverted, and it passes with the change.
Ran at the tip: CI tests-ok green at this SHA (test, plugins tree/social/timeline, package all success). On a copy of current main with this diff applied: plugins/timeline 96/96 (TZ=UTC), changelog.mjs check OK.
Not run: one-shot npm run verify (2 GiB OOM), core suite, smoke, Desktop.
Tier: T2. Awaiting the owner's click in the GitHub UI plus this PASS at the tip. Any new commit voids this PASS. Note: #531/#535/#539 all touch the same return line in window.js and the same docs row, so only one merges cleanly; after it lands, the other two need a rebase (and a fresh PASS at the new tip).
|
Do not approve this tip — it no longer merges. The PASS at |
…FlowLab#357/ThinkFlowLab#359) timeline/window gains toolUse: per Bot, its tools by call count with a failed count, names cleaned and capped. A flaky tool or connector surfaces where the run is drawn. Read-only, part of the existing answer.
918f8dd to
3523673
Compare
Birfy
left a comment
There was a problem hiding this comment.
Verdict: PASS at 3523673
Reviewed again at this tip: rebased onto main 33fb55ed, so the previous PASS (918f8dd7) is void.
Diff (4 files, +36 -2): `plugins/timeline/src/window.js` adds `toolUse` (per-Bot tool counts by name and per-tool failures, cleaned codes, capped lists), the new test in `plugins/timeline/test/window.test.js`, the `timeline.md` row for `window.js`, and `changelog.d/timeline-tooluse.added.md`. The rebase only moved the docs row onto #531's wording; no behaviour change since the version I reviewed at 918f8dd7.
Ran at the tip: CI tests-ok green. On a copy of this PR's own base with the whole diff applied: `ds-bot-patches.mjs check` OK, `check-arch-docs.mjs` OK (4 files), `changelog.mjs check` OK, plugins/timeline 97/97, `scripts/lint-plugins.mjs timeline` 0 errors / 8 warnings, TZ=UTC.
TDD: with only `window.js` at the base commit, the new "toolUse" test fails (23/24); all 97 pass with the change.
Conflicts: with #535 and with #568 (same `timeline.md` row; #535 also shares the `window.js` return line and the test file). Clean with main, #566, #572, #574, #578, #580.
Not run: one-shot npm run verify (2 GiB OOM), full core suite, smoke, Desktop, a real model call.
Merge matrix (local 3-way merges against main 33fb55e and against every other open PR): see the lines above; every claim is measured, not assumed.
Tier: T2 (plugins/**, core/src/host/**, core/types/**). The owner's own click in the GitHub UI is the approval and this PASS must sit on the tip; I do not approve or merge it myself. Any new commit voids this PASS.
|
One precision on the PASS above: I could not byte-diff the pre-rebase branch (the force-push removed the old commit), so "the only difference is the docs row" is my reading of the source hunks against my review notes for the old tip, not a diff I could run. What is verified at the new tip is everything else in the PASS: the full check set, the plugin test suite and the red-without-fix run were all redone here. |
|
Superseded by #586 (one combined timeline change, at main The maintenance round merged this PR's change, its test and its docs wording into #586 together with #535 and #568, because all three edit the same Closing this PR as superseded. Thank you for the original — the tests and the docs row came through unchanged. |
…lowLab#586) Fixes ThinkFlowLab#534, Fixes ThinkFlowLab#538, Fixes ThinkFlowLab#567 **Risk: T2** (`plugins/timeline/**`). ## What One PR for the three timeline read-model proposals that all touch the same return line of `plugins/timeline/src/window.js` and the same `docs/subsystems/timeline.md` row, so the owner gets one PASS and one click instead of three rebases: - `stuck`: the open request arrows, worst first (`waitedMs` floored at 0, capped at 20) — ThinkFlowLab#534 / ThinkFlowLab#469. - `toolUse`: per-Bot tool calls by cleaned name with their failure counts — ThinkFlowLab#538 / ThinkFlowLab#359. - `timeline/health`: new pure `plugins/timeline/src/health.js` (`buildHealth(window)`) plus the read-only endpoint, per-Bot signals and flags and a team `ok` — ThinkFlowLab#567 / ThinkFlowLab#357. Both new window fields are additive; the window version and every existing field are unchanged. Host-side has no copy of this code (the timeline is a plugin), so `patches/ds-bot/` is untouched and `ds-bot-patches.mjs check` still passes. ## Supersedes Closes ThinkFlowLab#535, ThinkFlowLab#539 and ThinkFlowLab#568 as superseded: their content, review and tests are all carried here, including the authorship trail in the three PRs. Thanks to HugoXK for the three originals. ## Checks (run by the maintenance round on this tree) - `ds-bot-patches.mjs check` — OK (10 patches, core == ds-bot + patches). - `check-arch-docs.mjs` with `ARCH_DOCS_BASE=HEAD~1` — OK (7 changed files). - `changelog.mjs check` — OK (12 fragments). - `plugins/timeline` tests — 101/101. - `scripts/lint-plugins.mjs timeline` — 0 errors, 8 warnings (all pre-existing). - TDD: with `window.js` and `timeline.js` at the base commit and `health.js` absent, 3 tests fail (the two new window tests and the health test file); all pass with the change. Not run: one-shot `npm run verify` (the container OOMs at 2 GiB), full core suite, smoke, Desktop.
…inkFlowLab#590) **Risk: T3** (`AGENTS.md` and the policy docs). The owner's new hard rule: when an issue already has a pull request, work in that one — do not open a second and close the first. This records it next to the admission line, so the two are read together. ## What changes (4 files, +7 -1) - `AGENTS.md`: a new paragraph in the admission-line section — re-base the open branch, use `update-branch`, or push a commit to its head, and put the tests there too; a contributor's fork is treated the same while it allows maintainer edits; the only exception is a branch that cannot be pushed, and then the reason goes on the issue, never a silent swap. - `docs/code-standards.md`: the same rule as a bullet under "Merge only a reviewed tip". - `docs/autonomous-dev.md`: one sentence in the merge-by-tier item. - `.claude/skills/fix-issue/SKILL.md`: a bullet in step 4 (PR). It applies to new work from now on: ThinkFlowLab#586 and its closed originals (ThinkFlowLab#535/ThinkFlowLab#539/ThinkFlowLab#568) are left exactly as they are, as the Chief asked. ## Checks - `node --test .github/scripts/*.test.mjs` — 93/93. - `check-arch-docs.mjs` with `ARCH_DOCS_BASE=HEAD~1` — OK (4 changed files). - `scripts/ds-bot-patches.mjs check` — OK (no code touched). - `changelog.mjs check` — OK; no fragment, a change users cannot see and that ships no code. - `TZ=UTC`. Not run: one-shot `npm run verify` (2 GiB OOM in this container), full core suite, smoke, Desktop.
Timeline: which tools each Bot used, and how often they failed
The timeline listed each tool call inside a turn but never summarized which tools a Bot relies on or how often they fail, so a flaky tool or connector was hard to notice.
timeline/windownow carriestoolUse: per Bot lane, its tools as{ name, calls, failed }, by call count (top 20), names cleaned and capped. Read-only, metadata only.Ran at the tip:
npm run verify(green,TZ=UTC),node scripts/lint-plugins.mjs timeline(0 errors). Not run: smoke, Desktop, real-model steward.