Repository navigation
Conversation
There was a problem hiding this comment.
Caution
hideWorkspaceFilterBadge is added to PersistedUIState but not to the strict ui.set schema (UiUpdateFields), so pnpm tc fails and paired web/mobile/relay clients will reject the writer's payload. See the inline comment for the one-line fix.
Reviewed changes
- New persisted preference
hideWorkspaceFilterBadge(defaultfalse) — optional field onPersistedUIState, wired through the UI slice contract, preference actions, hydration (=== true), the persisted-UI write baseline (type + field set), and the writer mirror. - Badge suppression in
SidebarWorkspaceOptionsMenu.tsx— render condition becomeshasAnyFilter && !hideWorkspaceFilterBadge; the accessible name and tooltip keep reporting the active-filter count. - New
FilterToggleRowinSidebarWorkspaceFilterSection.tsx— "Hide filter badge" with anEyeOfficon, appended to the Filters section. - Tests + i18n — new
SidebarWorkspaceOptionsMenu.test.tsx(3 tests, verified passing underconfig/vitest.config.ts);hideFilterBadgeadded toen.json; incidental alphabetical re-sort ofsendOutcomeLostinen-runtime-required.json(content identical).
The persistence seams that are present are correctly wired; I verified the localization gates (verify:localization-catalogs, runtime catalog) pass and that the new test genuinely fails with the bug present. The one blocker is the missing companion schema entry below.
ℹ️ Nitpicks
getDefaultUIState()andgetStartupErrorFallbackUI()omit the new field. Harmless today (hydration reads=== true, defaultfalse), but the neighboringhideDetachedHeadWorkspacesis listed there, so adding it keeps new profiles and the startup-error fallback consistent.
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📥 CommitsReviewing files that changed from the base of the PR and between ae6a886ba813cfeb4e9658fd77c5f1a57e582565 and 0dd7e64. 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughAdds the Priority: ⬇️ Low Severity of issue fixed: Low Merge Risk: ⚪ Minimal · up to The preference hides only the visual badge while retaining the accessible filter count and is described as persisting across reloads. No concrete merge-blocking risk is evident; the change appears ready for normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The setting changes badge visibility without changing workspace filtering or removing the accessible filter count. No introduced security issue was established. Compatibility with older connected hosts remains unconfirmed and could prevent a bundled preference update from saving. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 11 files. (1 skipped: 1 unsupported.)
🧰 Additional context used📚 Code guidelines (1)Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Verified against the code — this was correct, thank you. Fixed in ae6a886ba8: One incidental finding worth flagging for reviewers: local One semantic question for the maintainers: as written, the preference rides |
There was a problem hiding this comment.
✅ No new issues found.
The prior blocker is resolved: hideWorkspaceFilterBadge now spreads into the strict ui.set schema, pnpm tc:node passes, and the new component test stays green.
Reviewed changes (since the prior pullfrog review at 5493e59)
- Registered
hideWorkspaceFilterBadgein theui.setwire schema — addedhideWorkspaceFilterBadge: z.boolean().optional()toClientUiWorkspaceFilterFields, which spreads into the strictUiUpdateFields(client-ui-params.ts:163), satisfying the parity assertion and letting paired clients persist the preference.
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
The workspace options button shows a numeric badge with the active filter count. When filters are left on intentionally, the badge never goes away and becomes constant visual noise. Add a persisted "Hide filter badge" row at the end of the Filters section: on = suppress the badge only; the active-filter count stays in the button's accessible name, so screen-reader users keep the state. Fixes stablyai#25127 Generated-by: GLM-5.3-Flash (ZCode)
The field was added to PersistedUIState without a matching entry in ClientUiWorkspaceFilterFields, so the parity assertion in ui-state-schema-parity-checks fails with missingFromSchema, and paired clients' strict UiUpdate would reject the whole batched ui.set payload. Caught by pullfrog in review. Note: local `pnpm tc` stays green through the incremental tsbuildinfo cache; the red only shows under `tsc -b --force`. Generated-by: GLM-5.3-Flash (ZCode)
ae6a886 to
0dd7e64
Compare
|
Rebased onto current main (990b0ed): main extracted the inline badge into a shared |

ELI5
The sidebar's Workspace options button shows a little number badge while any workspace filter is active. If you keep filters on purpose (many people do), that badge never goes away and turns into permanent visual noise. This PR adds a "Hide filter badge" switch in the same menu, so you can keep your filters and lose the badge.
What Changed
hideWorkspaceFilterBadge(defaultfalse— current behavior is unchanged), wired through the same six persistence seams as the neighboring workspace-filter preferences: sharedPersistedUIStatetype, UI slice contract, preference actions, hydration, the persisted-UI write baseline (type + field set), and the persisted-UI writer mirror.FilterToggleRowat the end of the Filters section of the Workspace options menu.hasAnyFilter && !hideWorkspaceFilterBadge. The button's accessible name and tooltip still report the active-filter count while the badge is hidden — screen-reader users keep the state; only the visual noise goes.Why
The issue offers two directions: a setting to hide the badge, or a subtler dot. We implemented the toggle because it fully removes the noise for people who intentionally run filters long-term, and placing it inside the Workspace options menu — right where filters are turned on — makes it discoverable exactly where the annoyance comes from, without touching the Settings window. The dot variant can still be layered on later if the maintainers prefer it. No changes to the filter-count logic itself: the toggle is a display preference, not a filter, so it never counts toward the badge number.
Linked Issue
Fixes #25127
Visual Proof
N/A for attached media in this environment (headless run). Behavior is pinned by the new component tests: the badge renders with the default preference, disappears while "Hide filter badge" is on, and the accessible name keeps the active-filter count in both states.
Testing
New
SidebarWorkspaceOptionsMenu.test.tsx(3 tests, red before the fix — the hide case failed withexpected '2' to be null— green after). Also updatedpersisted-ui-write-baseline.test.tsfixture (typecheck-enforced).Commands actually run locally (macOS):
pnpm tc(exit 0), target + baseline + hydration suites viapnpm test(13 passed in the re-run), adjacent store/sidebar suites (123 passed across 7 files),npx oxlinton all touched files (0 warnings, 0 errors),verify:localization-catalogs/verify:localization-coverage(exit 0). Fullpnpm test/pnpm buildnot run locally — CI covers them.One incidental change:
en-runtime-required.jsonhadsendOutcomeLostout of alphabetical order (upstream commit landed without running the sync script);sync:localization-runtime-catalogmoved it back — content identical, and the catalog checks fail without it.AI Disclosure
Coding by GLM-5.3-Flash (ZCode agent), orchestrated and verified by a main agent with the same model; a human reviewed and committed the result.
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Pure renderer UI change: no platform branches, no SSH/remote surface, no wire/compat impact, no new dependencies. Cross-client persistence is additive — the new field is optional in
PersistedUIState, and older clients ignore unknown keys per the shared-state convention.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)