feat(console): new ui improvements - #733
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 Walkthrough📝 Walkthrough🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
memory/ui/src/page/BankRail.tsx (2)
56-63: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign the empty-state copy with the page voice.
The rest of the page uses lowercase prose.
memory/ui/src/page/index.tsxlines 281-285 render "no banks yet" and "create one on the left, ...". This block uses "No banks yet." and "Create one below, ...". Use one style for both empty states.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@memory/ui/src/page/BankRail.tsx` around lines 56 - 63, Update the empty-state copy in the BankRail component to match the lowercase prose used by the corresponding empty state in index.tsx: change the sentence starts in “No banks yet.” and “Create one below...” to lowercase while preserving the existing wording and layout.
36-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAnnounce the loading skeleton as busy.
The skeleton classes and width helpers are defined, but the
aria-hiddenskeleton hides the loading state from screen readers untilinitialLoadends. Setaria-busy={initialLoad}on the rail container while loading.♿ Proposed change
- <aside className="mem-ui-rail" aria-label="bank list"> + <aside className="mem-ui-rail" aria-label="bank list" aria-busy={initialLoad}>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@memory/ui/src/page/BankRail.tsx` around lines 36 - 55, Set aria-busy={initialLoad} on the bank rail container aside in BankRail so the loading skeleton state is announced to assistive technologies while preserving the existing skeleton rendering.memory/ui/src/page/index.tsx (1)
70-115: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting the shared page helpers into one module.
useContainerNarrowhere is byte-identical touseContainerNarrowinstate/ui/src/page/browser.tsx(lines 95-115), andconfirmDiscardrepeats the implementation atstate/ui/src/page/browser.tsxlines 162-163.readStored/writeStoredalso duplicate the best-effortlocalStoragepattern used inmemory/ui/src/page/memory-data.ts. A shared module (or an export from@iii-dev/console-ui) keeps the narrow threshold semantics and the guard prompt text in sync across pages.This is optional for this PR. The current code is correct.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@memory/ui/src/page/index.tsx` around lines 70 - 115, Optionally extract the duplicated page helpers into a shared module, reusing the existing useContainerNarrow implementation, confirmDiscard behavior and prompt text, and readStored/writeStored localStorage guards. Update memory/ui/src/page/index.tsx and the corresponding browser or memory-data callers to import the shared symbols while preserving the current threshold semantics and best-effort persistence behavior.state/ui/src/page/index.tsx (1)
17-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
PanelSideinStateBrowser.
PanelSideis exported from@iii-dev/console-uias'left' | 'right', andPageRenderProps['panelSide']uses that alias. ImportPanelSideinstate/ui/src/page/browser.tsxand use it for the optional prop so the child type stays aligned with the page render prop contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@state/ui/src/page/index.tsx` around lines 17 - 31, Update StateBrowser’s optional panelSide prop in the StateBrowser component to use the exported PanelSide type from `@iii-dev/console-ui`. Import PanelSide in browser.tsx and replace the narrower or duplicated prop type, preserving the existing left/right behavior and alignment with PageRenderProps['panelSide'].
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@memory/ui/styles.css`:
- Line 403: Replace the deprecated word-break: break-word declaration with
overflow-wrap: anywhere in memory/ui/styles.css lines 403-403 (.mem-ui-banner
.detail), 447-447 (.mem-ui-error-panel p), and 820-820 (.mem-ui-prompt-body),
plus state/ui/styles.css lines 291-291 (.state-ui-error-panel p) and 403-403
(.state-ui-banner .detail).
In `@packages/console-ui/index.d.ts`:
- Around line 76-82: Synchronize the declared workingDir prop with the runtime
rendering path: update ScreenBody and ExtPage in
console/web/src/pages/Ext/index.tsx to accept and forward workingDir alongside
panelSide, tabId, and onRequestClose, sourcing the active conversation’s current
directory; alternatively remove workingDir from the console-ui declaration if
the runtime cannot provide it.
In `@shell/ui/src/page/index.tsx`:
- Line 109: The explorer trusts workingDir without validating it against allowed
base paths. In shell/ui/src/page/index.tsx:109-109, add a shared
withinBase-based validation helper or normalized chat value and use it in the
persisted ?? workingDir ?? info.primary_root fallback; in
shell/ui/src/page/index.tsx:239-239, apply the same validation before
changeRoot(next) so invalid chat folders cannot re-root the explorer.
In `@state/ui/src/page/ValueEditor.tsx`:
- Around line 88-93: Update the conflict check in the ValueEditor live-update
handler to use the current dirty state rather than only checking whether
draftRef.current is non-null. Store the derived dirty value in dirtyRef.current
alongside the existing dirty derivation, then require that state when deciding
whether to show serverNotice and block applying nextText.
---
Nitpick comments:
In `@memory/ui/src/page/BankRail.tsx`:
- Around line 56-63: Update the empty-state copy in the BankRail component to
match the lowercase prose used by the corresponding empty state in index.tsx:
change the sentence starts in “No banks yet.” and “Create one below...” to
lowercase while preserving the existing wording and layout.
- Around line 36-55: Set aria-busy={initialLoad} on the bank rail container
aside in BankRail so the loading skeleton state is announced to assistive
technologies while preserving the existing skeleton rendering.
In `@memory/ui/src/page/index.tsx`:
- Around line 70-115: Optionally extract the duplicated page helpers into a
shared module, reusing the existing useContainerNarrow implementation,
confirmDiscard behavior and prompt text, and readStored/writeStored localStorage
guards. Update memory/ui/src/page/index.tsx and the corresponding browser or
memory-data callers to import the shared symbols while preserving the current
threshold semantics and best-effort persistence behavior.
In `@state/ui/src/page/index.tsx`:
- Around line 17-31: Update StateBrowser’s optional panelSide prop in the
StateBrowser component to use the exported PanelSide type from
`@iii-dev/console-ui`. Import PanelSide in browser.tsx and replace the narrower or
duplicated prop type, preserving the existing left/right behavior and alignment
with PageRenderProps['panelSide'].
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6466c61b-2390-48ed-be91-f85c54c5e279
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
memory/ui/page.tsxmemory/ui/src/page/BankRail.tsxmemory/ui/src/page/MemoriesPanel.tsxmemory/ui/src/page/MemoryGraph.tsxmemory/ui/src/page/ModeToggle.tsxmemory/ui/src/page/RecallPanel.tsxmemory/ui/src/page/RulesPanel.tsxmemory/ui/src/page/icons.tsxmemory/ui/src/page/index.tsxmemory/ui/src/page/widgets.tsxmemory/ui/styles.csspackages/console-ui/index.d.tsshell/ui/page.tsxshell/ui/src/page/index.tsxstate/ui/page.tsxstate/ui/src/lib/widgets.tsxstate/ui/src/page/ItemView.tsxstate/ui/src/page/ItemsView.tsxstate/ui/src/page/ScopesView.tsxstate/ui/src/page/ValueEditor.tsxstate/ui/src/page/browser.tsxstate/ui/src/page/index.tsxstate/ui/styles.css
💤 Files with no reviewable changes (3)
- state/ui/src/page/ScopesView.tsx
- state/ui/src/page/ItemView.tsx
- state/ui/src/page/ItemsView.tsx
| font-family: var(--font-mono, ui-monospace, monospace); | ||
| font-size: 11.5px; | ||
| color: var(--color-ink-faint); | ||
| word-break: break-word; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Deprecated word-break: break-word in both redesigned stylesheets. Stylelint 17.14.0 reports declaration-property-value-keyword-no-deprecated at all five declarations. The shared root cause is the break-word keyword on word-break; the supported replacement is overflow-wrap: anywhere.
memory/ui/styles.css#L403-L403: replaceword-break: break-wordwithoverflow-wrap: anywherein.mem-ui-banner .detail.memory/ui/styles.css#L447-L447: replaceword-break: break-wordwithoverflow-wrap: anywherein.mem-ui-error-panel p.memory/ui/styles.css#L820-L820: replaceword-break: break-wordwithoverflow-wrap: anywherein.mem-ui-prompt-body.state/ui/styles.css#L291-L291: replaceword-break: break-wordwithoverflow-wrap: anywherein.state-ui-error-panel p.state/ui/styles.css#L403-L403: replaceword-break: break-wordwithoverflow-wrap: anywherein.state-ui-banner .detail.
🧰 Tools
🪛 Stylelint (17.14.0)
[error] 403-403: Deprecated keyword "break-word" for property "word-break" (declaration-property-value-keyword-no-deprecated)
(declaration-property-value-keyword-no-deprecated)
📍 Affects 2 files
memory/ui/styles.css#L403-L403(this comment)memory/ui/styles.css#L447-L447memory/ui/styles.css#L820-L820state/ui/styles.css#L291-L291state/ui/styles.css#L403-L403
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@memory/ui/styles.css` at line 403, Replace the deprecated word-break:
break-word declaration with overflow-wrap: anywhere in memory/ui/styles.css
lines 403-403 (.mem-ui-banner .detail), 447-447 (.mem-ui-error-panel p), and
820-820 (.mem-ui-prompt-body), plus state/ui/styles.css lines 291-291
(.state-ui-error-panel p) and 403-403 (.state-ui-banner .detail).
Source: Linters/SAST tools
| /** | ||
| * The active chat conversation's working directory, live: the page | ||
| * re-renders with the new value when the user picks another folder in | ||
| * chat, so filesystem-shaped pages can follow along. `null`/absent when | ||
| * no conversation is active or none is set. | ||
| */ | ||
| workingDir?: string | null |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the runtime that builds page render props and check for workingDir.
rg -nP --glob '!**/node_modules/**' -C4 '\bworkingDir\b'
rg -nP --glob '!**/node_modules/**' -C6 '\brender\s*\(\s*\{?[^)]*panelSide'Repository: iii-hq/workers
Length of output: 152
🏁 Script executed:
#!/bin/bash
set -u
echo "## repository files around console-ui and page/index"
git ls-files | rg '(^|/)(index\.d\.ts|package\.json$|index\.tsx$|page)' | head -200
echo
echo "## locate exact files"
fd -a 'index\.d\.ts$|index\.tsx$|package\.json$' . | sed 's#^\./##' | head -200
echo
echo "## search workingDir exact across repo"
rg -n --hidden --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' --glob '!coverage/**' 'workingDir' . || true
echo
echo "## search render/panelSide broader"
rg -n --hidden --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' --glob '!coverage/**' 'panelSide|\.render\(|render\(' . | head -300Repository: iii-hq/workers
Length of output: 35727
🏁 Script executed:
#!/bin/bash
set -u
echo "## packages/console-ui/index.d.ts relevant sections"
sed -n '1,130p' packages/console-ui/index.d.ts | cat -n
echo
sed -n '340,440p' packages/console-ui/index.d.ts | cat -n
echo
echo "## console/ui and console/web package references"
sed -n '1,220p' console/ui/package.json | cat -n
echo
sed -n '1,220p' console/web/package.json | cat -n
echo
echo "## injectable types and usage"
sed -n '1,110p' console/web/src/types/injectable-ui.ts | cat -n
echo
rg -n --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' --glob '!coverage/**' 'injectableConsole|console/ui|PageRenderProps|panelSide|workingDir' console/ui console/web/. packages/console-ui -C3 || true
echo
echo "## locate files containing injectableConsole literal"
rg -n --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' --glob '!coverage/**' 'injectableConsole' . -C3 || trueRepository: iii-hq/workers
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -u
echo "## console/web src files with workingDir and PageRenderProps"
rg -n --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' --glob '!coverage/**' 'workingDir|working_dir|working-dir|PageRenderProps' console/web/src/pages/Ext console/web/src/App.tsx console/web/src -C2 || true
echo
echo "## console/ui files"
fd -a . console/ui -t f -x sh -c 'echo "--- $1"; sed -n "1,220p" "$1" | cat -n' sh {}
echo
echo "## console-web src relevant snippets"
sed -n '320,380p' console/web/src/App.tsx | cat -n
sed -n '1,160p' console/web/src/pages/Ext/index.tsx | cat -nRepository: iii-hq/workers
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -u
echo "## console/web src files with workingDir and PageRenderProps"
rg -n '--glob "!node_modules/**" --glob "!dist/**" --glob "!build/**" --glob "!coverage/**"' 'workingDir|working_dir|working-dir|PageRenderProps' console/web/src/pages/Ext console/web/src/App.tsx console/web/src -C2 || true
echo
echo "## console/ui files"
fd -a . console/ui -t f -x sh -c 'echo "--- $1"; sed -n "1,220p" "$1" | cat -n' sh {}
echo
echo "## console-web src relevant snippets"
sed -n '320,380p' console/web/src/App.tsx | cat -n
sed -n '1,160p' console/web/src/pages/Ext/index.tsx | cat -nRepository: iii-hq/workers
Length of output: 25459
Sync workingDir into PageRenderProps.
packages/console-ui/index.d.ts declares workingDir on page renders, but console/web/src/pages/Ext/index.tsx still renders ExtPage with only panelSide, tabId, and onRequestClose. Add workingDir at ScreenBody/ExtPage and pass it through, or remove the prop from @iii-dev/console-ui until the runtime produces it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/console-ui/index.d.ts` around lines 76 - 82, Synchronize the
declared workingDir prop with the runtime rendering path: update ScreenBody and
ExtPage in console/web/src/pages/Ext/index.tsx to accept and forward workingDir
alongside panelSide, tabId, and onRequestClose, sourcing the active
conversation’s current directory; alternatively remove workingDir from the
console-ui declaration if the runtime cannot provide it.
| const withinBase = (p: string) => | ||
| info.base_paths.includes(p) || info.base_paths.some((base) => p.startsWith(`${base}/`)) | ||
| const persisted = restored?.root && withinBase(restored.root) ? restored.root : null | ||
| const next = persisted ?? workingDir ?? info.primary_root |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
workingDir is used as a root without base-path validation. The component validates a persisted root with withinBase but trusts workingDir in both root paths. If the chat working directory lies outside info.base_paths, the explorer roots on a path the worker refuses, and refreshTree masks the failure with an empty tree.
shell/ui/src/page/index.tsx#L109-L109: applywithinBase(workingDir)before using it in the fallback chain, for exampleconst chat = workingDir && withinBase(workingDir) ? workingDir : nulland thenpersisted ?? chat ?? info.primary_root.shell/ui/src/page/index.tsx#L239-L239: lift the same base-path check into a shared helper and call it beforechangeRoot(next), so a chat folder outside the allowed bases does not re-root the explorer.
📍 Affects 1 file
shell/ui/src/page/index.tsx#L109-L109(this comment)shell/ui/src/page/index.tsx#L239-L239
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@shell/ui/src/page/index.tsx` at line 109, The explorer trusts workingDir
without validating it against allowed base paths. In
shell/ui/src/page/index.tsx:109-109, add a shared withinBase-based validation
helper or normalized chat value and use it in the persisted ?? workingDir ??
info.primary_root fallback; in shell/ui/src/page/index.tsx:239-239, apply the
same validation before changeRoot(next) so invalid chat folders cannot re-root
the explorer.
| if (draftRef.current !== null && draftRef.current !== nextText) { | ||
| // Never clobber an editor with unsaved changes — offer the new | ||
| // value instead. | ||
| setServerNotice({ kind: 'changed', value: e.new_value }) | ||
| return | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use the dirty state, not draft !== null, to detect a conflict.
draftRef.current stays non-null after the user edits and then types the value back to the original text. In that case dirty is false, but this branch still shows the "changed on the server while you were editing" banner and skips the in-place apply. The editor then shows a stale value while the status bar reports "all changes saved".
Compare the draft against the stored text so only a truly dirty editor blocks the live update.
🐛 Proposed fix
- const draftRef = useRef(draft)
- draftRef.current = draft
+ const draftRef = useRef(draft)
+ draftRef.current = draft
+ const dirtyRef = useRef(false) const nextText = JSON.stringify(e.new_value ?? null, null, 2)
- if (draftRef.current !== null && draftRef.current !== nextText) {
+ if (dirtyRef.current && draftRef.current !== nextText) {Set dirtyRef.current = dirty next to the existing dirty derivation (line 71).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@state/ui/src/page/ValueEditor.tsx` around lines 88 - 93, Update the conflict
check in the ValueEditor live-update handler to use the current dirty state
rather than only checking whether draftRef.current is non-null. Store the
derived dirty value in dirtyRef.current alongside the existing dirty derivation,
then require that state when deciding whether to show serverNotice and block
applying nextText.
2 participants
Improvements on the UI
Summary by CodeRabbit