Skip to content

Remove Config Errors from TabBar, move to Settings / WaveConfig#3060

Merged
sawka merged 3 commits intomainfrom
sawka/update-config-error
Mar 14, 2026
Merged

Remove Config Errors from TabBar, move to Settings / WaveConfig#3060
sawka merged 3 commits intomainfrom
sawka/update-config-error

Conversation

@sawka
Copy link
Member

@sawka sawka commented Mar 14, 2026

No description provided.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 14, 2026

Walkthrough

The changes refactor configuration error handling across multiple components. The TabBar component removes ConfigErrorIcon and ConfigErrorMessage exports along with related error logic. A new configErrorFilesAtom is added to WaveConfigViewModel to track files with configuration errors. The WaveConfigView component introduces error icons next to config file names and displays a persistent error panel. The Widgets component reads a hasConfigErrors flag from its environment and propagates it to UI elements including the settings button and floating window menu. A new preview file adds interactive controls for testing configuration error and development mode states.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive No description was provided by the author, making it impossible to assess relevance to the changeset. Add a pull request description explaining the rationale for moving config error handling and any relevant implementation details.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: removing config error components from TabBar and relocating error handling to Settings and WaveConfig views.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sawka/update-config-error
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Mar 14, 2026

Deploying waveterm with  Cloudflare Pages  Cloudflare Pages

Latest commit: 37f3443
Status: ✅  Deploy successful!
Preview URL: https://f19827cd.waveterm.pages.dev
Branch Preview URL: https://sawka-update-config-error.waveterm.pages.dev

View logs

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
frontend/app/view/waveconfig/waveconfig.tsx (1)

301-310: Consider using a stable key for error items.

Using array index as a React key is acceptable here since the error list is derived from atom state and re-renders fully on changes. However, if error order could change without the array length changing, consider using cerr.file or a composite key.

💡 Optional: Use file path as key
-                    {configErrors.map((cerr, i) => (
-                        <div key={i} className="text-sm">
+                    {configErrors.map((cerr) => (
+                        <div key={cerr.file} className="text-sm">
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/app/view/waveconfig/waveconfig.tsx` around lines 301 - 310, The
current list rendering in waveconfig.tsx uses the array index as the React key
for configErrors which can cause unstable identity when order changes; update
the key in the map inside the JSX that renders configErrors (the map using
configErrors.map((cerr, i) => ...)) to use a stable identifier such as cerr.file
or a composite like `${cerr.file}-${cerr.err}` instead of i so React can track
items reliably during reorders or updates.
frontend/preview/previews/widgets.preview.tsx (1)

121-127: Stale environment reference may not reflect atom updates.

The envRef.current is only set once when null, so changes to hasConfigErrorsAtom will be reflected since it's passed by reference. However, changes to isDev prop won't update the environment. The mockVersion key on the parent container forces a full re-mount, which works around this, but it's worth noting this coupling.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/preview/previews/widgets.preview.tsx` around lines 121 - 127,
envRef.current is initialized once and won't pick up changes to props like isDev
(even though hasConfigErrorsAtom updates by reference); update the environment
whenever relevant inputs change by recreating it via useMemo or in a useEffect
that calls makeWidgetsEnv and assigns envRef.current when dependencies change
(include baseEnv, isDev, hasCustomAIPresets, apps, and hasConfigErrorsAtom as
deps). Locate the envRef, useWaveEnv, and makeWidgetsEnv usage in
widgets.preview.tsx and ensure the ref is updated on those dependency changes so
the environment reflects new prop values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@frontend/app/view/waveconfig/waveconfig.tsx`:
- Around line 301-310: The current list rendering in waveconfig.tsx uses the
array index as the React key for configErrors which can cause unstable identity
when order changes; update the key in the map inside the JSX that renders
configErrors (the map using configErrors.map((cerr, i) => ...)) to use a stable
identifier such as cerr.file or a composite like `${cerr.file}-${cerr.err}`
instead of i so React can track items reliably during reorders or updates.

In `@frontend/preview/previews/widgets.preview.tsx`:
- Around line 121-127: envRef.current is initialized once and won't pick up
changes to props like isDev (even though hasConfigErrorsAtom updates by
reference); update the environment whenever relevant inputs change by recreating
it via useMemo or in a useEffect that calls makeWidgetsEnv and assigns
envRef.current when dependencies change (include baseEnv, isDev,
hasCustomAIPresets, apps, and hasConfigErrorsAtom as deps). Locate the envRef,
useWaveEnv, and makeWidgetsEnv usage in widgets.preview.tsx and ensure the ref
is updated on those dependency changes so the environment reflects new prop
values.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 469bb236-0a15-4103-a06b-6c7723dbe1f4

📥 Commits

Reviewing files that changed from the base of the PR and between ac6b2f3 and 37f3443.

📒 Files selected for processing (5)
  • frontend/app/tab/tabbar.tsx
  • frontend/app/view/waveconfig/waveconfig-model.ts
  • frontend/app/view/waveconfig/waveconfig.tsx
  • frontend/app/workspace/widgets.tsx
  • frontend/preview/previews/widgets.preview.tsx

@sawka sawka merged commit a2e6f7b into main Mar 14, 2026
8 checks passed
@sawka sawka deleted the sawka/update-config-error branch March 14, 2026 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant