Repository navigation
feat(todos): multi-project filter, and one shared cache for issue and PR lists - #434
Conversation
The project dropdown was single-select: one project or all of them. Each project row is now a checkbox that toggles in place, so the menu stays open while several are ticked; "All projects" clears the ticks and closes it. The trigger reads All projects, the one project's name, or "N projects". Only the ticked projects are fetched. The pick is saved under the profile-scoped `todosProjects` key (local, not synced: it is a list of this machine's project ids). It is checked against the project list on read and never pruned on a write, because a launch reads the list after this store and pruning then would wipe the saved pick. A pick naming only removed projects reads as all. DropdownCheckboxItem wraps Radix's checkbox item with the themed box drawn as a picture of the state, so the item is the one thing that toggles. The e2e handle exposes useTodos so todos.e2e.ts can seed rows: the fixture's origin is a local path no forge CLI answers for.
…d cache Todos kept its own query cache, keyed without the limit, and the New Task and Review PR dialogs fetched the same lists again with no cache at all. Nothing survived a reload, so the first open after launch said "No items found" while the forge CLIs were still answering. src/store/forgeLists.ts now owns every issue and PR list fetch: - Keyed by project, kind, scope and limit, with a missing limit taken as the IPC's default, so a bare call and an explicit 30 are one query. - One queue of at most three CLI subprocesses for every caller, and a query already in flight is joined rather than fetched twice (a forced Refresh during a fetch used to queue a second one). - The last ok answer is kept through a failed refresh, and a failure is cached for the TTL like before. - The ok answers are written to the scoped `forgeListSnapshot` key once the queue drains, under a 256K-character budget filled personal lists first, with bodies cut one past the prompts' cap so a prompt built from a restored row is the same text. Restored entries keep their fetch time, so the first reader re-fetches them behind the old rows. useTodos keeps only the filters. One table (todoQueries) says which lists a filter reads, for the fetch and for the rows alike, and the rows carry the project's current name. The view fetches whenever that set of lists changes, which also covers a project list that arrives after the view mounts, and says "Loading issues and PRs" rather than "No items found" until every list it reads has answered.
The New Task dialog's issue list, its PR lists (yours, waiting on your review, All open) and Review PR's waiting list now go through src/store/forgeLists.ts like Todos does. When the cache holds a list (from Todos, an earlier open, or the launch snapshot), its rows paint at once instead of a spinner; every entry still re-fetches, so a freshly filed issue or PR shows up as before. When the re-fetch lands: - an unchanged list keeps the object on screen, so nothing re-renders and no row moves under the cursor; - a changed list replaces it; - a failure with cached rows on screen keeps them and says "Showing the last list. Refreshing it failed: <reason>" rather than swapping a usable list for the error card; with nothing cached the failure is the answer, as before. Review PR's waiting list keeps its rows silently, since a failure there only ever hid the list. Every staleFor and sequence guard stays on the re-fetch, and the dialogs read the cache through getState(), never a subscription, so a Todos fetch does not re-render them. Fetches by PR number stay direct.
The todos spec polled the ticked box until its computed border colour
matched its fill. It passed alone three times and failed once late in
a full run, which is the sampled-during-a-transition trap docs/gotchas.md
describes ("Assert STATE, not the paint of state"). The tick is already
asserted through aria-checked, so the colour poll goes and the hit test
(a click on the box lands on the row) stays as its own case.
|
LGTM, the multi-select project filter and shared forge-lists cache are implemented correctly with no blocking issues. One nit: New Task and Review PR list fetches now share the same 3-wide queue as Todos, so opening one during a large Todos refresh can leave it waiting behind every Todos call. Thanks for the contribution! |
simion
left a comment
simion
left a comment
There was a problem hiding this comment.
Needs one change before merge: restored issue and PR bodies can lose the truncation note.
capBodies (src/store/forgeLists.ts:76) cuts the raw body at 4001 characters.
issueContext (src/lib/issuePrompt.ts:98) and prReviewPrompt (src/lib/prReview.ts:81) run san() first, which strips \r, and only then compare against 4000.
Bodies written in GitHub's web editor are CRLF.
Example: a 7700 character CRLF issue body.
Live row: 4000 characters plus "[body truncated, read the rest with gh issue view ...]".
Restored row: the 4001 cut loses ~130 \r to san, lands under 4000, no note.
The agent gets a body that stops mid-sentence and nothing says there is more.
Reproduced through serializeSnapshot → parseSnapshot → issueContext.
Fix: san + trim before cutting (or cut with a wide margin), plus a test for the CRLF case.
Worth knowing, not blocking:
- The New Task and Review PR pickers wait in the same 3-wide queue as Todos. Todos on All is 5 queries per project, so with 20 projects an uncached dialog list sits behind 100 jobs.
- Snapshot rows paint before the refetch, so an issue closed since the last fetch can be picked into a task while the refetch is still queued.
- Nit: ReviewPrDialog's limit 8 and New Task's
PR_OPEN_LIMIT = 50don't match Todos' 30, so those lists get separate cache keys and never share. - Nit:
projectsPickedis an unsuffixed plural key; docs/i18n.md wants_one/_other.
(Reviewed by Simion's agent, he'll take the final look.)
Review on simion#434 found a restored issue or PR body could lose its truncation note. The snapshot cut the RAW body one past the 4000 character cap, but issueContext and prReviewPrompt strip control characters and trim before they compare against it. A CRLF body from GitHub's web editor lost every \r to that cleaning, landed under the cap, and the agent got a body that stops mid-sentence with nothing saying there is more. cutBodyForSnapshot now cuts the cleaned, trimmed text, with each module's own cleaning (the issue one also strips C1 controls, the PR one does not), and extends the cut to a non-blank character so the prompt's trim cannot pull it back under either. Tests round-trip a CRLF issue body, a cut landing on blanks, and a CRLF PR description through the snapshot and compare the prompts; all three fail with the old cut. Also from the review: - The dialogs' fetches wait at the FRONT of the shared queue, and a dialog joining a list Todos queued first moves it there, so New Task and Review PR no longer wait out a large Todos refresh. A job that throws before returning its promise now frees its slot. - projectsPicked becomes projectsPicked_one / _other, picked with the ternary docs/i18n.md asks for.
|
Fixed in 30f67db. On the rest:
|
simion
left a comment
simion
left a comment
There was a problem hiding this comment.
LGTM, the CRLF cut is fixed.
cutBodyForSnapshot cuts the cleaned, trimmed text and extends past trailing blanks, so a restored row always lands over the cap and keeps the note.
Ran the three round-trip tests locally, green.
Nit: in TodosView.tsx:238 the outer ternary already handles picked.length === 1, so projectsPicked_one is never reached. Harmless.
(Reviewed by Simion's agent, he'll take the final look.)
|
This is live in v1.20.0, released today. Thanks for the contribution. |
Summary
The Todos project dropdown now takes a set of projects instead of one.
Each project is a checkbox row, the menu stays open while you tick several, and "All projects" clears the ticks and closes it.
The pick is saved per profile under
todosProjects(local only, not synced).Issue and PR lists now load through one shared cache,
src/store/forgeLists.ts, used by Todos, the New Task dialog's issue and PR pickers, and Review PR's waiting list.Before this, Todos had its own in-memory cache, the dialogs had none, and nothing survived a reload, so the first Todos open after launch said "No items found" while the forge CLIs were still answering.
What changed
DropdownCheckboxIteminui/Dropdown.tsx(Radix's checkbox item, drawn with the themedCheckbox).The trigger reads All projects, the one project's name, or "N projects".
A saved pick is checked against the project list on read and never pruned on a write, because on launch the list loads after the store and pruning then would wipe it.
projectForgePrs(id)and Todos' explicit 30 are one query.forgeListSnapshotkey once the queue drains, capped at 256K characters and filled with the personal lists first, since localStorage is one quota shared by every key.Bodies are cut one past the prompts' 4000-character cap, so a prompt built from a restored row is the same text.
Restored entries keep their fetch time, so they're stale and get re-fetched behind the old rows.
Rows carry the project's current name rather than the one at fetch time.
An unchanged answer keeps the same object, so nothing re-renders; a changed one replaces it.
A failed re-fetch with cached rows on screen keeps them and says "Showing the last list. Refreshing it failed: ".
Fetches by PR number stay direct.
Testing
I ran the change in the dev app and everything worked as expected.
npm test(3560) andnpm run buildpass.todos.e2e.tsis new and covers the picker: ticking with the menu staying open, the saved pick, All, and a picked project being removed.A new
git.e2e.tscase covers a cached issue list surviving a failed re-fetch with the notice.A full local
make e2epassed 29 of 40 spec files, includingreview-pr,taskand the newgit.e2e.tscase.todos.e2e.tsfailed once on a painted-colour check that had passed alone three times, so the last commit replaces it with a state assertion; that version hasn't run in the real window yet.The other failures were in specs this doesn't touch (caffeinate, scratchpad, terminal find, tabs, the command palette, theme import, the board drag, sidebar status, the PR card, and peers, which needs a dependency missing from my checkout).
The local harness stopped launching before I could rerun them, so the e2e jobs here are the better check.