NOT READY : Enforce frontend codestyle rulebook across iron_dashboard#65
NOT READY : Enforce frontend codestyle rulebook across iron_dashboard#65Colard wants to merge 12 commits into
Conversation
There was a problem hiding this comment.
System of Review completed analysis of 'Enforce frontend codestyle rulebook across iron_dashboard'. Found 15 findings across 7 categories.
📋 Review Summary
| Category | Count |
|---|---|
| 🏛️ ORGANIZATIONAL | 1 |
| 🐛 CORRECTNESS | 2 |
| 🛡️ SECURITY | 1 |
| 📐 DESIGN | 5 |
| 🔍 QUALITY | 4 |
| ✅ TESTING | 1 |
| 📖 DOCUMENTATION | 1 |
🏛️ ORGANIZATIONAL
Missing YAML frontmatter on rulebook
codestyle.frontend.rulebook.md starts directly with # Frontend Codestyle at line 1. The other local rulebook with the most mature structure (documentation_management.rulebook.md) has proper YAML frontmatter with status, version, authority, applicability, precedence fields. Without frontmatter, kbase .rulebooks may not discover or properly classify this rulebook.
Evidence:
Line 1: # Frontend Codestyle
Compare with documentation_management.rulebook.md:
---
status: active
version: 1.0
authority: iron_runtime
applicability: documentation
precedence: project_specific
---Action:
- Add YAML frontmatter block to
codestyle.frontend.rulebook.mdmatching the established pattern
🐛 CORRECTNESS
45 files fail Prettier format check
The PR introduces a rulebook that designates Prettier as "the single source of truth for formatting decisions" and claims to have "Ran Prettier on all UI primitive .vue and .ts files." However, running npx prettier --check 'src/**/*.{ts,vue}' against the current branch shows 45 files with formatting violations. The compliance pass was applied only to UI primitives — views, composables, stores, and the router were not formatted.
Evidence:
$ npx prettier --check 'src/**/*.{ts,vue}'
# Exit code: 1
# 45 files with violations across views/, composables/, stores/, lib/, components/Action:
- Run
npx prettier --write 'src/**/*.{ts,vue}'to format all files, or scope the PR description to clarify that only UI primitives were formatted (and track full formatting as a follow-up)
PR description test count is stale
The test plan claims "29/29 tests pass (3 test files)" but the actual suite runs 56 tests across 5 test files (formatters: 12, providers: 24, useConfirm: 6, auth: 11, useApi: 3). All 56 pass.
Evidence:
$ npx vitest run
# ✓ formatters.test.ts (12 tests)
# ✓ providers.test.ts (24 tests)
# ✓ useConfirm.test.ts (6 tests)
# ✓ auth.test.ts (11 tests)
# ✓ useApi.test.ts (3 tests)
# Test Files: 5 passed | Tests: 56 passedAction:
- Update PR description to reflect "56/56 tests pass (5 test files)"
🛡️ SECURITY
No v-html prohibition or XSS prevention rules
The rulebook has zero rules about v-html (Vue's primary XSS vector), input sanitization, or Content Security Policy. For a control panel that manages LLM provider keys and authentication tokens, a missing v-html prohibition is a significant gap. The auth store already has a SECURITY NOTE comment about localStorage XSS — the rulebook should codify this awareness.
Evidence:
$ grep -c 'v-html' codestyle.frontend.rulebook.md
0Action:
- Add a Security section to the rulebook with at minimum:
v-htmlprohibition (or require DOMPurify sanitization),Content-Type: application/jsonenforcement in API layer, and a note that client-side validation is UX-only (server is authoritative)
📐 DESIGN
utils.ts filename sanctioned despite global naming prohibition
The rulebook canonizes utils.ts at line 322 and in the directory layout at line 1268. The global CLAUDE.md Rule 3 and files_structure.rulebook.md explicitly prohibit utils.*, helpers.*, common.*, misc.* filenames as they violate the Unique Responsibility Principle. The file contains only the cn() class-merge utility — its responsibility is specific enough for a specific filename.
Evidence:
codestyle.frontend.rulebook.md line 322:
| Utilities / helpers | lowercase | `utils.ts`, `formatters.ts`, `providers.ts` |
codestyle.frontend.rulebook.md line 1268:
├── lib/
│ ├── utils.ts # cn() utility
Action:
- Either rename to
class_merge.tsorcn.ts, or add an explicit cross-rulebook exception paragraph documenting why the global prohibition does not apply here (shadcn-vue convention, single-purpose file)
Mocking sanctioned without documenting the global no-mock exception
The testing section at lines 1171-1187 shows vi.fn() and vi.stubGlobal('fetch', mockFetch) examples. The global CLAUDE.md, test_organization.rulebook.md, and iron_runtime.rulebook.md all forbid mocking. Frontend testing in jsdom legitimately requires fetch stubs — but this deviation needs explicit documentation.
Evidence:
// codestyle.frontend.rulebook.md line 1186-1187
const mockFetch = vi.fn()
vi.stubGlobal('fetch', mockFetch)Action:
- Add an exception paragraph to the Testing section: "The global no-mocking policy applies to Rust backend code. Frontend tests in jsdom require these permitted stubs: (1)
vi.stubGlobal('fetch', ...), (2)vi.fn()for event handler verification, (3)vi.stubGlobal('localStorage', ...). Mocking Vue component internals or Pinia store actions is FORBIDDEN."
PascalCase file naming not cross-referenced with global lowercase_snake_case rule
The rulebook mandates PascalCase for .vue files (line 319) and strictly forbids snake_case for directories (line 341). This is correct for the Vue ecosystem but directly contradicts CLAUDE.md Rule 3 (lowercase_snake_case for project files). The rulebook does not acknowledge this as an intentional ecosystem exception.
Evidence:
codestyle.frontend.rulebook.md line 319:
| Vue components | PascalCase | `MainLayout.vue`, `StatCard.vue` |
codestyle.frontend.rulebook.md line 341:
It is **strictly forbidden** to use PascalCase, camelCase, or snake_case for directory names.
Action:
- Add a cross-reference note: "Vue file naming conventions differ from the project-wide
lowercase_snake_casestandard. This is a tooling/ecosystem exception per CLAUDE.md Rule 3 ('when possible')."
No accessibility rules for custom components
The UI Primitives section (lines 978-986) mentions Reka UI provides accessible primitives, but the rulebook has zero rules about ARIA attributes, keyboard navigation, focus management, or color contrast for custom interactive elements not built on Reka UI.
Evidence:
$ grep -ciE 'aria|keyboard|focus.management|a11y|accessibility|screen.reader' codestyle.frontend.rulebook.md
0Action:
- Add an Accessibility section with: (1) keyboard accessibility for all interactive elements, (2) ARIA attributes for custom components, (3) focus management on route transitions, (4)
altattributes on images
No global error boundary rule
The Loading/Error/Empty pattern (lines 278-309) covers per-component query errors but not unhandled exceptions. No rule about app.config.errorHandler, onErrorCaptured, or window.addEventListener('unhandledrejection', ...).
Action:
- Add a rule mandating a global error handler via
app.config.errorHandlerand an unhandled rejection listener
🔍 QUALITY
Router lazy imports use ../ relative paths instead of @/
The rulebook mandates @/ for all cross-directory imports. All 6 lazy-loaded route imports in router/index.ts use ../views/ relative paths.
Evidence:
// router/index.ts lines 21, 27, 33, 39, 50, 56
component: () => import('../views/DashboardView.vue')
component: () => import('../views/AgentsView.vue')
component: () => import('../views/UsageView.vue')
component: () => import('../views/BudgetsView.vue')
component: () => import('../views/ProvidersView.vue')
component: () => import('../views/UsersView.vue')Action:
- Replace all 6 with
@/views/...paths
IconPlus and IconX inconsistently formatted vs other 9 icons
These two icons got the SFC reorder (template→script) but NOT the Prettier formatting pass that the other 9 icons received. They lack trailing commas in defineOptions and use single-line SVG attributes.
Evidence:
<!-- IconPlus.vue line 3 — missing trailing comma -->
defineOptions({ inheritAttrs: false })
<!-- vs IconBan.vue line 3 — has trailing comma -->
defineOptions({
inheritAttrs: false,
})Action:
- Run Prettier on
IconPlus.vueandIconX.vueto match the other 9 icons
Boolean ref naming violations
The rulebook requires is/has/show/should prefix for boolean refs.
Evidence:
// LoginView.vue:18
const loading = ref(false) // should be: isLoading
// UsageView.vue:224, UsersView.vue:71
const mobileFiltersOpen = ref(false) // should be: showMobileFiltersAction:
- Rename
loadingtoisLoading,mobileFiltersOpentoshowMobileFilters
Event handler functions missing handle prefix
Evidence:
// AgentsView.vue:242
function openUpdateModal(agent: Agent) { ... } // → handleOpenUpdateModal
// AgentsView.vue:392
function copyTokenToClipboard() { ... } // → handleCopyTokenToClipboard
// ProvidersView.vue:230
function closeQuickAdd() { ... } // → handleCloseQuickAddAction:
- Add
handleprefix to these 3 functions and update their call sites
✅ TESTING
6 mutations lack toast.success() feedback on success
The state management rules mandate user feedback via toast notifications for mutations. Several mutations only show error toasts but no success feedback.
Evidence:
AgentsView.vue:166 createMutation.onSuccess — no success toast
AgentsView.vue:193 updateMutation.onSuccess — no success toast
BudgetsView.vue:66 updateBudgetMutation.onSuccess — no success toast
ProvidersView.vue:82 createMutation.onSuccess — no success toast
ProvidersView.vue:91 quickAddMutation.onSuccess — no success toast
UsersView.vue:128 createMutation.onSuccess — no success toast
Action:
- Add
toast.success('...')calls to each mutation'sonSuccesscallback
📖 DOCUMENTATION
Import order rule uses "should" instead of "must"
Every other rule in the rulebook uses "must" for mandatory requirements. The import ordering rule at line 459 uses "should", creating ambiguity about enforcement.
Evidence:
codestyle.frontend.rulebook.md line 459:
Imports **should** be organized in the following order
Action:
- Change "should" to "must" for consistency with all other rules
This is a strong contribution — the rulebook is well-structured with clear description/rationale/examples for all 29 rules. The codebase compliance pass covers the right areas (SFC order, emit syntax, cn() migration, design tokens, useMutation refactor). The main gap is that the compliance enforcement is partial: Prettier wasn't run on all files, relative imports remain in the router, and several naming convention rules from the rulebook itself aren't fully applied to the codebase. A second pass focusing on Prettier + path aliases + naming should close everything out.
❗ > Note: This System of Review operates with the same information access and constraints as any human developer working on this repository. While the system is highly accurate (99%+ reliability), any confusion or apparent misunderstanding typically indicates gaps in repository documentation, unclear code organization, or insufficient project discipline — the same issues that would impact any team member. If the System cannot locate critical information and draws incorrect conclusions, a human developer would face identical challenges. We maintain high standards to respect our teammates' time, our clients' investment, and the integrity of this project. Please ensure the repository provides clear, discoverable context for all reviewers.
Review approval is a quality floor, not a quality ceiling. It confirms that identified issues are resolved — not that the code is free of all issues. The System reviews the changes in this PR, not the entire codebase. Proactive quality is a developer responsibility: unit test coverage, edge case analysis, and consistency with the surrounding codebase cannot be delegated to any review process. If the review surfaces one instance of a pattern, the developer is responsible for auditing all occurrences of that pattern throughout the codebase — not just the specific line cited.
When addressing review feedback: Open a separate commit for each point you address, clearly referencing the finding in the commit message. This ensures knowledge is captured in the repository history and helps other developers avoid the same pitfalls. If you cannot address a point, leave a detailed comment in this PR review thread explaining specifically what is wrong with the finding or why it cannot be addressed — never ignore feedback silently. Use comments only in rare cases when the System has genuinely missed existing context in the repository; prefer commits as the primary response mechanism to build institutional knowledge.
wanguardd
changed the title
Fixes — 2026-04-08Fix #1 — Missing YAML frontmatter on rulebookFile(s): Fix #2 — 45 files fail Prettier format checkFile(s): All Fix #3 — No v-html prohibition or XSS prevention rulesFile(s): Fix #4 — utils.ts filename sanctioned despite global naming prohibitionFile(s): Fix #5 — Mocking sanctioned without documenting the global no-mock exceptionFile(s): Fix #6 — PascalCase file naming not cross-referenced with global ruleFile(s): Fix #7 — No accessibility rules for custom componentsFile(s): Fix #8 — No global error boundary ruleFile(s): Fix #9 — Router lazy imports use relative paths instead of @/File(s): Fix #10 — IconPlus and IconX inconsistently formattedFile(s): Fix #11 — Boolean ref naming violationsFile(s): Fix #12 — Event handler functions missing handle prefixFile(s): Fix #13 — 6 mutations lack toast.success() feedback on successFile(s): Fix #14 — Import order rule uses "should" instead of "must"File(s): |
- Add YAML frontmatter, Security, Accessibility sections to rulebook - Add cross-rulebook exception notes (utils.ts, PascalCase, mocking) - Change import order "should" to "must" for consistency - Run Prettier on all frontend source files (45 files formatted) - Fix router imports to use @/ alias instead of relative paths - Rename boolean refs to use is/has/show prefix convention - Add handle prefix to event handler functions - Add toast.success() to 6 mutations missing success feedback
Colard
changed the title
There was a problem hiding this comment.
System of Review completed analysis of 'Enforce frontend codestyle rulebook across iron_dashboard'. Found 6 findings across 3 categories.
📊 Progress Summary
All 15 findings from Review #1 have been resolved. The fixes are thorough and correctly applied:
- ✅ YAML frontmatter added to
codestyle.frontend.rulebook.md - ✅ Prettier now passes for all 47
src/**/*.{ts,vue}files (All matched files use Prettier code style!) - ✅ PR description test count updated to
56/56 tests pass (5 test files) - ✅
v-htmlProhibition and XSS Prevention section added (rulebook line 1322) - ✅
utils.tsnaming exception documented with cross-rulebook rationale (rulebook line 1318) - ✅ Frontend mock exception paragraph added to Testing section (rulebook line 1194)
- ✅ PascalCase cross-reference note added (
Vue file naming conventions differ from...at line 343) - ✅ Accessibility section added with ARIA requirements (rulebook line 1343)
- ✅ Global error boundary rule added (rulebook line 1337)
- ✅ All 6 router lazy imports updated to
@/views/... - ✅
IconPlus.vueandIconX.vuePrettier-formatted to match the rest - ✅
loading→isLoading,mobileFiltersOpen→showMobileFilters - ✅
handleOpenUpdateModal,handleCopyTokenToClipboard,handleCloseQuickAddnaming fixed - ✅
toast.success()added to all 6 mutations that were missing it - ✅ Import order rule updated from "should" to "must"
📋 Review Summary
| Category | Count |
|---|---|
| 🏛️ ORGANIZATIONAL | 3 |
| 🐛 CORRECTNESS | 1 |
| 🔍 QUALITY | 2 |
🏛️ ORGANIZATIONAL
Missing Responsibility Table — src/views/
module/iron_dashboard/src/views/ contains 7 .vue files (AgentsView, BudgetsView, DashboardView, LoginView, ProvidersView, UsageView, UsersView). This PR touches all 7. No readme.md exists.
Evidence:
find module/iron_dashboard/src/views -name "readme.md"
# → (no output)
ls module/iron_dashboard/src/views/
# → 7 .vue files, no readme.mdAction:
- Create
module/iron_dashboard/src/views/readme.mdwith a Responsibility Table. Each row:| ViewName.vue | One-sentence responsibility |.
Missing Responsibility Table — src/components/
module/iron_dashboard/src/components/ contains 9 .vue files and 3 subdirectories (cards/, icons/, ui/). This PR touches 8 of the 9 component files. No readme.md exists.
Evidence:
find module/iron_dashboard/src/components -maxdepth 1 -name "readme.md"
# → (no output)Action:
- Create
module/iron_dashboard/src/components/readme.mdwith a Responsibility Table covering each component file and each subdirectory.
Missing Responsibility Table — src/components/ui/
module/iron_dashboard/src/components/ui/ contains 17 subdirectories (alert, badge, button, card, dialog, dropdown-menu, input, label, popover, scroll-area, select, separator, skeleton, sonner, switch, tabs, textarea). This PR modifies primitives in this directory. No readme.md exists.
Evidence:
find module/iron_dashboard/src/components/ui -maxdepth 1 -name "readme.md"
# → (no output)
ls module/iron_dashboard/src/components/ui/
# → 17 subdirectories, no readme.mdAction:
- Create
module/iron_dashboard/src/components/ui/readme.mdwith a Responsibility Table listing each UI primitive category and its purpose (e.g.,| dialog/ | Accessible dialog primitives wrapping Reka UI |).
🐛 CORRECTNESS
handleCopyText is duplicated and silently fails when clipboard API is absent
The same function is defined identically in two view files. Both copies use ?.then()?.catch() chaining: when navigator.clipboard is null or undefined (non-HTTPS contexts, certain browsers), the entire optional chain short-circuits to undefined and neither the success nor the error toast fires. The user clicks copy, nothing visually happens, and there is no feedback.
This is inconsistent with handleCopyTokenToClipboard in the same AgentsView.vue, which correctly uses try/catch and provides feedback in both success and failure cases.
Evidence:
// AgentsView.vue:425–430 (identical to ProvidersView.vue:309–314)
function handleCopyText(text: string, label = 'Copied') {
navigator.clipboard
?.writeText(text)
?.then(() => toast.success(label))
?.catch(() => toast.error('Copy failed'))
// ↑ If navigator.clipboard is absent: ?.writeText → undefined,
// then undefined?.then → undefined, undefined?.catch → undefined.
// No toast fires.
}
// AgentsView.vue:432–442 — correct pattern in the same file
async function handleCopyTokenToClipboard() {
try {
await navigator.clipboard?.writeText(tokenDialogValue.value)
copyMessage.value = 'Copied to clipboard'
} catch (_err: unknown) {
const message = _err instanceof Error ? _err.message : 'Copy failed'
copyMessage.value = message
}
}Action:
- Extract
handleCopyTextinto a shared composable (e.g.,src/composables/useClipboard.ts) to eliminate the duplication — one function, one fix point. - In the extracted composable, guard against absent clipboard explicitly:
if (!navigator.clipboard) { toast.error('Copy not supported'); return }, then use.then()/.catch()ortry/catchon the non-null clipboard.
🔍 QUALITY
createMutation and quickAddMutation in ProvidersView.vue missing onError handlers
Both creation mutations have onSuccess callbacks but no onError. Network failures and server errors silently fail — no toast, no user notification. The sibling updateMutation (line 141) and deleteMutation (line 152) both have correct onError handlers, making these two mutations inconsistent.
Evidence:
// ProvidersView.vue:97–119
const createMutation = useMutation({
mutationFn: (data) => api.createProviderKey(data),
onSuccess: () => {
queryClient.invalidateQueries({ queryKey: ['providerKeys'] })
toast.success('Provider key created successfully')
},
// ← no onError
})
const quickAddMutation = useMutation({
mutationFn: (data) => api.createProviderKey(data),
onSuccess: () => {
queryClient.invalidateQueries({ queryKey: ['providerKeys'] })
toast.success('Provider key added successfully')
},
// ← no onError
})
// Compare — updateMutation at line 141 has correct onError:
onError: (err) => {
toast.error(err instanceof Error ? err.message : 'Failed to update provider key')
},Action:
- Add
onError: (err) => { toast.error(err instanceof Error ? err.message : 'Failed to create provider key') }tocreateMutation. - Add
onError: (err) => { toast.error(err instanceof Error ? err.message : 'Failed to add provider key') }toquickAddMutation.
PR description still claims "99 files changed" — actual is 47
The description body says "Codebase fixes (99 files changed)" but the actual diff is 47 files. The test count was correctly updated to 56/56, but the file count was not updated when the branch was squashed.
Evidence:
git diff --stat reafactor/frontend...docs/frontend-rulebook | tail -1
# → 47 files changed, 1491 insertions(+), 646 deletions(-)Action:
- Update the PR description: change "99 files changed" to "47 files changed".
Strong iteration — 15/15 prior findings resolved, tests pass (56/56), Prettier passes on all 47 source files. The 3 organizational items are small readme.md files to create; the clipboard duplication and the missing onError handlers are the only substantive code changes needed.
wanguardd
changed the title
Fixes — 2026-04-13Fix #1 — Responsibility tables for views, components, and uiFile(s):
Root cause: Three directories touched by the PR lacked the responsibility table that What changed: Added one readme per directory. Each lists every Also fixed in: n/a Risk: None — documentation only, no code paths affected. Fix #2 —
|
Colard
changed the title
wanguardd
left a comment
wanguardd
left a comment
There was a problem hiding this comment.
System of Review completed analysis of 'Enforce frontend codestyle rulebook across iron_dashboard'. Found 8 findings across 4 categories.
📊 Progress Summary
All 6 findings from Review #2 have been resolved. The fixes are correct and thorough:
- ✅
src/views/readme.mdcreated with 7-row Responsibility Table - ✅
src/components/readme.mdcreated with 12-row Responsibility Table - ✅
src/components/ui/readme.mdcreated with 17-row Responsibility Table - ✅
handleCopyTextextracted touseClipboard.tscomposable with explicit null guard and proper error feedback - ✅
onErroradded tocreateMutationandquickAddMutationinProvidersView.vue - ✅ PR description file count updated
📋 Review Summary
| Category | Count |
|---|---|
| 🏛️ ORGANIZATIONAL | 1 |
| 🐛 CORRECTNESS | 2 |
| 📐 DESIGN | 3 |
| 🔍 QUALITY | 2 |
🏛️ ORGANIZATIONAL
**Description:** attribute label used 40 times in codestyle.frontend.rulebook.md
Every rule block in the rulebook opens with **Description:** <text>. The **Description:** label is a semantically vacuous attribute — it adds no information signal since the content following it is already the description by context. The governance standard explicitly prohibits this label and provides alternatives: **Purpose:**, **Governs:**, **Rationale:**, or no label (when the body text is the statement).
Evidence:
grep -c '^\*\*Description:\*\* ' codestyle.frontend.rulebook.md
# → 40Example at line 103:
**Description:** Prettier is the **sole** authority for code formatting.Should be either:
Prettier is the **sole** authority for code formatting.or, if a label is needed:
**Purpose:** Establish Prettier as the sole formatting authority.Action:
- Remove all 40
**Description:**attribute labels. For each rule block, either drop the label entirely (body text already communicates the description) or replace with a semantically meaningful label (**Purpose:**,**Governs:**,**Rationale:**).
🐛 CORRECTNESS
useApi.test.ts uses mocking patterns the rulebook explicitly forbids
The rulebook's Testing section (added in the previous iteration) documents a narrow frontend mock exception and then states: "Mocking Vue component internals, Pinia store actions, or Vue Router navigation is strictly forbidden." The existing useApi.test.ts violates both halves of this prohibition — it mocks Vue Router at module level and spies on Pinia store actions.
Evidence:
// useApi.test.ts:5-6 — Vue Router module mock (explicitly forbidden)
const mockReplace = vi.fn()
vi.mock('vue-router', () => ({
useRouter: () => ({ replace: mockReplace }),
}))
// useApi.test.ts:52, 96, 134 — Pinia store action spy (explicitly forbidden)
const logoutSpy = vi.spyOn(authStore, 'logout')
// useApi.test.ts:100 — Pinia store action mock with implementation (explicitly forbidden)
const refreshMock = vi.spyOn(authStore, 'refresh').mockImplementation(async (...))Rulebook line 1194:
Mocking Vue component internals, Pinia store actions, or Vue Router navigation
is strictly forbidden.
Action:
- Rewrite
useApi.test.tstests to removevi.mock('vue-router', ...). Verify redirect behavior by inspecting router state directly (e.g.,router.currentRoute.value.path) rather than spying onreplace. - Remove
vi.spyOn(authStore, 'logout')andvi.spyOn(authStore, 'refresh').mockImplementation(...). Test the 401 deduplication behavior by observing observable outcomes (store state,isAuthenticated,accessToken) after the sequence of fetch calls completes.
useConfirm.ts swallows errors from confirm actions silently
The confirmCallback wrapper catches all errors from the user-confirmed action and logs them to the console, but never surfaces them to the user. When a delete or revoke operation fails inside the confirm dialog, the dialog closes, the loading spinner stops, and the user sees nothing — no toast, no message, no visible indication that anything went wrong.
Evidence:
// useConfirm.ts:22-29
confirmCallback.value = async () => {
confirmCallback.value = null
try {
await action()
} catch (err) {
console.error('Confirm action failed:', err)
// ↑ Error is logged but never surfaced to the user.
// Dialog closes, UI reverts, no feedback given.
}
}Action:
- Re-throw the error after logging so callers can handle it:
console.error('Confirm action failed:', err); throw err. This allows eachonErrorhandler in the calling mutation to fire its toast. Alternatively, accept an optionalonErrorcallback parameter inopenConfirm()and invoke it here.
📐 DESIGN
API_BASE_URL defined identically in two files — Anti-Duplication violation
useApi.ts and auth.ts each independently define API_BASE_URL with the same value. This is a textbook Anti-Duplication violation: the same knowledge exists in two locations, so a URL change requires two edits, with inevitable divergence risk. The auth.ts store also calls fetch() directly instead of routing through fetchApi(), bypassing the centralized error handling, 401 deduplication, and retry logic in useApi.ts.
Evidence:
// composables/useApi.ts:5
const API_BASE_URL = import.meta.env.VITE_API_URL || 'http://localhost:3001'
// stores/auth.ts:34 — identical definition
const API_BASE_URL = import.meta.env.VITE_API_URL || 'http://localhost:3001'Action:
- Export
API_BASE_URLfromuseApi.ts(or define it in a sharedlib/config.ts) and import it inauth.ts. The login and refresh calls inauth.tscannot usefetchApi()(they run before authentication is established), so the URL constant import is the correct fix.
Term collision: "Reka UI" and "Radix Vue" used interchangeably throughout rulebook
The rulebook uses "Reka UI", "Radix Vue", and "Reka UI / Radix Vue" as if they are synonyms — they appear in the vocabulary section, rule descriptions, rationale paragraphs, and the canonical directory layout. Reka UI is the correct canonical name for the Vue-specific port; "Radix Vue" is a predecessor name. Using both in the same document violates the Ubiquitous Language Enforcement rule and creates ambiguity about which library is actually in use.
Evidence:
grep -c 'Radix Vue' codestyle.frontend.rulebook.md
# → 5
grep -c 'Reka UI' codestyle.frontend.rulebook.md
# → 10Lines 16, 997, 1000, 1001, 1003 all use "Reka UI / Radix Vue" as a compound term, implying they differ. The components/ui/readme.md created in the last iteration also inherited this: "Shadcn-vue style Reka UI / Radix Vue wrapper primitives".
Action:
- Choose one canonical term —
Reka UI— and replace all occurrences of "Radix Vue" (standalone and in compound "Reka UI / Radix Vue"). Update the vocabulary definition to note that "Radix Vue" was the previous name. - Update
src/components/ui/readme.mdrow to use only "Reka UI".
Canonical project structure includes mock/ directory — name conflicts with no-mocking policy
The canonical directory layout (line 1302) lists ├── mock/ # Mock data for development. The rulebook's own Testing section states the no-mocking rule applies to "Rust backend code" with a narrow exception list for jsdom stubs. A top-level mock/ directory named "mock" will create confusion: contributors will be unsure whether this is a permitted location for test fixtures, development stubs, or something else. The directory does not currently exist in the codebase.
Evidence:
# codestyle.frontend.rulebook.md:1302
├── mock/ # Mock data for development
# ls module/iron_dashboard/src/mock/
ls: cannot access '.../src/mock/': No such file or directory
Action:
- If the intent is static development fixtures (seed data for local dev), rename the entry to
fixtures/orseed/in the canonical layout to avoid the naming collision with the mocking prohibition. - If no such directory is planned, remove the
mock/entry from the canonical layout entirely.
🔍 QUALITY
deleteMutation and revokeIcTokenMutation in AgentsView.vue have no toast.success() feedback
Both mutations have onError handlers with user-visible toasts but their onSuccess callbacks are silent. Agent deletion and IC token revocation are destructive, irreversible operations — users need confirmation that the action completed. All other mutations in this file (createMutation, updateMutation, generateIcTokenMutation, regenerateIcTokenMutation) correctly show success toasts.
Evidence:
// AgentsView.vue:224-232 — no toast in onSuccess
const deleteMutation = useMutation({
mutationFn: (id: number) => api.deleteAgent(id),
onSuccess: () => {
queryClient.invalidateQueries({ queryKey: ['agents'] })
// ← no toast.success()
},
onError: (err) => { toast.error(...) },
})
// AgentsView.vue:365-384 — no toast in onSuccess
const revokeIcTokenMutation = useMutation({
mutationFn: (agentId: number) => api.revokeIcToken(agentId),
onSuccess: (_data, agentId) => {
queryClient.setQueryData(['ic-token-status', agentId], { ... })
// ← no toast.success()
},
onError: (err) => { toast.error(...) },
onSettled: () => { ... },
})Action:
- Add
toast.success('Agent deleted')todeleteMutation.onSuccess. - Add
toast.success('IC token revoked')torevokeIcTokenMutation.onSuccess.
Multi-statement @click handlers in template — should be named functions
Three @click attributes in AgentsView.vue contain multi-statement inline expressions (5-6 state reset assignments each). The rulebook's naming conventions section requires event handlers to be extracted as named handle* functions. Inline multi-statement handlers are not readable, cannot be tested, and cannot be reused.
Evidence:
<!-- AgentsView.vue:705-713 — 6-statement inline cancel handler -->
@click="
showCreateModal = false
name = ''
selectedProviderKeyIds = []
addingProviderKeyId = ''
initialBudgetUsd = undefined
selectedOwnerId = ''
"
<!-- AgentsView.vue:816-824 — 5-statement inline cancel handler -->
@click="
showUpdateModal = false
name = ''
selectedProviderKeyIds = []
addingProviderKeyId = ''
"
<!-- AgentsView.vue:879-886 — 5-statement inline dialog close handler -->
@click="
showTokenDialog = false
tokenDialogValue = ''
tokenDialogAgentName = ''
tokenDialogWarning = ''
copyMessage = ''
"Action:
- Extract each to a named function:
handleCancelCreate(),handleCancelUpdate(),handleCloseTokenDialog(). Replace inline handler with@click="handleCancelCreate"etc.
Strong iteration — 6/6 prior findings resolved. The remaining 8 items are split between rulebook corrections (the **Description:** labels and term collision) and code fixes (mocking violations, silent errors, missing toasts, inline handlers, duplicated constant). The mocking finding is the highest priority since the test code actively contradicts a rule the PR itself introduced.
❗ > Note: This System of Review operates with the same information access and constraints as any human developer working on this repository. While the system is highly accurate (99%+ reliability), any confusion or apparent misunderstanding typically indicates gaps in repository documentation, unclear code organization, or insufficient project discipline — the same issues that would impact any team member. If the System cannot locate critical information and draws incorrect conclusions, a human developer would face identical challenges. We maintain high standards to respect our teammates' time, our clients' investment, and the integrity of this project. Please ensure the repository provides clear, discoverable context for all reviewers.
Review approval is a quality floor, not a quality ceiling. It confirms that identified issues are resolved — not that the code is free of all issues. The System reviews the changes in this PR, not the entire codebase. Proactive quality is a developer responsibility: unit test coverage, edge case analysis, and consistency with the surrounding codebase cannot be delegated to any review process. If the review surfaces one instance of a pattern, the developer is responsible for auditing all occurrences of that pattern throughout the codebase — not just the specific line cited.
When addressing review feedback: Open a separate commit for each point you address, clearly referencing the finding in the commit message. This ensures knowledge is captured in the repository history and helps other developers avoid the same pitfalls. If you cannot address a point, leave a detailed comment in this PR review thread explaining specifically what is wrong with the finding or why it cannot be addressed — never ignore feedback silently. Use comments only in rare cases when the System has genuinely missed existing context in the repository; prefer commits as the primary response mechanism to build institutional knowledge.
wanguardd
left a comment
wanguardd
left a comment
There was a problem hiding this comment.
System of Review completed analysis of 'Enforce frontend codestyle rulebook across iron_dashboard'. Found 8 findings across 4 categories.
📊 Progress Summary
All 6 findings from Review #2 have been resolved. The fixes are correct and thorough:
- ✅
src/views/readme.mdcreated with 7-row Responsibility Table - ✅
src/components/readme.mdcreated with 12-row Responsibility Table - ✅
src/components/ui/readme.mdcreated with 17-row Responsibility Table - ✅
handleCopyTextextracted touseClipboard.tscomposable with explicit null guard and proper error feedback - ✅
onErroradded tocreateMutationandquickAddMutationinProvidersView.vue - ✅ PR description file count updated
📋 Review Summary
| Category | Count |
|---|---|
| 🏛️ ORGANIZATIONAL | 1 |
| 🐛 CORRECTNESS | 2 |
| 📐 DESIGN | 3 |
| 🔍 QUALITY | 2 |
🏛️ ORGANIZATIONAL
**Description:** attribute label used 40 times in codestyle.frontend.rulebook.md
Every rule block in the rulebook opens with **Description:** <text>. The **Description:** label is a semantically vacuous attribute — it adds no information signal since the content following it is already the description by context. The governance standard explicitly prohibits this label and provides alternatives: **Purpose:**, **Governs:**, **Rationale:**, or no label (when the body text is the statement).
Evidence:
grep -c '^\*\*Description:\*\* ' codestyle.frontend.rulebook.md
# → 40Example at line 103:
**Description:** Prettier is the **sole** authority for code formatting.Should be either:
Prettier is the **sole** authority for code formatting.or, if a label is needed:
**Purpose:** Establish Prettier as the sole formatting authority.Action:
- Remove all 40
**Description:**attribute labels. For each rule block, either drop the label entirely (body text already communicates the description) or replace with a semantically meaningful label (**Purpose:**,**Governs:**,**Rationale:**).
🐛 CORRECTNESS
useApi.test.ts uses mocking patterns the rulebook explicitly forbids
The rulebook's Testing section (added in the previous iteration) documents a narrow frontend mock exception and then states: "Mocking Vue component internals, Pinia store actions, or Vue Router navigation is strictly forbidden." The existing useApi.test.ts violates both halves of this prohibition — it mocks Vue Router at module level and spies on Pinia store actions.
Evidence:
// useApi.test.ts:5-6 — Vue Router module mock (explicitly forbidden)
const mockReplace = vi.fn()
vi.mock('vue-router', () => ({
useRouter: () => ({ replace: mockReplace }),
}))
// useApi.test.ts:52, 96, 134 — Pinia store action spy (explicitly forbidden)
const logoutSpy = vi.spyOn(authStore, 'logout')
// useApi.test.ts:100 — Pinia store action mock with implementation (explicitly forbidden)
const refreshMock = vi.spyOn(authStore, 'refresh').mockImplementation(async (...))Rulebook line 1194:
Mocking Vue component internals, Pinia store actions, or Vue Router navigation
is strictly forbidden.
Action:
- Rewrite
useApi.test.tstests to removevi.mock('vue-router', ...). Verify redirect behavior by inspecting router state directly (e.g.,router.currentRoute.value.path) rather than spying onreplace. - Remove
vi.spyOn(authStore, 'logout')andvi.spyOn(authStore, 'refresh').mockImplementation(...). Test the 401 deduplication behavior by observing observable outcomes (store state,isAuthenticated,accessToken) after the sequence of fetch calls completes.
useConfirm.ts swallows errors from confirm actions silently
The confirmCallback wrapper catches all errors from the user-confirmed action and logs them to the console, but never surfaces them to the user. When a delete or revoke operation fails inside the confirm dialog, the dialog closes, the loading spinner stops, and the user sees nothing — no toast, no message, no visible indication that anything went wrong.
Evidence:
// useConfirm.ts:22-29
confirmCallback.value = async () => {
confirmCallback.value = null
try {
await action()
} catch (err) {
console.error('Confirm action failed:', err)
// ↑ Error is logged but never surfaced to the user.
// Dialog closes, UI reverts, no feedback given.
}
}Action:
- Re-throw the error after logging so callers can handle it:
console.error('Confirm action failed:', err); throw err. This allows eachonErrorhandler in the calling mutation to fire its toast. Alternatively, accept an optionalonErrorcallback parameter inopenConfirm()and invoke it here.
📐 DESIGN
API_BASE_URL defined identically in two files — Anti-Duplication violation
useApi.ts and auth.ts each independently define API_BASE_URL with the same value. This is a textbook Anti-Duplication violation: the same knowledge exists in two locations, so a URL change requires two edits, with inevitable divergence risk. The auth.ts store also calls fetch() directly instead of routing through fetchApi(), bypassing the centralized error handling, 401 deduplication, and retry logic in useApi.ts.
Evidence:
// composables/useApi.ts:5
const API_BASE_URL = import.meta.env.VITE_API_URL || 'http://localhost:3001'
// stores/auth.ts:34 — identical definition
const API_BASE_URL = import.meta.env.VITE_API_URL || 'http://localhost:3001'Action:
- Export
API_BASE_URLfromuseApi.ts(or define it in a sharedlib/config.ts) and import it inauth.ts. The login and refresh calls inauth.tscannot usefetchApi()(they run before authentication is established), so the URL constant import is the correct fix.
Term collision: "Reka UI" and "Radix Vue" used interchangeably throughout rulebook
The rulebook uses "Reka UI", "Radix Vue", and "Reka UI / Radix Vue" as if they are synonyms — they appear in the vocabulary section, rule descriptions, rationale paragraphs, and the canonical directory layout. Reka UI is the correct canonical name for the Vue-specific port; "Radix Vue" is a predecessor name. Using both in the same document violates the Ubiquitous Language Enforcement rule and creates ambiguity about which library is actually in use.
Evidence:
grep -c 'Radix Vue' codestyle.frontend.rulebook.md
# → 5
grep -c 'Reka UI' codestyle.frontend.rulebook.md
# → 10Lines 16, 997, 1000, 1001, 1003 all use "Reka UI / Radix Vue" as a compound term, implying they differ. The components/ui/readme.md created in the last iteration also inherited this: "Shadcn-vue style Reka UI / Radix Vue wrapper primitives".
Action:
- Choose one canonical term —
Reka UI— and replace all occurrences of "Radix Vue" (standalone and in compound "Reka UI / Radix Vue"). Update the vocabulary definition to note that "Radix Vue" was the previous name. - Update
src/components/ui/readme.mdrow to use only "Reka UI".
Canonical project structure includes mock/ directory — name conflicts with no-mocking policy
The canonical directory layout (line 1302) lists ├── mock/ # Mock data for development. The rulebook's own Testing section states the no-mocking rule applies to "Rust backend code" with a narrow exception list for jsdom stubs. A top-level mock/ directory named "mock" will create confusion: contributors will be unsure whether this is a permitted location for test fixtures, development stubs, or something else. The directory does not currently exist in the codebase.
Evidence:
# codestyle.frontend.rulebook.md:1302
├── mock/ # Mock data for development
# ls module/iron_dashboard/src/mock/
ls: cannot access '.../src/mock/': No such file or directory
Action:
- If the intent is static development fixtures (seed data for local dev), rename the entry to
fixtures/orseed/in the canonical layout to avoid the naming collision with the mocking prohibition. - If no such directory is planned, remove the
mock/entry from the canonical layout entirely.
🔍 QUALITY
deleteMutation and revokeIcTokenMutation in AgentsView.vue have no toast.success() feedback
Both mutations have onError handlers with user-visible toasts but their onSuccess callbacks are silent. Agent deletion and IC token revocation are destructive, irreversible operations — users need confirmation that the action completed. All other mutations in this file (createMutation, updateMutation, generateIcTokenMutation, regenerateIcTokenMutation) correctly show success toasts.
Evidence:
// AgentsView.vue:224-232 — no toast in onSuccess
const deleteMutation = useMutation({
mutationFn: (id: number) => api.deleteAgent(id),
onSuccess: () => {
queryClient.invalidateQueries({ queryKey: ['agents'] })
// ← no toast.success()
},
onError: (err) => { toast.error(...) },
})
// AgentsView.vue:365-384 — no toast in onSuccess
const revokeIcTokenMutation = useMutation({
mutationFn: (agentId: number) => api.revokeIcToken(agentId),
onSuccess: (_data, agentId) => {
queryClient.setQueryData(['ic-token-status', agentId], { ... })
// ← no toast.success()
},
onError: (err) => { toast.error(...) },
onSettled: () => { ... },
})Action:
- Add
toast.success('Agent deleted')todeleteMutation.onSuccess. - Add
toast.success('IC token revoked')torevokeIcTokenMutation.onSuccess.
Multi-statement @click handlers in template — should be named functions
Three @click attributes in AgentsView.vue contain multi-statement inline expressions (5-6 state reset assignments each). The rulebook's naming conventions section requires event handlers to be extracted as named handle* functions. Inline multi-statement handlers are not readable, cannot be tested, and cannot be reused.
Evidence:
<!-- AgentsView.vue:705-713 — 6-statement inline cancel handler -->
@click="
showCreateModal = false
name = ''
selectedProviderKeyIds = []
addingProviderKeyId = ''
initialBudgetUsd = undefined
selectedOwnerId = ''
"
<!-- AgentsView.vue:816-824 — 5-statement inline cancel handler -->
@click="
showUpdateModal = false
name = ''
selectedProviderKeyIds = []
addingProviderKeyId = ''
"
<!-- AgentsView.vue:879-886 — 5-statement inline dialog close handler -->
@click="
showTokenDialog = false
tokenDialogValue = ''
tokenDialogAgentName = ''
tokenDialogWarning = ''
copyMessage = ''
"Action:
- Extract each to a named function:
handleCancelCreate(),handleCancelUpdate(),handleCloseTokenDialog(). Replace inline handler with@click="handleCancelCreate"etc.
Strong iteration — 6/6 prior findings resolved. The remaining 8 items are split between rulebook corrections (the **Description:** labels and term collision) and code fixes (mocking violations, silent errors, missing toasts, inline handlers, duplicated constant). The mocking finding is the highest priority since the test code actively contradicts a rule the PR itself introduced.
❗ > Note: This System of Review operates with the same information access and constraints as any human developer working on this repository. While the system is highly accurate (99%+ reliability), any confusion or apparent misunderstanding typically indicates gaps in repository documentation, unclear code organization, or insufficient project discipline — the same issues that would impact any team member. If the System cannot locate critical information and draws incorrect conclusions, a human developer would face identical challenges. We maintain high standards to respect our teammates' time, our clients' investment, and the integrity of this project. Please ensure the repository provides clear, discoverable context for all reviewers.
Review approval is a quality floor, not a quality ceiling. It confirms that identified issues are resolved — not that the code is free of all issues. The System reviews the changes in this PR, not the entire codebase. Proactive quality is a developer responsibility: unit test coverage, edge case analysis, and consistency with the surrounding codebase cannot be delegated to any review process. If the review surfaces one instance of a pattern, the developer is responsible for auditing all occurrences of that pattern throughout the codebase — not just the specific line cited.
When addressing review feedback: Open a separate commit for each point you address, clearly referencing the finding in the commit message. This ensures knowledge is captured in the repository history and helps other developers avoid the same pitfalls. If you cannot address a point, leave a detailed comment in this PR review thread explaining specifically what is wrong with the finding or why it cannot be addressed — never ignore feedback silently. Use comments only in rare cases when the System has genuinely missed existing context in the repository; prefer commits as the primary response mechanism to build institutional knowledge.
wanguardd
changed the title
- Remove 40 redundant **Description:** labels from frontend rulebook - Unify rulebook terminology on "Reka UI" (drop "Radix Vue" compound) - Drop unused mock/ directory from canonical layout - Dedupe API_BASE_URL into lib/config.ts shared by useApi and auth store - Re-throw errors from useConfirm action so callers can surface them - Rewrite useApi.test.ts to avoid forbidden vue-router and Pinia mocks - Add success toasts for destructive mutations across Agents/Users/Providers - Extract multi-statement inline @click handlers to named handlers - Fix ConfirmDialog multi-statement inline handler that broke the build
Fixes — 2026-04-20Third-pass audit findings addressed on Fix #1 — Remove "Description:" attribute labels from frontend rulebookFile(s): Fix #2 — Remove forbidden Vue Router and Pinia mocks from useApi.test.tsFile(s): Fix #3 — Surface errors from useConfirm action callbacksFile(s): Fix #4 — Deduplicate API_BASE_URL between useApi.ts and auth.tsFile(s): Fix #5 — Unify rulebook terminology on "Reka UI"File(s): Fix #6 — Remove unused
|
Colard
changed the title
wanguardd
left a comment
wanguardd
left a comment
There was a problem hiding this comment.
System of Review completed analysis of 'Enforce frontend codestyle rulebook across iron_dashboard'. Found 5 findings across 4 files.
📊 Progress Summary
Review #2
Progress since Review #1:
- Previous review: 15 findings
- This review: 5 findings
- Net change: −10 findings
Excellent progress — 13 of 15 previous findings are fully resolved: YAML frontmatter, Security section, v-html prohibition, accessibility rules, global error boundary, mocking exception, PascalCase cross-reference, utils.ts naming exception, router path aliases (router/index.ts), IconPlus/IconX Prettier formatting, boolean ref naming (isLoading, showMobileFilters), event handler handle prefixes, and the import order wording. Two findings carry forward as RE-REQUESTs.
📋 Review Summary
| Category | Count |
|---|---|
| 🐛 CORRECTNESS | 1 |
| 🔍 QUALITY | 3 |
| ✅ TESTING | 1 |
🐛 CORRECTNESS
RE-REQUEST: Prettier violations remain in AgentsView.vue and ProvidersView.vue
The previous review found 45 files failing Prettier. The compliance pass resolved 43 — but Fix #8 (2026-04-20), which extracted inline handlers to named functions in AgentsView.vue and ProvidersView.vue, introduced new code that was not run through Prettier before commit. Two files remain non-compliant.
Evidence:
$ npx prettier --check 'src/**/*.{ts,vue}'
[warn] src/views/AgentsView.vue
[warn] src/views/ProvidersView.vue
[warn] Code style issues found in 2 files. Run Prettier with --write to fix.
# Exit code: 1Action:
- Run
npx prettier --write src/views/AgentsView.vue src/views/ProvidersView.vueand commit the formatting fix
🔍 QUALITY
App.vue and MainLayout.vue use relative imports into subdirectories
The rulebook (Imports & Module Organization : Path Aliases) states: "Relative imports (../, ./) are only permitted within the same immediate directory." Two files modified in this PR violate this rule by importing from child directories using relative paths.
App.vue (in src/) imports from ./components/, which is a subdirectory:
// src/App.vue line 6
import MainLayout from './components/MainLayout.vue'
// ↑ should be: '@/components/MainLayout.vue'MainLayout.vue (in src/components/) imports 10 icon components from ./icons/, which is a subdirectory:
// src/components/MainLayout.vue lines 9–18
import IconX from './icons/IconX.vue'
import IconHome from './icons/IconHome.vue'
import IconChevronDown from './icons/IconChevronDown.vue'
import IconCog from './icons/IconCog.vue'
import IconBarChart from './icons/IconBarChart.vue'
import IconCoin from './icons/IconCoin.vue'
import IconChip from './icons/IconChip.vue'
import IconUsers from './icons/IconUsers.vue'
import IconLogOut from './icons/IconLogOut.vue'
import IconMenu from './icons/IconMenu.vue'
// ↑ all should use '@/components/icons/Icon*.vue'(The ./AvatarInitial.vue import on line 8 is in the same directory — that one is correct.)
Action:
- In
App.vue, replace'./components/MainLayout.vue'with'@/components/MainLayout.vue' - In
MainLayout.vue, replace all'./icons/Icon*.vue'imports with'@/components/icons/Icon*.vue'paths (10 imports, lines 9–18)
StatusBadge.vue applies conditional classes without cn()
The rulebook (Styling : Dynamic Class Composition with cn()) states: "When a component needs to apply conditional Tailwind classes, the cn() utility must be used." StatusBadge.vue uses a raw ternary expression in :class and does not import cn at all.
Evidence:
// src/components/StatusBadge.vue — line 2 (script block)
import { Badge } from '@/components/ui/badge'
// ↑ cn is not imported
// src/components/StatusBadge.vue — line 20 (template)
:class="active ? 'text-success border-none' : 'text-muted-foreground border-none'"
// ↑ must use cn() for conditional class compositionAction:
- Add
import { cn } from '@/lib/utils'to the script block - Rewrite the
:classbinding::class="cn(active ? 'text-success border-none' : 'text-muted-foreground border-none')"
useApi.ts missing blank line before type-only import group
The rulebook (Imports & Module Organization : Import Order) requires groups to be separated by blank lines, with type-only imports in their own final group. In useApi.ts, import type immediately follows regular @/lib/ imports with no blank line.
Evidence:
// src/composables/useApi.ts lines 3–5
import { useAuthStore } from '@/stores/auth'
import { API_BASE_URL } from '@/lib/config'
import type { ProviderType } from '@/lib/providers' // ← no blank line before thisAction:
- Add a blank line between
import { API_BASE_URL } from '@/lib/config'andimport type { ProviderType }to separate Group 2 (stores/composables/lib) from Group 4 (types)
✅ TESTING
RE-REQUEST: quickAddMutation in ProvidersView.vue still missing success toast
The previous review flagged 6 mutations that lacked toast.success() feedback. Five were fixed. The inline onSuccess callback for quickAddMutation (passed to mutate() directly, not defined at construction) was not addressed — it only closes the modal with no user confirmation.
Evidence:
// src/views/ProvidersView.vue lines 316–324
quickAddMutation.mutate(
{ provider: detectedProvider.value, api_key: quickAddKey.value.trim(), alias: previewAlias.value },
{
onSuccess: () => handleCloseQuickAdd(), // ← no toast.success()
onError: (err) => {
quickAddError.value = err instanceof Error ? err.message : 'Failed to add key — please try again'
},
}
)// src/views/ProvidersView.vue line 295
function handleCloseQuickAdd() {
showQuickAddModal.value = false // ← no toast here either
}Action:
- Add
toast.success('Provider key added successfully')beforehandleCloseQuickAdd()in the quickAddonSuccesscallback
Strong iteration — the rulebook is now comprehensive (Security, Accessibility, Global Error Boundary, mocking exception, ecosystem cross-references all in place) and the compliance pass resolved the systemic issues from the first review. A focused third pass on the 3 blocking items above will close this out.
wanguardd
changed the title
2 participants
Summary
Introduces
codestyle.frontend.rulebook.md— a comprehensive codestyle rulebook for the frontend project — and brings the entireiron_dashboardcodebase into full compliance with it.Rulebook (29 rules across 12 categories)
@/path aliases, barrel exports)cn()utility, CVA variants)Codebase fixes (126 files changed)
Formatting
.vueand.tsfiles — fixed double quotes to single quotes.Imports
../imports with@/path aliases (10 files).Switch.vuedirect import → barrel import.Component structure
Input.vueandTextarea.vue.State management
AgentsView.vuefrom manualtry/catchtouseMutationwith properonSuccess/onError/onSettledcallbacks.Styling
AvatarInitial.vuewith Tailwind design token classes.bg-blue-500,bg-purple-500) with chart tokens inproviders.ts.cn()utility (MainLayout.vue,BudgetsView.vue,providers.ts).Naming
handleprefix to 11 event handler functions across 5 view files.Type safety
auth.tsJWT decoder to fixvue-tsc -bstrict build.Test plan
vue-tsc -b— zero type errorsvue-tsc --noEmit— zero type errorsvite build— successful production buildvitest run— 56/56 tests pass (5 test files)