fix(web,install): model refresh UI + CLI-only install summary - #6
Merged
Conversation
- web: add refresh_models / models_refreshed handling with optimistic modelsRefreshing spinner and ModelPicker Refresh control - install: report embedded core for CLI-only installs; avoid blank print_box continuations that abort set -e after success - docs: clarify that terminal-only prebuilts do not install catcode-core
📝 WalkthroughWalkthroughThe PR adds web model-list refresh controls with optimistic state, reducer handling, completion toasts, and provider-aware UI. It also updates Unix installer summaries and documentation for embedded-core TUI installations. ChangesWeb model refresh
Embedded-core installation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ModelPicker
participant useAgent
participant Core
participant reducer
User->>ModelPicker: Click refresh
ModelPicker->>useAgent: refreshModels()
useAgent->>reducer: Set refreshing state
useAgent->>Core: Send refresh_models
Core-->>reducer: Emit models_refreshed
reducer-->>ModelPicker: Clear loading state and show refresh toast
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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
🤖 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 `@docs/installation.md`:
- Around line 31-34: Update the installation documentation around the prebuilt
TUI’s extracted core location to refer to the platform user cache directory
followed by catalyst-code, rather than hard-coding ~/.cache/catalyst-code.
Preserve the existing note that terminal-only installs do not place catcode-core
on PATH.
In `@install.sh`:
- Around line 1621-1634: Restore the source-install state in load_state() by
setting BUILD_FROM_SOURCE when the persisted METHOD is source, before
do_update() or do_update_source() dispatches. Ensure source CLI-only updates and
reinstalls follow the source path and summary_update()/summary_reinstall()
report the external catcode-core location consistently with save_state().
- Around line 406-407: Make the banner entries near the TUI/web-service
descriptions conditional on BUILD_FROM_SOURCE: for source builds, report
catcode-core as a separate binary consistent with the later installation
summary; otherwise retain the existing embedded-core and --with-web wording.
Update only the banner construction in install.sh.
In `@web/src/lib/use-agent.ts`:
- Around line 1113-1125: Update the refresh_models background task handling,
including the spawned task that performs discovery/enrichment, to catch panics
and always emit a correlated terminal models_refreshed state when the task
fails. Preserve the existing optimistic refresh state flow in refreshModels,
ensuring modelsRefreshing is cleared after both successful completion and
panic-driven failure.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a6dc9463-f0d5-4cbc-ba45-618e17a16377
📒 Files selected for processing (12)
CHANGELOG.mddocs/installation.mdinstall.shweb/src/components/chat.tsxweb/src/components/header.tsxweb/src/components/ide/shell.tsxweb/src/components/model-picker.tsxweb/src/components/settings.tsxweb/src/lib/reducer.test.tsweb/src/lib/reducer.tsweb/src/lib/types.tsweb/src/lib/use-agent.ts
Comment on lines
+31
to
+34
| Installs the `catcode` TUI binary to `/usr/local/bin` (system-wide). The | ||
| prebuilt TUI embeds the Rust core and extracts it to | ||
| `~/.cache/catalyst-code/` on first run — a separate `catcode-core` on PATH is | ||
| **not** installed for terminal-only installs. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 5 'os\.UserCacheDir|cacheDir := filepath.Join' tui/embed_core.go
rg -n -C 3 '~/.cache/catalyst-code' docs/installation.md
if command -v go >/dev/null 2>&1; then
go doc os.UserCacheDir
fiRepository: catalystctl/catcode
Length of output: 2424
Document the platform user cache directory.
embeddedCorePath() uses os.UserCacheDir() before appending catalyst-code, so the documented fixed path can be wrong when XDG_CACHE_HOME, macOS ~/Library/Caches, or unavailable user cache paths are in effect. Use “the platform user cache directory” or list platform-specific paths.
Proposed fix
The
prebuilt TUI embeds the Rust core and extracts it to
-`~/.cache/catalyst-code/` on first run — a separate `catcode-core` on PATH is
+the platform user cache directory on first run — a separate `catcode-core` on PATH is
**not** installed for terminal-only installs.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Installs the `catcode` TUI binary to `/usr/local/bin` (system-wide). The | |
| prebuilt TUI embeds the Rust core and extracts it to | |
| `~/.cache/catalyst-code/` on first run — a separate `catcode-core` on PATH is | |
| **not** installed for terminal-only installs. | |
| Installs the `catcode` TUI binary to `/usr/local/bin` (system-wide). The | |
| prebuilt TUI embeds the Rust core and extracts it to | |
| the platform user cache directory on first run — a separate `catcode-core` on PATH is | |
| **not** installed for terminal-only installs. |
🤖 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 `@docs/installation.md` around lines 31 - 34, Update the installation
documentation around the prebuilt TUI’s extracted core location to refer to the
platform user cache directory followed by catalyst-code, rather than hard-coding
~/.cache/catalyst-code. Preserve the existing note that terminal-only installs
do not place catcode-core on PATH.
Comment on lines
+406
to
+407
| "TUI (catcode, core embedded) -> PATH" \ | ||
| "optional web service (+ catcode-core) via --with-web" \ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the banner conditional for source builds.
When $BUILD_FROM_SOURCE is true, Lines 1566-1570 report $PREFIX/catcode-core as a separate binary. Lines 406-407 still state that the TUI embeds the core and that catcode-core is added only by --with-web. Source users receive conflicting installation information.
Build these lines from $BUILD_FROM_SOURCE.
Proposed fix
local mode="download (prebuilt)"
$BUILD_FROM_SOURCE && mode="build-from-source"
+ local tui_line="TUI (catcode, core embedded) -> PATH"
+ local web_line="optional web service (+ catcode-core) via --with-web"
+ if $BUILD_FROM_SOURCE; then
+ tui_line="TUI (catcode, separate core) -> PATH"
+ web_line="optional web service; catcode-core is installed for source builds"
+ fi
print_box "Catalyst Code — installer v${VERSION_DETECTED}" \
- "TUI (catcode, core embedded) -> PATH" \
- "optional web service (+ catcode-core) via --with-web" \
+ "$tui_line" \
+ "$web_line" \🤖 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 `@install.sh` around lines 406 - 407, Make the banner entries near the
TUI/web-service descriptions conditional on BUILD_FROM_SOURCE: for source
builds, report catcode-core as a separate binary consistent with the later
installation summary; otherwise retain the existing embedded-core and --with-web
wording. Update only the banner construction in install.sh.
Comment on lines
+1621
to
+1634
| local core_line | ||
| if [[ "${WEB_INSTALLED:-no}" == yes ]] || $BUILD_FROM_SOURCE || $WITH_WEB; then | ||
| core_line="core: $PREFIX/catcode-core" | ||
| else | ||
| core_line="core: embedded in TUI (extracted on first run)" | ||
| fi | ||
| local box_lines=( | ||
| "tui: $PREFIX/catcode" | ||
| "$core_line" | ||
| "web: $web_line" | ||
| ) | ||
| [[ -n "$expose_line" ]] && box_lines+=("$expose_line") | ||
| box_lines+=("source: ${METHOD:-download} @ ${BASE_URL:-${REPO_DIR:-<unknown>}}") | ||
| print_box "✓ Updated ${APP_NAME} v${VERSION_DETECTED}" "${box_lines[@]}" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 8 \
'summary_update|WEB_INSTALLED|BUILD_FROM_SOURCE|WITH_WEB|METHOD|load.*state|state.*load' \
install.shRepository: catalystctl/catcode
Length of output: 26579
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
sed -n '1007,1345p' install.sh
printf '\n--- action dispatch ---\n'
sed -n '1348,1560p' install.shRepository: catalystctl/catcode
Length of output: 18035
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Read-only behavioral probe: derive summary_update core_line for representative
# source-install CLI-only state and an update run without explicit --build-from-source.
python3 - <<'PY'
base = {"METHOD": "source", "REPO_DIR": "/tmp/catalyst-code", "VERSION": "1.2.3"}
for cli_with_web, cli_build_from_source in [(False, False), (False, True), (True, False), (True, True)]:
state = dict(base)
state["WEB_INSTALLED"] = "yes" if cli_with_web else "no"
# simulate the update dispatch flags before do_update_source (from do_update)
if state["WEB_INSTALLED"] == "yes":
with_web = True
else:
with_web = cli_with_web
build_from_source = state.get("METHOD") == "source" and cli_build_from_source
core_line = "$PREFIX/catcode-core" if state["WEB_INSTALLED"] == "yes" or build_from_source or with_web else "embedded in TUI (extracted on first run)"
lines = [f"cli_with_web={cli_with_web}, cli_build_from_source={cli_build_from_source} ->"]
lines.append(f" state.METHOD={state['METHOD']}, state.WEB_INSTALLED={state['WEB_INSTALLED']}")
lines.append(f" effective with_web={with_web}, build_from_source={build_from_source}")
lines.append(f" core_line={core_line}")
print("\n".join(lines))
PYRepository: catalystctl/catcode
Length of output: 899
Restore METHOD while loading installer state.
For a source-installed CLI-only install, load_state() reads METHOD=source, but do_update() only forces WITH_WEB=true; $BUILD_FROM_SOURCE stays false. That makes both summary_update() and summary_reinstall() call do_update_source() while the final summary still reports embedded in TUI, even though save_state() rewrites METHOD=source. Load/set the source install flag from the state before dispatching source updates/reinstalls.
🤖 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 `@install.sh` around lines 1621 - 1634, Restore the source-install state in
load_state() by setting BUILD_FROM_SOURCE when the persisted METHOD is source,
before do_update() or do_update_source() dispatches. Ensure source CLI-only
updates and reinstalls follow the source path and
summary_update()/summary_reinstall() report the external catcode-core location
consistently with save_state().
Comment on lines
+1113
to
+1125
| const refreshModels = useCallback(async () => { | ||
| // Optimistic spinner; cleared by the terminal `models_refreshed` event | ||
| // (or immediately if the post fails before core sees the command). | ||
| setState((s) => reduce(s, { type: "_set_models_refreshing", refreshing: true })); | ||
| try { | ||
| const ok = await send({ type: "refresh_models" }); | ||
| if (!ok) { | ||
| setState((s) => reduce(s, { type: "_set_models_refreshing", refreshing: false })); | ||
| } | ||
| } catch { | ||
| setState((s) => reduce(s, { type: "_set_models_refreshing", refreshing: false })); | ||
| } | ||
| }, [send]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Map the core command dispatcher before inspecting refresh handling.
ast-grep outline core/src/commands/dispatcher.rs --items all
# Inspect refresh command success and failure emissions.
rg -n -C 8 '\b(refresh_models|models_refreshed)\b|\berror\b' core web/src/lib
# Inspect current reducer and hook test coverage for terminal failure behavior.
rg -n -C 6 'modelsRefreshing|models_refreshed|refreshModels|type: "error"' \
web/src/lib/reducer.ts web/src/lib/reducer.test.ts web/src/lib/use-agent.tsRepository: catalystctl/catcode
Length of output: 50376
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== locate refresh command handlers/usages =="
rg -n --max-count 200 '\brefresh_models\b|models_refreshed|send\(.*refresh' \
core/src web/src/lib web/src
echo
echo "== relevant dispatcher outline and snippets =="
if [ -f core/src/commands/dispatcher.rs ]; then
ast-grep outline core/src/commands/dispatcher.rs --items all | sed -n '1,220p'
echo "-- refresh snippets --"
rg -n -C 12 '\brefresh_models\b|\bmodels_refreshed\b' core/src/commands/dispatcher.rs
fi
echo
echo "== relevant web state/hook snippets =="
echo "-- reducer =="
rg -n -C 10 'modelsRefreshing|_set_models_refreshing|models_refreshed' web/src/lib/reducer.ts
echo "-- use-agent =="
sed -n '1060,1155p' web/src/lib/use-agent.ts
echo
echo "-- tests mentioning refresh_models/models_refreshed/error =="
rg -n -C 8 'refresh_models|modelsRefreshed|models_refreshed|modelsRefreshing|type: "error"' \
web/src/lib web/src || true
echo
echo "== command outcome helpers around refresh_models =="
rg -n -C 8 'refresh_models|Outcome::|\.ok|success|failed' core/src/commands core/src/lib.rs core/src -g '*.rs' | sed -n '1,260p'Repository: catalystctl/catcode
Length of output: 50376
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== refresh core implementation =="
sed -n '1060,1185p' core/src/main.rs
echo
echo "== refresh protocol command definition =="
sed -n '390,430p' core/src/protocol/commands.rs
echo
echo "== exact refresh_methods/force implementation details =="
rg -n -C 15 'async fn .*refresh_models|refresh_models_force|refresh_models\(&self|pub \(async\) fn discover_models_force_refresh|emits|Event::new\("models_refreshed"\)|Event::new\("error"\)' core/src/main.rs core/src -g '*.rs' | sed -n '1,260p'
echo
echo "== web command send implementation around send/error handling =="
sed -n '520,555p' web/src/lib/use-agent.ts
echo
echo "== focused core refresh emit/error references =="
python3 - <<'PY'
from pathlib import Path
for p in ["core/src/main.rs","core/src/commands/dispatcher.rs","web/src/lib/reducer.test.ts","web/src/lib/use-agent.ts"]:
text=Path(p).read_text()
print(f"-- {p} --")
for i, line in enumerate(text.splitlines(),1):
if any(tok in line for tok in ["refresh_models", "models_refreshed", "Event::new(\"models_refreshed\")", "Event::new(\"error\")", "models_refreshed")):
print(f"{i}: {line}")
PYRepository: catalystctl/catcode
Length of output: 26202
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== model discovery force implementation =="
rg -n -C 25 'pub async fn (discover|aggregate_models_for)|discover_models_force_refresh|refresh_model|cached|models\.dev|http|reqwest|models_refreshed' core/src -g '*.rs' | sed -n '1,260p'
echo
echo "== refresh_models_force direct call sites =="
rg -n -C 8 'refresh_models_force|RefreshModels =>' core/src -g '*.rs'
echo
echo "== focused Rust line references =="
grep -n -- 'refresh_models_force\|models_refreshed\|Event::new("error")\|Outcome::err\|fetch_models' core/src/main.rs core/src/commands/dispatcher.rs -C 4 | sed -n '1,220p'
echo
echo "== web command send implementation around send/error handling =="
sed -n '520,555p' web/src/lib/use-agent.ts
echo
echo "== focused web test/type references =="
grep -n -- 'models_refreshed\|refresh_models\|modelsRefreshing\|error' web/src/lib/reducer.test.ts web/src/lib/types.ts -C 4 | sed -n '1,220p'Repository: catalystctl/catcode
Length of output: 25650
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== locate model discovery helpers =="
rg -n -C 20 'discover_models_force_refresh|fn discover_models|aggregate_models_for|ProviderPresets|models\.dev|modelsdev|Reqwest|reqwest|http::|ProviderModel|Outcome' core/src -g '*.rs' | sed -n '1,340p'
echo
echo "== focused panic handling in refresh_models_force =="
sed -n '1120,1155p' core/src/main.rs
echo
echo "== refresh_models_force without surrounding errors =="
sed -n '1129,1155p' core/src/main.rsRepository: catalystctl/catcode
Length of output: 22897
Guard the background refresh against panics.
refresh_models_force always emits models_refreshed after aggregation, but a panic in discovery/enrichment will leave the on-demand refresh_models task stuck without clearing modelsRefreshing. Catch panics around the spawned task and emit/emit a correlated terminal state so the refresh UI is not left enabled/disabled incorrectly.
🤖 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 `@web/src/lib/use-agent.ts` around lines 1113 - 1125, Update the refresh_models
background task handling, including the spawned task that performs
discovery/enrichment, to catch panics and always emit a correlated terminal
models_refreshed state when the task fails. Preserve the existing optimistic
refresh state flow in refreshModels, ensuring modelsRefreshing is cleared after
both successful completion and panic-driven failure.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
refresh_models/models_refreshedhandling (optimistic spinner + ModelPicker Refresh) so the CORE_EVENT_TYPES coverage test no longer fails on master.install.shsummary for embedded core and avoid blankprint_boxcontinuations that abort underset -e.catcode-core.Why
Master CI web job failed on
0a95f73because SDK catalogsmodels_refreshedbut the web reducer had no case. This branch adds the handler + UI.Test plan
bash -n install.shnode scripts/check-protocol-schema.mjs+ architecture checkcd web && bun run typecheck && bun run lint && bun test && next buildcd core && cargo fmt --check && cargo clippy --all-targets && cargo test --lockedcd tui && gofmt/vet/test/buildSummary by CodeRabbit
New Features
Bug Fixes
Documentation