feat(folders): folders carry tags; new chats in a tagged folder inherit them - #5432
Conversation
3a9da84 to
517b247
Compare
GPT 5.6 Review (fork) — ✅ no blocking findingsReviewed Review detailsNo findings. |
UX Review (Fable 5, fork) — ✅ PASSUX-level review of UX-Verdict: PASS Complete state coverage (loading/error+retry/empty/populated), accurate creation-only copy, and keyboard-operable chips — the only rough edge is the cold-start path to a first tag. Suggestions
[UX-REVIEWED] aafa878 |
Design Review (Fable 5, fork) — ✅ PASSDesign-level review of
Design-Verdict: PASS Additive optional-key model, creation-only inheritance validated at the point of application, and crash-consistent filing — the right shape, verified coherent with the slot-save metadata contract. [DESIGN-REVIEWED] aafa878 |
First Principles Review (Fable 5, fork) — 🟡 CONCERNSPremise-level review of All claims verified against the base checkout. The fork copy-by-value precedent is real ( First-Principles-Verdict: CONCERNS The feature earns its place end-to-end and is unusually fully declared; the modal-only vocabulary Retry and a fifth tag-validator spelling are point patches with counted, deferred siblings. What this change shipsIntent: let a folder auto-tag every new chat started inside it, so tag-filtered views populate without hand-tagging. ADDITION (issue #5419).
Watch
[FIRST-PRINCIPLES-REVIEWED] aafa878 |
Opus 4.8 Review (fork) — ✅ no blocking findingsReviewed Review detailsBoth candidates confirmed low-value and below the bar:
I also verified No findings. [OPUS-REVIEWED] aafa878 |
517b247 to
72a3b30
Compare
|
Dispositions for the GPT 5.6 Review 🔴 on
|
|
Dispositions for the UX Review 🟡 CONCERNS on
|
|
Dispositions for the Design Review 🟡 CONCERNS on
|
|
Dispositions for the First Principles Review 🟡 CONCERNS on
|
|
Disposition for the Opus 4.8 Review ✅ advisory on
|
|
Round-2 CI reds on |
72a3b30 to
38aafe3
Compare
0b5727d to
eb677e7
Compare
eb677e7 to
1c96394
Compare
|
Disposition for GPT round (BLOCK + one finding), fixed in BLOCKING — stale folder-tags snapshot before filing: ACCEPTED and fixed with the suggested mechanism. The pass-top bulk read is deleted; the folder's raw tag ids are now read fresh via FINDING — function-local imports: ACCEPTED for |
|
Disposition for UX round (CONCERNS), addressed in Unresolved vocabulary renders nothing: ACCEPTED and fixed. The unresolved state ( No inline tag creation: DEFERRED as a follow-up, not folded into this round. The point is fair — |
|
Disposition for First Principles round (CONCERNS): Item 10 — undeclared eslint suppression: ACCEPTED, now declared in the PR body. The Item 6 subtraction (drop the crash-durability half): DECLINED, with the zero option now weighed in the body. Dropping the |
|
Disposition for UX round (CONCERNS), addressed in Failed fetch renders a permanent "Loading tags…": ACCEPTED and fixed with the suggested mechanism.
|
|
Disposition for First Principles round (CONCERNS): Fold the three inline slot-tag prunes onto |
|
Disposition for GPT round (BLOCK), fixed in BLOCKING — stale folder read before |
|
Disposition for UX round (CONCERNS), addressed in
Empty state sends the user out of the modal (inline "New tag" input): position unchanged from the prior two rounds. The draft-loss framing is noted and real, but the calculus hasn't moved: an inline create input gives this modal a tag-vocabulary write path (today it only selects), which is the scope expansion the First Principles lane is simultaneously pressing to shrink on this same head. The inline-create follow-up (bundling this, the |
|
Disposition for First Principles round (CONCERNS): Fold |
|
Disposition for GPT round (BLOCK), fixed in BLOCKING — malformed vocabulary id crashes the validator's set build: ACCEPTED and fixed. The vocabulary set now admits only string ids ( BLOCKING — quadratic dedupe ( Both fixes are in the shared helper this PR introduces ( |
|
Disposition for UX round (CONCERNS), fixed in "Close and reopen to retry" tells users to destroy their draft: ACCEPTED and fixed with the suggested mechanism. The error line now carries an inline Retry action wired to the sidebar's existing The once-per-open background refetch from the prior round stays as a belt-and-braces first attempt; the inline Retry is the user-visible path when that attempt also fails. |
|
Disposition for First Principles round (CONCERNS), addressed in Subtraction 1 — delete the once-per-open auto-retry: ACCEPTED. The Subtraction 2 — drop Watch — the deferred consolidation should count 4 sites, not 3: ACCEPTED. The helper's docstring now names Verified: unpiped tsc rc=0, eslint ratchet clean, 53→52 modal tests + 517 sidebar + integration suite green, black/flake8 clean. |
|
Disposition for First Principles round (CONCERNS, Watch items only) — holding position, no code change: Watch — extend the vocabulary Retry to the two other Watch — hold the author to the validator-consolidation follow-up: ACKNOWLEDGED. The commitment stands as declared in the helper's docstring (all five spellings, strict-mode-flag shape). Follow-up issues are queued for filing. With CI green and GPT/UX/Design/Opus all clean on this head, these advisory Watch items are the only open lane feedback. |
Problem / Motivation
Folders and tags are the sidebar's two organizational dimensions, but they don't compose: a folder that represents a workstream gives its sessions no tags, so every new chat launched inside it starts untagged and is invisible to tag-filtered board columns until someone tags it by hand — repeating a manual step for information the folder placement already expresses. Issue #5419 (design + use cases discussed there).
Why it matters
A session lives in exactly one folder (a tree) but can carry many tags (sets). Folder tags let the tree project into overlapping tag sets automatically, which turns tag-filtered board columns into views that populate by provenance: cross-folder rollups, attention/risk lanes, non-kanban context columns, and durable machine-readable selectors for automation — all with zero per-session bookkeeping. Without this, tag columns only work for users willing to hand-tag every session.
What changed (motivation → approach → change)
Goal: folders carry tags; new chats born in a tagged folder inherit them. Approach: extend the folder record's existing optional-metadata pattern (
color/project_dir/default_agentare already absent-means-default keys infolders.json, sotagsneeds no migration), and mirror the tag-inheritance mechanism #4419 established for forks (copy-by-value at the moment of association). Alternatives considered and rejected on the issue: LLM-derived tags (heavier, non-deterministic, and gated by the_bg-session constraint from #2253) and coupling tags to runtime scope (#2164 keeps folders/tags organizational).What was built:
src/kiro_crew/dashboard/chat_folders.py): optionaltags: list[str]of existing tag-vocabulary ids on create and update, validated via the sharedvalidate_folder_tag_idschokepoint (only a non-array payload 400s; unknown and non-string ids are silently filtered, deduped preserving order — no count cap — and the vocabulary intersection is skipped whentags.jsonwas unreadable at boot, failing open so a save cannot wipe stored tags). Malformed persisted entries (a hand-edited non-string infolders.json) are skipped at inheritance time rather than crashing slot creation.src/kiro_crew/dashboard/chat_handlers.py,api_chat_slot_create): when a genuinely NEW slot is created with afolder_id, the folder's tags are copied by value onto the slot after the folder's existence is confirmed. Creation-only: addressing an existing slot by name never re-tags, and moving a session into a folder never retro-tags.src/kiro_crew/dashboard/channel_slots.py): the per-channel default-filing path applies the same copy-by-value inheritance on its first-filing branch, so a channel chat born into a tagged folder inherits exactly like a dashboard chat (its restore branch — a persistedfolder_id— never re-tags). The folder's raw tag ids are read fresh insidetags_write_lock, immediately before validation and the filing write (lock orderingtags_write_lock→ folder-store lock, matching the folder create/PATCH paths), so a concurrent folder PATCH or tag deletion can never stamp an obsolete tag set onto a freshly filed chat. The inherited ids are persisted in the channel conversation's metadata (atagskey written atomically with thechannel_folder_filedmarker) and re-applied on every surface through an authority-aware validator that fails open whentags.jsonis unreadable — this is permanent on-disk state, required because the sticky filing marker blocks re-inheritance, so a crash between the filing write and the slot's first save must be recoverable. The zero option (accept the crash-window loss) was weighed and rejected: with the marker persisted alone, the loss is permanent and silent (no pass ever re-offers the tags); dropping the marker instead would re-run inheritance onto chats whose user deliberately removed the tags. Carrying the tags in the already-required atomic marker write is the minimal crash-consistent shape.website/src/components/FolderConfigModal.tsx): while thechat-tagsquery is loading or has failed, the Tags section renders its heading with a muted "Loading tags…" placeholder (localized in all 11 catalogs) instead of nothing, so a failed query no longer silently erases the feature and the section resolving after open does not shift the layout.FolderConfigModal.tsxwere fixed with real a11y wiring (id/htmlFor/aria-labelon the tag chips), plus one justifiedjsx-a11y/label-has-fordisable on the folder-name label — the deprecated rule cannot see thehtmlFor→idlink to the customInputcontrol (same precedent asRegistryManager.tsx).channel_slots.pywas black-formatted because the gate flagged it as a new offender (it is not baselined).tagswrites silently filter unknown ids, mirroringapi_chat_slot_tags' filter-don't-400 shape — a dangling id left by a failed best-effort strip is shed on the next save rather than 400ing the folder. The two validators differ deliberately on one axis: the folder/inheritance paths carry an_tags_authoritativefail-open gate (an unreadabletags.jsonat boot must not wipe a folder's stored tags on save), whileapi_chat_slot_tagsvalidates user-submitted ids strictly against the live vocabulary with no such gate (a vocabulary outage should not silently accept arbitrary ids from a PUT body). They are one filtering shape with two intentionally different failure postures, not two spellings of one rule.src/kiro_crew/dashboard/chat_tags.py,api_chat_tag_delete): deleting a tag now strips its id from every folder's tags list, exactly as it already strips it from every slot.website/src/components/FolderConfigModal.tsx+ChatSidebar.tsx,types/index.ts,api/client.ts): a Tags section in the folder settings modal renders the vocabulary as color chips. Each chip is a label wrapping a visually-hidden checkbox (a multi-select choice control, not an action row — keeps the picker inside themax-two-buttons-per-rowdesign rule), wired into the modal's touched-based PATCH. The draft seed filters persisted ids against the live vocabulary, so a dangling reference (left by a failed best-effort strip) can never make the folder uneditable via a 400. With an empty vocabulary the section shows a one-line onboarding hint instead of vanishing. Keyboard focus (ring-2) is visually distinct from selection (ring-1 + tint). New strings added to all i18n catalogs, en-XA regenerated.docs/system-specs/modules/learn-cron-dashboard.md): the folder POST/PATCH contract and the slot-create inheritance are documented in the same commit, per AGENTS.md.Tests
Backend (
test/test_dashboard_chat.py,test/test_chat_slot_create_folder.py— including the newTestFolderTagInheritance, plustest/test_channel_folders.pyfor the channel first-filing branch):folder_idinherits the folder's tagsFrontend (
website/src/test/FolderConfigModal.test.tsx— 46 pass, component statement coverage 92%):touchedonly reportstagswhen the selection actually changedManual verification
Verified on a seeded dev-harness gateway (built dist, port-isolated home): created a tag vocabulary and a tagged folder via the API, opened the folder settings modal in a headless browser, exercised the chip toggles, and captured the screenshots below from that live session.
Screenshots / video
Folder settings modal with the new Tags section — chips carry the tag's color; the folder's tags are pre-selected in edit mode:
Toggling a third chip on (selection ring + tinted fill):
Related Issues
Fixes #5419
Checklist
feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)