feat: setup wizard UX overhaul -- mode choice, step reorder, provider fixes#907
feat: setup wizard UX overhaul -- mode choice, step reorder, provider fixes#907
Conversation
WalkthroughIntroduces a hidden "mode" step that branches the setup wizard into Guided (mode → template → company → providers → agents → theme → complete) and Quick (mode → company → providers → complete) flows, and reorders steps so providers precede agents. Updates completion validation to require a company plus at least one provider (agents optional). Adds agent-personality request/response models, two setup API endpoints, a new SetupPersonalityController, store and UI integrations, new setup_helpers module consolidating setup logic, test updates, and removes cost-estimation utilities, UI, and related tests. Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches. Re-running this action after a short time may resolve the issue. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
There was a problem hiding this comment.
Code Review
This pull request introduces a 'Quick Setup' mode to the first-run wizard, allowing users to bypass detailed agent and theme configuration. The setup flow has been reordered to prioritize provider configuration, and agent personality presets can now be selected during the guided flow. Notably, cost estimation and budget capping features have been removed from the setup process. Review feedback suggests improving the readability of the design documentation and refactoring hardcoded layout values in the organization chart component into named constants.
docs/design/page-structure.md
Outdated
| | `/login` | Login | No sidebar, full page | | ||
| | `/setup` | Setup Wizard | No sidebar, full page. Redirects to `/` if already complete | | ||
| | `/setup/:step` | Setup Wizard step | `account` (conditional), `template`, `company`, `agents`, `providers`, `theme`, `complete` | | ||
| | `/setup/:step` | Setup Wizard step | `account` (conditional), `mode`, `template`, `company`, `providers`, `agents`, `theme`, `complete` (guided); `mode`, `company`, `providers`, `complete` (quick) | |
There was a problem hiding this comment.
The description of the wizard steps in this table cell is very dense. To improve readability, consider using a line break and bolding to separate the guided and quick setup flows.
| | `/setup/:step` | Setup Wizard step | `account` (conditional), `mode`, `template`, `company`, `providers`, `agents`, `theme`, `complete` (guided); `mode`, `company`, `providers`, `complete` (quick) | | |
| | `/setup/:step` | Setup Wizard step | **Guided**: `account` (conditional), `mode`, `template`, `company`, `providers`, `agents`, `theme`, `complete`<br>**Quick**: `mode`, `company`, `providers`, `complete` | |
web/src/pages/setup/MiniOrgChart.tsx
Outdated
| const isSmallTeam = agents.length <= 5 | ||
| const avatarRadius = isSmallTeam ? 16 : 14 | ||
| const nodeWidth = isSmallTeam ? 110 : 90 | ||
| const nodeHeight = 36 | ||
| const hGap = isSmallTeam ? 40 : 28 | ||
| const vGap = isSmallTeam ? 60 : 50 |
There was a problem hiding this comment.
This component uses several hardcoded 'magic numbers' for layout calculations and thresholds (e.g., 5, 16, 14, 110, 90). To improve readability and maintainability, it's better to define these as named constants at the top of the file.
For example:
const SMALL_TEAM_THRESHOLD = 5;
const LARGE_AVATAR_RADIUS = 16;
const SMALL_AVATAR_RADIUS = 14;
// ... and so on for other layout valuesThis also applies to the string truncation logic on line 170, which uses 14 and 12.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #907 +/- ##
==========================================
+ Coverage 92.10% 92.12% +0.01%
==========================================
Files 593 595 +2
Lines 31324 31404 +80
Branches 3041 3043 +2
==========================================
+ Hits 28851 28930 +79
Misses 1955 1955
- Partials 518 519 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/pages/setup/CompanyStep.tsx (1)
78-85:⚠️ Potential issue | 🟡 MinorLine exceeds 88 character limit.
Line 84 is 116 characters. Consider extracting the validation logic or reformatting.
🔧 Suggested fix
+ const nameError = companyName.trim() === '' + ? null + : companyName.trim().length > 200 + ? 'Max 200 characters' + : null + <InputField label="Company Name" required value={companyName} onChange={(e) => setCompanyName(e.currentTarget.value)} placeholder="Your organization name" - error={companyName.trim() === '' ? null : companyName.trim().length > 200 ? 'Max 200 characters' : null} + error={nameError} />Or inline across multiple lines:
<InputField label="Company Name" required value={companyName} onChange={(e) => setCompanyName(e.currentTarget.value)} placeholder="Your organization name" - error={companyName.trim() === '' ? null : companyName.trim().length > 200 ? 'Max 200 characters' : null} + error={ + companyName.trim() === '' + ? null + : companyName.trim().length > 200 + ? 'Max 200 characters' + : null + } />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/setup/CompanyStep.tsx` around lines 78 - 85, The long inline validation in the InputField error prop makes the line exceed the limit; extract the check into a small helper or local variable (e.g., const companyNameError = companyName.trim() === '' ? null : companyName.trim().length > 200 ? 'Max 200 characters' : null) and pass companyNameError to the InputField error prop, or break the ternary across multiple lines inside the CompanyStep component to keep the line length under 88 characters while preserving the current logic for companyName and setCompanyName.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/design/page-structure.md`:
- Around line 131-133: The Quick Setup flow description and route list omits the
conditional account creation step; update the "Quick mode steps" and the
URL-addressable step list so that when admin/account setup is required the
sequence is account → mode selection → company creation → provider setup →
completion (and ensure `/setup/{step}` ordering reflects `account` appears
before `mode` for fresh installs); modify the text where "Quick mode steps" is
defined and any route table entries that list steps (the "Quick mode steps"
sentence and the `/setup/{step}` route list) to include `account` in the Quick
path.
In `@src/synthorg/api/controllers/setup_models.py`:
- Around line 262-275: The _validate_preset_exists model validator currently
does its own normalization/membership check against PERSONALITY_PRESETS,
bypassing the shared get_personality_preset helper and its warning log; replace
the manual check in _validate_preset_exists with a call to
get_personality_preset(personality_preset_raw) (importing it from
synthorg.templates.presets) so the helper performs normalization, membership
checking and emits the required WARNING on unknown presets, then set
values["personality_preset"] to the normalized key returned (or propagate the
helper's ValueError) to keep validation and logging consistent with
create/update paths.
In `@src/synthorg/api/controllers/setup.py`:
- Around line 598-691: The SetupController has grown too large—split the
personality-related endpoints and helpers into a new controller/service: move
update_agent_personality and list_personality_presets (and any small helpers
they use such as get_existing_agents, _check_setup_not_complete, and the
_AGENT_LOCK usage) into a dedicated PersonalityController (or PersonalityService
+ controller) module; ensure the new controller keeps the same route decorators
and guards, inject or import AppState/settings_service the same way, update
imports for PERSONLITY_PRESETS usage, and export the new controller so routing
remains unchanged; keep each function under 50 lines and remove the personality
logic from SetupController to keep files <800 lines.
- Around line 623-644: The endpoint currently persists whatever
personality_preset the client sends; before calling settings_svc.set() in the
handler (around where get_existing_agents(), _validate_agent_index(),
updated_agent is built and agents replaced), validate that
data.personality_preset is a member of the server-side PERSONALITY_PRESETS
(import from src.synthorg.templates.presets) and if it is not, raise
ApiValidationError with a clear message; perform this check after
_validate_agent_index and before forming updated_agent so no invalid preset is
saved.
- Around line 834-839: complete_setup() currently calls
_check_has_agents(settings_svc, strict=True) which causes malformed
company/agents data to abort completion; change the call to use strict=False (or
otherwise pass strict based on quick-setup mode) so the agent existence check is
non-strict and only logs a warning/info instead of failing the request; update
the callsite in complete_setup() (referencing _check_has_agents and
settings_svc) to pass strict=False and ensure any error-paths remain
non-blocking.
In `@web/src/pages/setup/AgentsStep.tsx`:
- Around line 130-134: The personalityPresetsError block currently shows only
static text; add a "Retry" button alongside that message in the same JSX so
users can re-attempt loading presets. Have the button call the existing presets
loading function (e.g., loadPersonalityPresets or fetchPersonalityPresets —
create it if missing) and show a loading state while it runs; update the JSX in
the AgentsStep component where personalityPresetsError is used to mirror the
agents error block UX by invoking that retry handler on click.
- Around line 36-41: The effect in AgentsStep.tsx repeatedly retries fetching
because it only checks personalityPresets.length and personalityPresetsLoading;
update the useEffect dependency/condition to also guard against a persistent
error by checking personalityPresetsError (e.g., only call
fetchPersonalityPresets when personalityPresets.length === 0,
!personalityPresetsLoading, and !personalityPresetsError) or implement a
retry/backoff flag so fetchPersonalityPresets is not re-triggered on every
render after a failure; adjust the condition and dependencies to include
personalityPresetsError and/or a retry control to stop the infinite retry loop.
In `@web/src/pages/setup/CompanyStep.tsx`:
- Around line 161-171: The agents list render in CompanyStep.tsx uses agent.name
as the React key which can duplicate (see agents.map and key={agent.name}),
causing key warnings; change the key to a stable unique value by combining a
tiebreaker (e.g., the map index or an agent.id if available) like using
`${agent.name}-${index}` in the agents.map callback so each <li> has a unique
key; update the map callback signature to include index and use that composite
key to avoid duplicates (mirrors the approach noted in SetupSummary.tsx).
In `@web/src/pages/setup/MiniOrgChart.tsx`:
- Around line 174-176: The agentX calculation inside pos.dept.agents.map is too
long; refactor by extracting the horizontal spacing expression into a named
constant (e.g., const agentSpacing = avatarRadius * 2 + 10) and then compute
agentX in two shorter steps (compute offset = (agentIdx -
(pos.dept.agents.length - 1) / 2) * agentSpacing; const agentX = pos.x +
offset), keeping agentY as pos.y + vGap; update references to use these new
locals so each line stays under 88 characters and improves readability.
In `@web/src/pages/setup/ProvidersStep.tsx`:
- Around line 57-63: The double-fetch happens because fetchedRef.current is
being reset elsewhere (around where you reset it at the end of the file), which
causes the useEffect with fetchedRef/fetchProviders/fetchPresets to run again;
instead of resetting fetchedRef.current to false, remove that reset and either
(A) call fetchProviders() / fetchPresets() directly from the action that
currently resets fetchedRef.current, or (B) introduce a separate reload
flag/state (e.g., reloadProviders) and set that rather than mutating fetchedRef;
update the reset logic so fetchedRef.current remains the single-once guard used
by the useEffect and avoid flipping it back to false which triggers the
duplicate fetch.
In `@web/src/pages/setup/SetupAgentCard.tsx`:
- Around line 70-75: The SelectField's onChange expects a synchronous handler
but the current code returns the async promise from onPersonalityChange; update
the onChange prop for SelectField (used with agent.personality_preset,
personalityOptions) so it does not return the promise—either call
onPersonalityChange inside a void/wrapped callback or catch errors (e.g.,
onChange={(val) => { onPersonalityChange(index, val).catch(console.error) }} or
onChange={(val) => { void onPersonalityChange(index, val) }}), ensuring no
promise is returned to SelectField.
In `@web/src/pages/setup/TemplateCard.tsx`:
- Around line 59-61: Remove the redundant empty label prop being passed to
StatPill in TemplateCard.tsx: locate the StatPill usages (e.g., the category
pill and the tag pills) and delete label="" so the component relies on its own
label guard (StatPill). Keep the value and className props as-is and ensure no
other logic depends on an explicit empty label prop.
In `@web/src/pages/setup/WizardModeStep.tsx`:
- Around line 57-60: The useEffect in WizardModeStep currently calls
markStepComplete('mode') on mount (relying on the default wizardMode ===
'guided'), which can be surprising; add a concise code comment immediately above
the useEffect explaining the intent: that the step is considered complete on
mount because a default mode ('guided') exists and no explicit user selection is
required, or alternatively note that if you want explicit user confirmation the
logic should be changed to markStepComplete when wizardMode changes via user
action—reference the useEffect, markStepComplete, and wizardMode identifiers so
future readers know why the step is auto-completed.
- Around line 24-27: The ternary expression in WizardModeStep.tsx produces a
string literal that exceeds the 88-character line limit; extract the long shadow
class into a named constant (e.g., shadowClass or SELECTED_SHADOW) and replace
the inline string in the selected branch of the expression with that constant
(keep the existing 'border-accent' and 'bg-accent/5' parts inline or concatenate
them), or break the JSX expression across lines so the final ternary in the
component (the expression using selected ? ... : ...) conforms to the
88-character rule.
In `@web/src/stores/setup-wizard.ts`:
- Around line 316-320: When switching wizard mode in setWizardMode(mode) the
code only updates stepOrder and leaves template-derived state (like
selectedTemplate and any selectedTemplateAgents) and skipped-step completion
flags intact, so submitCompany() can still send stale selectedTemplate; update
setWizardMode to, when mode === 'quick', reset selectedTemplate to
null/undefined, clear any selectedTemplateAgents/list, and clear or set to false
the completion flags for the template/agents steps (the flags your code uses to
mark skipped/complete), then update stepOrder as before (use get(), set(), and
the same wizardMode). Apply the same reset logic to the other mode-switch path
referenced in the review (the similar block around the other
setWizardMode/handler).
- Around line 517-531: When post-create discovery fails, don't swallow the
error: in the catch block for discoverModels(name, presetName) /
getProvider(name) set the store's error state (providersError) with the
discoveryErr details and mark the specific provider entry so the UI can
block/provide retry (e.g. set providers[name].discoveryFailed = true or
providers[name].needsDiscovery = true and update providers[name] accordingly)
instead of leaving provider.models === 0 silently; ensure you still keep the
provider in providers but with the error flag so the UI can surface a retry
action that calls discoverModels or refreshes via getProvider.
- Around line 50-75: The step-order array literals (GUIDED_STEP_ORDER,
QUICK_STEP_ORDER, GUIDED_STEP_ORDER_WITH_ACCOUNT, QUICK_STEP_ORDER_WITH_ACCOUNT)
and any long expressions used by getStepOrder exceed the 88-character line
limit; break each array literal onto multiple lines (one item per line or
grouped to keep lines <88 chars), keep trailing commas, and reflow any long
conditional return expressions in getStepOrder so no single line exceeds 88
characters; apply the same wrapping style to the other long literals mentioned
(the other step-order-like constants around the file) to satisfy the repo
linter.
---
Outside diff comments:
In `@web/src/pages/setup/CompanyStep.tsx`:
- Around line 78-85: The long inline validation in the InputField error prop
makes the line exceed the limit; extract the check into a small helper or local
variable (e.g., const companyNameError = companyName.trim() === '' ? null :
companyName.trim().length > 200 ? 'Max 200 characters' : null) and pass
companyNameError to the InputField error prop, or break the ternary across
multiple lines inside the CompanyStep component to keep the line length under 88
characters while preserving the current logic for companyName and
setCompanyName.
🪄 Autofix (Beta)
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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 820ea212-f2eb-49a8-8f01-4a3e8bd46288
📒 Files selected for processing (34)
docs/design/page-structure.mddocs/user_guide.mdsrc/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/setup_models.pysrc/synthorg/observability/events/setup.pytests/unit/api/controllers/test_setup.pyweb/src/__tests__/pages/setup/WizardProgress.test.tsxweb/src/__tests__/stores/setup-wizard.test.tsweb/src/__tests__/utils/cost-estimator.property.test.tsweb/src/__tests__/utils/cost-estimator.test.tsweb/src/api/endpoints/setup.tsweb/src/api/types.tsweb/src/components/ui/stat-pill.tsxweb/src/pages/setup/AgentsStep.tsxweb/src/pages/setup/CompanyStep.tsxweb/src/pages/setup/CompleteStep.tsxweb/src/pages/setup/CostEstimatePanel.tsxweb/src/pages/setup/MiniOrgChart.tsxweb/src/pages/setup/ProviderAddForm.tsxweb/src/pages/setup/ProviderProbeResults.tsxweb/src/pages/setup/ProvidersStep.tsxweb/src/pages/setup/SetupAgentCard.tsxweb/src/pages/setup/SetupSummary.tsxweb/src/pages/setup/TemplateCard.tsxweb/src/pages/setup/TemplateCategoryGroup.tsxweb/src/pages/setup/TemplateCompareDrawer.tsxweb/src/pages/setup/TemplateCostBadge.tsxweb/src/pages/setup/TemplateStep.tsxweb/src/pages/setup/TemplateVariables.tsxweb/src/pages/setup/WizardModeStep.tsxweb/src/pages/setup/WizardProgress.tsxweb/src/pages/setup/WizardShell.tsxweb/src/stores/setup-wizard.tsweb/src/utils/cost-estimator.ts
💤 Files with no reviewable changes (7)
- web/src/pages/setup/TemplateCompareDrawer.tsx
- web/src/tests/utils/cost-estimator.property.test.ts
- web/src/pages/setup/TemplateCostBadge.tsx
- web/src/tests/utils/cost-estimator.test.ts
- web/src/pages/setup/TemplateCategoryGroup.tsx
- web/src/pages/setup/CostEstimatePanel.tsx
- web/src/utils/cost-estimator.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Dashboard Test
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: Build Sandbox
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Preferinterfacefor defining object shapes in TypeScript
Use camelCase for variable names and function identifiers
Files:
web/src/components/ui/stat-pill.tsxweb/src/pages/setup/WizardProgress.tsxweb/src/__tests__/pages/setup/WizardProgress.test.tsxweb/src/pages/setup/CompanyStep.tsxweb/src/pages/setup/CompleteStep.tsxweb/src/pages/setup/TemplateStep.tsxweb/src/pages/setup/WizardModeStep.tsxweb/src/pages/setup/TemplateVariables.tsxweb/src/pages/setup/AgentsStep.tsxweb/src/__tests__/stores/setup-wizard.test.tsweb/src/api/endpoints/setup.tsweb/src/api/types.tsweb/src/pages/setup/ProvidersStep.tsxweb/src/pages/setup/WizardShell.tsxweb/src/pages/setup/MiniOrgChart.tsxweb/src/pages/setup/ProviderAddForm.tsxweb/src/pages/setup/SetupSummary.tsxweb/src/pages/setup/SetupAgentCard.tsxweb/src/pages/setup/TemplateCard.tsxweb/src/pages/setup/ProviderProbeResults.tsxweb/src/stores/setup-wizard.ts
web/src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
web/src/**/*.tsx: ALWAYS reuse existing shared components fromweb/src/components/ui/before creating new ones (e.g.,StatusBadge,MetricCard,AgentCard,DeptHealthBar,SectionCard)
Use Tailwind semantic classes (text-foreground,bg-card,text-accent,text-success) or CSS variables (var(--so-accent)); NEVER hardcode hex values or rgba() in .tsx files
Usefont-sansorfont-monofor typography (maps to Geist tokens); NEVER setfontFamilydirectly in CSS
Use density-aware spacing tokens (p-card,gap-section-gap,gap-grid-gap) or standard Tailwind spacing; NEVER hardcode pixel values for layout spacing
Do NOT recreate complex (>8 line) JSX inside.map()blocks; extract to a shared component inweb/src/components/ui/
Files:
web/src/components/ui/stat-pill.tsxweb/src/pages/setup/WizardProgress.tsxweb/src/__tests__/pages/setup/WizardProgress.test.tsxweb/src/pages/setup/CompanyStep.tsxweb/src/pages/setup/CompleteStep.tsxweb/src/pages/setup/TemplateStep.tsxweb/src/pages/setup/WizardModeStep.tsxweb/src/pages/setup/TemplateVariables.tsxweb/src/pages/setup/AgentsStep.tsxweb/src/pages/setup/ProvidersStep.tsxweb/src/pages/setup/WizardShell.tsxweb/src/pages/setup/MiniOrgChart.tsxweb/src/pages/setup/ProviderAddForm.tsxweb/src/pages/setup/SetupSummary.tsxweb/src/pages/setup/SetupAgentCard.tsxweb/src/pages/setup/TemplateCard.tsxweb/src/pages/setup/ProviderProbeResults.tsx
web/src/components/ui/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
web/src/components/ui/**/*.tsx: New shared components must have a corresponding.stories.tsxStorybook file with all component states (default, hover, loading, error, empty)
Export component props as a TypeScript interface
Use design token variables exclusively for shadows and borders (var(--so-shadow-card-hover),border-border,border-bright); never hardcode shadow or border values
Files:
web/src/components/ui/stat-pill.tsx
**/*.{py,ts,tsx,go}
📄 CodeRabbit inference engine (CLAUDE.md)
Lines must not exceed 88 characters (enforced by ruff for Python, configured in web/.eslintrc for TypeScript)
Files:
web/src/components/ui/stat-pill.tsxweb/src/pages/setup/WizardProgress.tsxweb/src/__tests__/pages/setup/WizardProgress.test.tsxweb/src/pages/setup/CompanyStep.tsxweb/src/pages/setup/CompleteStep.tsxweb/src/pages/setup/TemplateStep.tsxweb/src/pages/setup/WizardModeStep.tsxtests/unit/api/controllers/test_setup.pyweb/src/pages/setup/TemplateVariables.tsxsrc/synthorg/observability/events/setup.pyweb/src/pages/setup/AgentsStep.tsxweb/src/__tests__/stores/setup-wizard.test.tsweb/src/api/endpoints/setup.tsweb/src/api/types.tsweb/src/pages/setup/ProvidersStep.tsxweb/src/pages/setup/WizardShell.tsxweb/src/pages/setup/MiniOrgChart.tsxweb/src/pages/setup/ProviderAddForm.tsxweb/src/pages/setup/SetupSummary.tsxweb/src/pages/setup/SetupAgentCard.tsxweb/src/pages/setup/TemplateCard.tsxsrc/synthorg/api/controllers/setup_models.pysrc/synthorg/api/controllers/setup.pyweb/src/pages/setup/ProviderProbeResults.tsxweb/src/stores/setup-wizard.ts
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Import
cnfrom@/lib/utilsfor conditional class merging in components
Files:
web/src/components/ui/stat-pill.tsxweb/src/pages/setup/WizardProgress.tsxweb/src/__tests__/pages/setup/WizardProgress.test.tsxweb/src/pages/setup/CompanyStep.tsxweb/src/pages/setup/CompleteStep.tsxweb/src/pages/setup/TemplateStep.tsxweb/src/pages/setup/WizardModeStep.tsxweb/src/pages/setup/TemplateVariables.tsxweb/src/pages/setup/AgentsStep.tsxweb/src/__tests__/stores/setup-wizard.test.tsweb/src/api/endpoints/setup.tsweb/src/api/types.tsweb/src/pages/setup/ProvidersStep.tsxweb/src/pages/setup/WizardShell.tsxweb/src/pages/setup/MiniOrgChart.tsxweb/src/pages/setup/ProviderAddForm.tsxweb/src/pages/setup/SetupSummary.tsxweb/src/pages/setup/SetupAgentCard.tsxweb/src/pages/setup/TemplateCard.tsxweb/src/pages/setup/ProviderProbeResults.tsxweb/src/stores/setup-wizard.ts
web/src/pages/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Lazy-load page components and place page-scoped sub-components in
pages/<page-name>/subdirectories (e.g.,pages/tasks/,pages/org-edit/)
Files:
web/src/pages/setup/WizardProgress.tsxweb/src/pages/setup/CompanyStep.tsxweb/src/pages/setup/CompleteStep.tsxweb/src/pages/setup/TemplateStep.tsxweb/src/pages/setup/WizardModeStep.tsxweb/src/pages/setup/TemplateVariables.tsxweb/src/pages/setup/AgentsStep.tsxweb/src/pages/setup/ProvidersStep.tsxweb/src/pages/setup/WizardShell.tsxweb/src/pages/setup/MiniOrgChart.tsxweb/src/pages/setup/ProviderAddForm.tsxweb/src/pages/setup/SetupSummary.tsxweb/src/pages/setup/SetupAgentCard.tsxweb/src/pages/setup/TemplateCard.tsxweb/src/pages/setup/ProviderProbeResults.tsx
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Never usefrom __future__ import annotations— Python 3.14 has PEP 649 native lazy annotations
Useexcept A, B:syntax without parentheses for multiple exception catching (PEP 758)
All public functions and classes must have type hints and Google-style docstrings (enforced by ruff D rules)
Files:
tests/unit/api/controllers/test_setup.pysrc/synthorg/observability/events/setup.pysrc/synthorg/api/controllers/setup_models.pysrc/synthorg/api/controllers/setup.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Mark all tests with@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, or@pytest.mark.slow
Always run pytest with-n autofor parallel execution via pytest-xdist; never run tests sequentially
Use vendor-agnostic names (test-provider,test-small-001, etc.) instead of real vendor names (Anthropic, OpenAI, Claude, GPT). Real vendor names only allowed in: (1) docs/design/operations.md, (2) .claude/ files, (3) third-party imports, (4) provider presets in src/synthorg/providers/presets.py
For timing-sensitive tests, mocktime.monotonic()andasyncio.sleep()to make them deterministic; for indefinite blocking, useasyncio.Event().wait()instead ofasyncio.sleep(large_number)
Files:
tests/unit/api/controllers/test_setup.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic must import and useget_loggerfromsynthorg.observabilitywith variable namelogger
Use event constants from domain-specific modules undersynthorg.observability.events(e.g.,API_REQUEST_STARTEDfromevents.api). Never use string literals for event names
Use structured logging format:logger.info(EVENT, key=value)— never uselogger.info('msg %s', val)string formatting
All error paths must log at WARNING or ERROR level with context before raising exceptions
All state transitions must be logged at INFO level
Create new objects instead of mutating existing ones; usecopy.deepcopy()at construction andMappingProxyTypefor non-Pydantic collections; usemodel_copy(update=...)for runtime state in mutable models
Use frozen Pydantic models for config/identity; separate mutable-via-copy models for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model
Use@computed_fieldin Pydantic models for derived values instead of storing redundant fields and adding validators
UseNotBlankStrfromcore.typesfor all identifier/name fields including optional (NotBlankStr | None) and tuple variants instead of manual whitespace validators
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code instead of barecreate_task
Keep functions under 50 lines and files under 800 lines
Validate user input, external APIs, and config files explicitly at system boundaries; never silently swallow errors
Never useimport loggingorlogging.getLogger()in application code; only useget_loggerfromsynthorg.observability(exception:observability/setup.pyandobservability/sinks.pymay use stdlib logging for bootstrap)
Files:
src/synthorg/observability/events/setup.pysrc/synthorg/api/controllers/setup_models.pysrc/synthorg/api/controllers/setup.py
docs/design/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
When an implementation deviates from the spec (better approach found, scope evolved, etc.), update the relevant
docs/design/page to reflect the new reality
Files:
docs/design/page-structure.md
**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Documentation must be written in Markdown and built with Zensical (config:
mkdocs.yml); design spec lives indocs/design/(11 pages linked fromdocs/DESIGN_SPEC.md)
Files:
docs/design/page-structure.mddocs/user_guide.md
🧠 Learnings (10)
📚 Learning: 2026-03-28T14:27:45.936Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T14:27:45.936Z
Learning: Applies to web/src/**/*.tsx : Do NOT recreate complex (>8 line) JSX inside `.map()` blocks; extract to a shared component in `web/src/components/ui/`
Applied to files:
web/src/pages/setup/CompleteStep.tsxweb/src/pages/setup/WizardShell.tsx
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to docs/design/**/*.md : Design specification pages in `docs/design/` must be consulted before implementing features (7 pages: index, agents, organization, communication, engine, memory, operations)
Applied to files:
docs/design/page-structure.md
📚 Learning: 2026-03-26T15:18:16.848Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T15:18:16.848Z
Learning: Applies to src/synthorg/api/**/*.py : Litestar API must include setup wizard, auth/, auto-wiring, and lifecycle management
Applied to files:
docs/user_guide.md
📚 Learning: 2026-03-15T21:20:09.993Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:20:09.993Z
Learning: Applies to web/src/components/** : Vue components organized by feature (agents/, approvals/, budget/, common/, dashboard/, layout/, messages/, org-chart/, tasks/).
Applied to files:
web/src/pages/setup/SetupSummary.tsx
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (via `model_copy(update=...)`) for runtime state that evolves
Applied to files:
src/synthorg/api/controllers/setup_models.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`. For derived values use `computed_field` instead of storing + validating redundant fields. Use `NotBlankStr` (from `core.types`) for all identifier/name fields — including optional (`NotBlankStr | None`) and tuple (`tuple[NotBlankStr, ...]`) variants — instead of manual whitespace validators.
Applied to files:
src/synthorg/api/controllers/setup_models.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; separate mutable-via-copy models (using `model_copy(update=...)`) for runtime state
Applied to files:
src/synthorg/api/controllers/setup_models.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Applied to files:
src/synthorg/api/controllers/setup_models.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/api/**/*.py : API package (api/): Litestar REST + WebSocket with controllers, guards, channels, JWT + API key + WS ticket auth, approval gate integration, coordination endpoint, collaboration endpoint, settings endpoint, provider management endpoint (CRUD + test + presets), backup endpoint, RFC 9457 structured errors, AppState hot-reload slots, service auto-wiring (Phase 1 at construction, Phase 2 on startup), lifecycle helpers
Applied to files:
src/synthorg/api/controllers/setup.py
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to src/synthorg/templates/**/*.py : Templates: pre-built company templates, personality presets, and builder.
Applied to files:
web/src/stores/setup-wizard.ts
🔇 Additional comments (22)
web/src/pages/setup/MiniOrgChart.tsx (5)
15-18: LGTM!Height bounds are well-documented and the values align with the PR objectives for dynamic org chart sizing (140–500px).
29-71: LGTM!The
radiusprop cleanly parameterizes agent node sizing, and the conditional font size (radius > 14 ? 10 : 8) ensures readability at both scales.
93-111: LGTM!The dynamic sizing logic based on
isSmallTeamis clean, and the height clamping withMath.min(Math.max(...))correctly enforces the MIN_HEIGHT/MAX_HEIGHT bounds.
125-135: LGTM!The SVG setup correctly uses
displayHeightfor the rendered height while preserving the fullsvgHeightin the viewBox, enabling proper scaling. The fixed root circle radius is appropriate for the company node.
150-171: LGTM!Department node rendering correctly uses dynamic
nodeWidth/nodeHeightand scales font size based on team size. Thefill-surfaceclass follows the project's semantic class naming convention.web/src/pages/setup/SetupSummary.tsx (1)
74-74: LGTM!Clean replacement of the cost estimate metric with provider count, aligning with the PR's removal of cost-estimation UI. The
Object.keys(providers).lengthcalculation is appropriate here.web/src/components/ui/stat-pill.tsx (1)
17-21: LGTM!Good fix for the leading space issue. The conditional
{label && (...)}correctly skips rendering the label span when an empty string is passed.Per coding guidelines, shared UI components should have a corresponding
.stories.tsxfile with all states (including the empty-label case). Please ensure the Storybook file is updated if not already.web/src/pages/setup/WizardProgress.tsx (1)
10-19: LGTM!The
STEP_LABELSrecord correctly includes the newmodestep and all existing steps. The display order is correctly determined by thestepOrderprop rather than the record key order.src/synthorg/observability/events/setup.py (1)
100-104: LGTM!New event constants follow the established
setup.<entity>.<action>naming convention and align with the new personality preset API endpoints.web/src/__tests__/pages/setup/WizardProgress.test.tsx (1)
6-17: LGTM!Test fixtures correctly updated to reflect the new step ordering (providers before agents) and the addition of the
modestep to the completion tracking record. ThestepOrderfixture intentionally omitsmodesince it auto-advances and isn't part of the visible progress indicator in these test scenarios.web/src/pages/setup/CompleteStep.tsx (1)
1-1: LGTM!Clean removal of
useMemoimport (no longer needed after cost estimate computation removal), and the updated comment accurately describes the error handling flow.Also applies to: 25-31
web/src/api/types.ts (1)
1278-1289: LGTM!New interfaces correctly mirror the backend Pydantic models. The
readonlymodifiers onPersonalityPresetInfoandPersonalityPresetsListResponseappropriately reflect the frozen nature of the backend models. Field names match the backend exactly for proper JSON serialization.tests/unit/api/controllers/test_setup.py (1)
454-479: LGTM!The test correctly validates the new Quick Setup behavior where agents are optional. The setup properly stubs a provider registry (required for completion), and cleanup is thorough—restoring the original registry and removing all test-specific settings keys.
docs/user_guide.md (1)
69-84: LGTM!Documentation accurately reflects the code changes: mode selection gate, flat template grid with category tags, personality presets, and the updated completion requirements (agents optional in Quick Setup).
web/src/pages/setup/TemplateVariables.tsx (1)
19-23: LGTM!The
localizeCurrencyLabelfunction correctly uses word-boundary regex to replace only whole-word "USD" occurrences, avoiding partial matches in words like "USDA". The early return for undefined currency is efficient.web/src/pages/setup/TemplateStep.tsx (1)
145-159: LGTM!The flat grid implementation using
StaggerGroup/StaggerItemfor animation is clean. Each template renders via the extractedTemplateCardcomponent, avoiding complex JSX inside the.map()block. The responsive breakpoints for column counts are well-structured.web/src/__tests__/stores/setup-wizard.test.ts (2)
39-44: LGTM!Test correctly updated to verify the new mode-first step order. The assertion that
stepOrder[0]equals'mode'whenneedsAdminis false aligns with the store'sGUIDED_STEP_ORDERdefinition.
98-108: LGTM!Navigation gating tests correctly reflect the updated step order. The test on lines 103-108 properly validates that
canNavigateTo('providers')returnsfalsewhencompanyis incomplete, even if prior steps (mode,template) are marked complete.web/src/api/endpoints/setup.ts (1)
87-104: Fits the existing setup API surface cleanly.The new helpers follow the same index validation and
unwrap()pattern as the existing agent endpoints, so this extension stays consistent with the rest of the client.web/src/pages/setup/WizardShell.tsx (1)
30-31: Progress handling is nicely isolated.Filtering
modeout ofprogressStepsand hiding the shared navigation on that step keeps the Guided/Quick branching easy to reason about without special-casing later steps.Also applies to: 47-50, 99-135
web/src/pages/setup/ProviderAddForm.tsx (1)
21-29: Nice inline duplicate-name guard.Surfacing the name clash on the field and disabling submit should avoid the common duplicate-provider round trip.
Also applies to: 73-95
web/src/pages/setup/ProviderProbeResults.tsx (1)
71-118: The per-preset add state is a good UX cleanup.Tracking
addingPresetalongsidealreadyAddedmakes the scan results much clearer and prevents duplicate clicks on the item currently being created.
| @model_validator(mode="before") | ||
| @classmethod | ||
| def _validate_preset_exists(cls, values: dict[str, Any]) -> dict[str, Any]: | ||
| """Normalize and validate the personality preset.""" | ||
| from synthorg.templates.presets import PERSONALITY_PRESETS # noqa: PLC0415 | ||
|
|
||
| raw = values.get("personality_preset", "") | ||
| key = str(raw).strip().lower() if raw else "" | ||
| if key not in PERSONALITY_PRESETS: | ||
| available = sorted(PERSONALITY_PRESETS) | ||
| msg = f"Unknown personality preset {raw!r}. Available: {available}" | ||
| raise ValueError(msg) | ||
| values["personality_preset"] = key | ||
| return values |
There was a problem hiding this comment.
Route preset validation through the shared lookup helper.
This open-codes the normalization/membership check instead of calling get_personality_preset() from src/synthorg/templates/presets.py:407-429. The new update path therefore bypasses the existing warning log for unknown presets, and the create/update validators can drift on accepted values or error text.
As per coding guidelines, "All error paths must log at WARNING or ERROR level with context before raising exceptions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/api/controllers/setup_models.py` around lines 262 - 275, The
_validate_preset_exists model validator currently does its own
normalization/membership check against PERSONALITY_PRESETS, bypassing the shared
get_personality_preset helper and its warning log; replace the manual check in
_validate_preset_exists with a call to
get_personality_preset(personality_preset_raw) (importing it from
synthorg.templates.presets) so the helper performs normalization, membership
checking and emits the required WARNING on unknown presets, then set
values["personality_preset"] to the normalized key returned (or propagate the
helper's ValueError) to keep validation and logging consistent with
create/update paths.
| @put( | ||
| "/agents/{agent_index:int}/personality", | ||
| status_code=HTTP_200_OK, | ||
| guards=[require_ceo], | ||
| ) | ||
| async def update_agent_personality( | ||
| self, | ||
| agent_index: int, | ||
| data: UpdateAgentPersonalityRequest, | ||
| state: State, | ||
| ) -> ApiResponse[SetupAgentSummary]: | ||
| """Update a single agent's personality preset during setup. | ||
|
|
||
| Args: | ||
| agent_index: Zero-based index of the agent to update. | ||
| data: New personality preset assignment. | ||
| state: Application state. | ||
|
|
||
| Returns: | ||
| Updated agent summary. | ||
|
|
||
| Raises: | ||
| ConflictError: If setup has already been completed. | ||
| NotFoundError: If the agent index is out of range. | ||
| """ | ||
| app_state: AppState = state.app_state | ||
| settings_svc = app_state.settings_service | ||
| await _check_setup_not_complete(settings_svc) | ||
|
|
||
| async with _AGENT_LOCK: | ||
| agents = await get_existing_agents(settings_svc) | ||
| _validate_agent_index(agent_index, agents) | ||
|
|
||
| updated_agent = { | ||
| **agents[agent_index], | ||
| "personality_preset": data.personality_preset, | ||
| } | ||
| agents = [ | ||
| *agents[:agent_index], | ||
| updated_agent, | ||
| *agents[agent_index + 1 :], | ||
| ] | ||
| await settings_svc.set( | ||
| "company", | ||
| "agents", | ||
| json.dumps(agents), | ||
| ) | ||
|
|
||
| logger.info( | ||
| SETUP_AGENT_PERSONALITY_UPDATED, | ||
| agent_index=agent_index, | ||
| personality_preset=data.personality_preset, | ||
| ) | ||
|
|
||
| return ApiResponse( | ||
| data=agent_dict_to_summary(agents[agent_index]), | ||
| ) | ||
|
|
||
| @get( | ||
| "/personality-presets", | ||
| guards=[require_read_access], | ||
| ) | ||
| async def list_personality_presets( | ||
| self, | ||
| state: State, # noqa: ARG002 | ||
| ) -> ApiResponse[PersonalityPresetsListResponse]: | ||
| """List all available personality presets. | ||
|
|
||
| Args: | ||
| state: Application state. | ||
|
|
||
| Returns: | ||
| Personality presets data envelope. | ||
| """ | ||
| from synthorg.templates.presets import ( # noqa: PLC0415 | ||
| PERSONALITY_PRESETS, | ||
| ) | ||
|
|
||
| presets = tuple( | ||
| PersonalityPresetInfoResponse( | ||
| name=name, | ||
| description=str(preset.get("description", "")), | ||
| ) | ||
| for name, preset in sorted(PERSONALITY_PRESETS.items()) | ||
| ) | ||
|
|
||
| logger.debug( | ||
| SETUP_PERSONALITY_PRESETS_LISTED, | ||
| count=len(presets), | ||
| ) | ||
|
|
||
| return ApiResponse( | ||
| data=PersonalityPresetsListResponse(presets=presets), | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Split the personality routes out of SetupController.
These additions push an already very large controller even further past the repo’s 800-line limit. Extracting the personality endpoints and their helper logic into a dedicated controller/service module would make the setup API easier to test and change. As per coding guidelines "Keep functions under 50 lines and files under 800 lines."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/api/controllers/setup.py` around lines 598 - 691, The
SetupController has grown too large—split the personality-related endpoints and
helpers into a new controller/service: move update_agent_personality and
list_personality_presets (and any small helpers they use such as
get_existing_agents, _check_setup_not_complete, and the _AGENT_LOCK usage) into
a dedicated PersonalityController (or PersonalityService + controller) module;
ensure the new controller keeps the same route decorators and guards, inject or
import AppState/settings_service the same way, update imports for
PERSONLITY_PRESETS usage, and export the new controller so routing remains
unchanged; keep each function under 50 lines and remove the personality logic
from SetupController to keep files <800 lines.
| app_state: AppState = state.app_state | ||
| settings_svc = app_state.settings_service | ||
| await _check_setup_not_complete(settings_svc) | ||
|
|
||
| async with _AGENT_LOCK: | ||
| agents = await get_existing_agents(settings_svc) | ||
| _validate_agent_index(agent_index, agents) | ||
|
|
||
| updated_agent = { | ||
| **agents[agent_index], | ||
| "personality_preset": data.personality_preset, | ||
| } | ||
| agents = [ | ||
| *agents[:agent_index], | ||
| updated_agent, | ||
| *agents[agent_index + 1 :], | ||
| ] | ||
| await settings_svc.set( | ||
| "company", | ||
| "agents", | ||
| json.dumps(agents), | ||
| ) |
There was a problem hiding this comment.
Reject unknown personality presets at the API boundary.
This endpoint persists whatever personality_preset the client sends, even though the allowed set already exists server-side in src/synthorg/templates/presets.py as PERSONALITY_PRESETS. A stale UI or typo can therefore save an unsupported preset and only fail later when runtime code consumes it. Please validate membership before settings_svc.set() and return ApiValidationError for unknown names. As per coding guidelines "Validate user input, external APIs, and config files explicitly at system boundaries; never silently swallow errors."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/api/controllers/setup.py` around lines 623 - 644, The endpoint
currently persists whatever personality_preset the client sends; before calling
settings_svc.set() in the handler (around where get_existing_agents(),
_validate_agent_index(), updated_agent is built and agents replaced), validate
that data.personality_preset is a member of the server-side PERSONALITY_PRESETS
(import from src.synthorg.templates.presets) and if it is not, raise
ApiValidationError with a clear message; perform this check after
_validate_agent_index and before forming updated_agent so no invalid preset is
saved.
6f5ec6a to
603bdcd
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
web/src/pages/setup/MiniOrgChart.tsx (2)
161-204: 🛠️ Refactor suggestion | 🟠 MajorExtract the department subtree out of
deptPositions.map().This inline branch is now the bulk of
MiniOrgChart, which makes the new geometry harder to test and keeps too much render logic in the parent component. A dedicatedDepartmentGroup/DepartmentNodecomponent would isolate the SVG layout concerns and keep the main render path small.
Based on learnings: "Do NOT create complex (>8 line) JSX inside.map()-- extract to a shared component".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/setup/MiniOrgChart.tsx` around lines 161 - 204, The deptPositions.map() inline JSX inside MiniOrgChart is too large and should be extracted into a dedicated component (e.g. DepartmentGroup or DepartmentNode) to isolate SVG layout and simplify testing; create a new component that accepts a single pos (the same shape used in deptPositions), renders the department rect and label and maps pos.dept.agents to AgentNode (preserving props agent, agentX, agentY, deptX, deptY, radius and the current key pattern), then replace the inline JSX in MiniOrgChart's deptPositions.map(...) with a call to the new component, passing nodeWidth/nodeHeight/avatarRadius/vGap/AGENT_SPACING_GAP as needed or deriving them from props so layout logic is moved out of MiniOrgChart.
46-76: 🛠️ Refactor suggestion | 🟠 MajorReuse the shared avatar implementation for agent nodes.
AgentNodestill hand-draws the initials circle and text, which means avatar fallback, styling, and accessibility behavior will diverge from the rest of the app. Please render the existing<Avatar>here, or wrap it in an SVG-friendly adapter if the chart needs SVG positioning.
Based on learnings: "Do NOT render initials circles manually -- use<Avatar>".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/setup/MiniOrgChart.tsx` around lines 46 - 76, AgentNode currently draws the avatar manually (circle + text) which deviates from the shared Avatar implementation; replace the manual drawing with the shared Avatar component (or an SVG-friendly adapter/wrapper) so avatar fallback, styling and accessibility are consistent. In the AgentNode function, remove the <circle> and <text> initials rendering and instead render <Avatar name={agent.name} title={agent.role} size={radius * 2} ...> (or mount Avatar inside a foreignObject / AvatarSvg adapter if you need exact SVG positioning) and pass the agent props (agent.name, agent.role) and the computed size/position (agentX, agentY, radius) so the avatar is centered where the circle used to be. Ensure the linking line (<line>) and any ARIA/title behavior remains unchanged and that getInitials is no longer used here.tests/unit/api/controllers/test_setup.py (2)
1407-1421:⚠️ Potential issue | 🟠 Major
read_name_locales()should not returnNonefor a missing setting anymore.The
setup_helpersimplementation creates the default locale list whenget_entry()raisesSettingNotFoundError, so this test should assert the resolved default instead ofNone. With the new import target, the current expectation is stale.🧪 Suggested test update
from synthorg.settings.errors import SettingNotFoundError + from synthorg.templates.locales import ALL_LATIN_LOCALES ... try: result = await _read_name_locales(settings_svc) - assert result is None + assert result == list(ALL_LATIN_LOCALES) finally: settings_svc.get_entry = original🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/api/controllers/test_setup.py` around lines 1407 - 1421, The test expectation is stale: when settings_svc.get_entry raises SettingNotFoundError, read_name_locales now returns the created default locale list instead of None; update the assertion to check that result equals the default locale list produced by read_name_locales (call _read_name_locales as done) rather than asserting None, referencing read_name_locales/_read_name_locales and the mock of settings_svc.get_entry (which raises SettingNotFoundError("company","name_locales")) to verify the resolved default.
1338-1351:⚠️ Potential issue | 🟠 MajorExpect
RuntimeErrorhere, notFalse.After switching this import to
synthorg.api.controllers.setup_helpers,check_has_name_locales()no longer returnsFalseon unexpectedget_entry()failures; it logs and raisesRuntimeErrorinstead. This test is still pinned to the old wrapper behavior.🧪 Suggested test update
try: - result = await _check_has_name_locales(settings_svc) - assert result is False + with pytest.raises(RuntimeError, match="Failed to check name_locales"): + await _check_has_name_locales(settings_svc) finally: settings_svc.get_entry = original🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/api/controllers/test_setup.py` around lines 1338 - 1351, The test expects False on get_entry failures but the imported function check_has_name_locales (referenced here as _check_has_name_locales) now logs and re-raises the RuntimeError; update the test to assert that calling _check_has_name_locales(settings_svc) raises RuntimeError instead of returning False (use pytest.raises or equivalent), keeping the settings_svc.get_entry AsyncMock(side_effect=RuntimeError("db connection lost")) setup and restoring original get_entry after the test.web/src/pages/setup/ProvidersStep.tsx (1)
34-199: 🛠️ Refactor suggestion | 🟠 MajorSplit
ProvidersStepbefore the next setup change lands.This one function now mixes initial loading, auto-probing, validation side effects, create/reprobe handlers, and multiple render-state branches. Extracting the fetch/probe orchestration into a hook and lifting the major sections into small components will make this step much safer to extend and bring it back under the repo limit.
As per coding guidelines, "Keep functions under 50 lines and files under 800 lines".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/setup/ProvidersStep.tsx` around lines 34 - 199, ProvidersStep is too large and mixes orchestration, side-effects, handlers and UI; extract the fetch/probe/validation logic into a custom hook (e.g., useProvidersSetup) and lift major UI sections into small components to keep ProvidersStep under 50 lines. Move the mounting fetch logic that calls fetchProviders and fetchPresets, the auto-probe logic that uses probeAllPresets and probeAttemptedRef, the reprobe handler (reprobePresets), the create handlers (createProviderFromPreset + fetchProviders) and the validation side-effect that calls validateProvidersStep/markStepComplete/markStepIncomplete into useProvidersSetup; have that hook return state values (providers, presets, probeResults, probing, loading/error flags) and action handlers (handleAddPreset, handleAddCloud, handleReprobe). Then refactor ProvidersStep to only compose the returned state/handlers into small presentation components (e.g., LoadingState, ProviderProbeResults, ProviderAddForm, ConfiguredProviders, ValidationMessages) so the top-level component is concise and focused on rendering.src/synthorg/api/controllers/setup_models.py (1)
223-232:⚠️ Potential issue | 🟠 MajorGuard these
beforevalidators against non-object payloads.Both
SetupAgentRequest._validate_preset_existsandUpdateAgentPersonalityRequest._validate_preset_existscallvalues.get(...)assuming the raw input is always a dict. When the request body arrives as a list or string, this raisesAttributeErrorwhich escapes as an unhandled internal error rather than a cleanValidationError. Pydantic v2'smode="before"validators only wrapValueErrorandAssertionError; other exceptions propagate uncaught. Checkisinstance(values, dict)and raiseValueErrorwith a clear message if the payload is not an object, per the coding guideline "Validate input at system boundaries."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/api/controllers/setup_models.py` around lines 223 - 232, The before-mode validators SetupAgentRequest._validate_preset_exists and UpdateAgentPersonalityRequest._validate_preset_exists should first confirm the incoming payload is a mapping: add an isinstance(values, dict) check at the top of each validator and if not a dict raise ValueError("request body must be an object") (or similar clear message) so Pydantic treats it as a validation error; after that continue to call values.get(...) and use _normalize_and_validate_preset(...) as before.
♻️ Duplicate comments (1)
src/synthorg/api/controllers/setup.py (1)
607-708: 🛠️ Refactor suggestion | 🟠 Major
SetupControlleris still over the repo size cap.These routes push
src/synthorg/api/controllers/setup.pyback above 800 lines, so the personality flow should live in its own controller/module instead of extendingSetupControlleragain.As per coding guidelines "Keep functions under 50 lines and files under 800 lines."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/api/controllers/setup.py` around lines 607 - 708, The SetupController file exceeds the size cap—extract the personality-related endpoints into a new controller module: create a new controller (e.g., PersonalityController) and move the methods update_agent_personality and list_personality_presets out of SetupController into that file, preserving route decorators and guards (put "/agents/{agent_index:int}/personality" with require_ceo and get "/personality-presets" with require_read_access). Import the same dependencies used in those handlers (State/AppState, _check_setup_not_complete, _AGENT_LOCK, get_existing_agents, _validate_agent_index, get_personality_preset, PERSONALITY_PRESETS, logger, ApiResponse and response models like UpdateAgentPersonalityRequest, SetupAgentSummary, PersonalityPresetsListResponse, PersonalityPresetInfoResponse, and helper agent_dict_to_summary), wire the new controller into the app/router registration where SetupController is registered, and remove the moved methods from src/synthorg/api/controllers/setup.py so the file falls under 800 lines; run tests and update any imports or references to these methods if used elsewhere.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/design/operations.md`:
- Line 1111: Update the route group documentation so all setup-agent endpoints
use the same placeholder and note: replace `{index}` with `{agent_index}` across
the listed routes (`GET /api/v1/setup/agents`, `PUT
/api/v1/setup/agents/{agent_index}/model`, `PUT
/api/v1/setup/agents/{agent_index}/name`, `POST
/api/v1/setup/agents/{agent_index}/randomize-name`, `PUT
/api/v1/setup/agents/{agent_index}/personality`) and move the
zero-based/unstable-index note ("{agent_index} = zero-based position in the list
returned by GET /api/v1/setup/agents; not a stable ID -- re-fetch to resolve;
out-of-range returns 404") to apply to the whole route group so clients
understand the lookup contract consistently with the handler semantics in setup
controller code (e.g., the setup controller endpoints that perform agent
lookups).
In `@web/src/pages/setup/AccountStep.tsx`:
- Around line 24-35: The current useEffect swallows failures from getSetupStatus
and falls back to DEFAULT_MIN_PASSWORD_LENGTH, which masks contract errors;
change it to read status.min_password_length directly (no nullish fallback) and
handle errors explicitly by logging and surfacing them to UI/state (e.g., set an
error state or show an inline message) instead of an empty catch; update the
useEffect that calls getSetupStatus and the state setters (setMinPasswordLength)
so contract mismatches are visible, and remove the silent fallback to
DEFAULT_MIN_PASSWORD_LENGTH until the frontend type aligns with the backend.
In `@web/src/pages/setup/CompanyStep.tsx`:
- Around line 167-178: The "Generated Agents" manual card in CompanyStep.tsx
(rendered when companyResponse && agents.length > 0) should be replaced with the
shared SectionCard primitive: wrap the existing content in <SectionCard> using
its header/title prop or slot for "Generated Agents" and move the <ul> mapping
(agents.map with the same key `${agent.name}-${index}` and list item content)
into the SectionCard body; ensure existing classes/spacing are preserved by
using SectionCard’s props or adding minimal container classes, and remove the
hand-built <div className="rounded-lg border..."> block while keeping the
conditional rendering on companyResponse and agents.
In `@web/src/pages/setup/MiniOrgChart.tsx`:
- Around line 112-121: The height calculation currently uses maxAgentsInDept
which only reflects the largest single department and causes small teams
concentrated in one dept to inflate height; compute the total number of agents
(e.g., totalAgents = departments.reduce((s,d)=>s + d.agents.length, 0)) or the
actual number of rendered rows and use that value instead of maxAgentsInDept
when computing svgHeight and displayHeight, while leaving deptWidths and
svgHeight tied to the real content bounds so displayHeight = clamp(svgHeight,
MIN_HEIGHT, MAX_HEIGHT) reflects total team size; update references to
maxAgentsInDept in this file (MiniOrgChart.tsx) accordingly.
- Around line 20-26: The component currently hardcodes layout pixels
(LARGE_AVATAR_RADIUS, SMALL_AVATAR_RADIUS, LARGE_NODE_WIDTH, SMALL_NODE_WIDTH,
AGENT_SPACING_GAP and local derived values like nodeWidth, hGap, vGap) — replace
these hardcoded numbers with shared density-aware design tokens or centralized
layout constants (e.g., import p-card / gap-section-gap / gap-grid-gap or
existing spacing tokens from the design system or a shared constants file) and
use Tailwind spacing tokens where appropriate; update MiniOrgChart.tsx to import
those tokens and compute avatar radii/node width/gaps from them (and remove the
local numeric constants), and propagate the same change for the other
occurrences that compute nodeWidth/hGap/vGap so the chart follows global density
settings.
In `@web/src/pages/setup/ProviderAddForm.tsx`:
- Around line 8-12: The form currently lists presets regardless of auth_type but
only renders an API key input and forwards apiKey via onAdd; to fix, filter the
selectable presets to only those with auth_type === 'api_key' (apply this filter
where the component uses the presets prop and builds the preset
dropdown/selection in ProviderAddForm), or alternatively branch the form UI by
auth_type to render appropriate fields for 'oauth', 'subscription', or
'custom_header' and adapt onAdd accordingly; for the minimal safe fix, update
the preset list used in the selection to presets.filter(p => p.auth_type ===
'api_key') so only API-key-compatible presets are shown and onAdd remains
unchanged.
In `@web/src/pages/setup/ProviderProbeResults.tsx`:
- Around line 71-85: Currently addingPreset (state and setAddingPreset) is a
single string/null so concurrent adds race and clear each other; change
addingPreset to track per-preset pending state (e.g., a Set<string> or
Record<string, boolean> or string[]), update handleAdd to add the preset name to
that collection before awaiting onAddPreset and remove it in finally, and update
any UI checks that compare addingPreset === presetName to instead test
collection.has(presetName) (apply the same change to the other occurrence that
uses addingPreset and setAddingPreset).
In `@web/src/pages/setup/TemplateStep.tsx`:
- Around line 145-159: The mapped JSX for templates inside StaggerGroup is too
large and should be extracted into a small component: create a TemplateGridItem
component that renders the StaggerItem wrapper and the TemplateCard; give it
props for template, selectedTemplate (or selected boolean), comparedTemplates
(or compared boolean), recommendedTemplates (or recommended boolean), onSelect
(handleSelect), onToggleCompare (handleToggleCompare) and compareDisabled
(derived from MAX_COMPARE/comparedTemplates.length); replace the current
templates.map block with templates.map(t => <TemplateGridItem key={t.name}
template={t} ... />) so all multi-line JSX is moved out of the .map(),
preserving the same prop names/behaviour for StaggerItem and TemplateCard.
In `@web/src/pages/setup/WizardModeStep.tsx`:
- Around line 17-20: The SELECTED_SHADOW constant currently hardcodes the shadow
geometry and color-mix; replace its value with the shared shadow token (use
var(--so-shadow-card-hover) or the project's standard shadow variable) so it
uses the tokenized shadow instead of the literal string, and update any usage of
SELECTED_SHADOW in WizardModeStep.tsx to rely on that tokenized constant.
---
Outside diff comments:
In `@src/synthorg/api/controllers/setup_models.py`:
- Around line 223-232: The before-mode validators
SetupAgentRequest._validate_preset_exists and
UpdateAgentPersonalityRequest._validate_preset_exists should first confirm the
incoming payload is a mapping: add an isinstance(values, dict) check at the top
of each validator and if not a dict raise ValueError("request body must be an
object") (or similar clear message) so Pydantic treats it as a validation error;
after that continue to call values.get(...) and use
_normalize_and_validate_preset(...) as before.
In `@tests/unit/api/controllers/test_setup.py`:
- Around line 1407-1421: The test expectation is stale: when
settings_svc.get_entry raises SettingNotFoundError, read_name_locales now
returns the created default locale list instead of None; update the assertion to
check that result equals the default locale list produced by read_name_locales
(call _read_name_locales as done) rather than asserting None, referencing
read_name_locales/_read_name_locales and the mock of settings_svc.get_entry
(which raises SettingNotFoundError("company","name_locales")) to verify the
resolved default.
- Around line 1338-1351: The test expects False on get_entry failures but the
imported function check_has_name_locales (referenced here as
_check_has_name_locales) now logs and re-raises the RuntimeError; update the
test to assert that calling _check_has_name_locales(settings_svc) raises
RuntimeError instead of returning False (use pytest.raises or equivalent),
keeping the settings_svc.get_entry AsyncMock(side_effect=RuntimeError("db
connection lost")) setup and restoring original get_entry after the test.
In `@web/src/pages/setup/MiniOrgChart.tsx`:
- Around line 161-204: The deptPositions.map() inline JSX inside MiniOrgChart is
too large and should be extracted into a dedicated component (e.g.
DepartmentGroup or DepartmentNode) to isolate SVG layout and simplify testing;
create a new component that accepts a single pos (the same shape used in
deptPositions), renders the department rect and label and maps pos.dept.agents
to AgentNode (preserving props agent, agentX, agentY, deptX, deptY, radius and
the current key pattern), then replace the inline JSX in MiniOrgChart's
deptPositions.map(...) with a call to the new component, passing
nodeWidth/nodeHeight/avatarRadius/vGap/AGENT_SPACING_GAP as needed or deriving
them from props so layout logic is moved out of MiniOrgChart.
- Around line 46-76: AgentNode currently draws the avatar manually (circle +
text) which deviates from the shared Avatar implementation; replace the manual
drawing with the shared Avatar component (or an SVG-friendly adapter/wrapper) so
avatar fallback, styling and accessibility are consistent. In the AgentNode
function, remove the <circle> and <text> initials rendering and instead render
<Avatar name={agent.name} title={agent.role} size={radius * 2} ...> (or mount
Avatar inside a foreignObject / AvatarSvg adapter if you need exact SVG
positioning) and pass the agent props (agent.name, agent.role) and the computed
size/position (agentX, agentY, radius) so the avatar is centered where the
circle used to be. Ensure the linking line (<line>) and any ARIA/title behavior
remains unchanged and that getInitials is no longer used here.
In `@web/src/pages/setup/ProvidersStep.tsx`:
- Around line 34-199: ProvidersStep is too large and mixes orchestration,
side-effects, handlers and UI; extract the fetch/probe/validation logic into a
custom hook (e.g., useProvidersSetup) and lift major UI sections into small
components to keep ProvidersStep under 50 lines. Move the mounting fetch logic
that calls fetchProviders and fetchPresets, the auto-probe logic that uses
probeAllPresets and probeAttemptedRef, the reprobe handler (reprobePresets), the
create handlers (createProviderFromPreset + fetchProviders) and the validation
side-effect that calls validateProvidersStep/markStepComplete/markStepIncomplete
into useProvidersSetup; have that hook return state values (providers, presets,
probeResults, probing, loading/error flags) and action handlers
(handleAddPreset, handleAddCloud, handleReprobe). Then refactor ProvidersStep to
only compose the returned state/handlers into small presentation components
(e.g., LoadingState, ProviderProbeResults, ProviderAddForm, ConfiguredProviders,
ValidationMessages) so the top-level component is concise and focused on
rendering.
---
Duplicate comments:
In `@src/synthorg/api/controllers/setup.py`:
- Around line 607-708: The SetupController file exceeds the size cap—extract the
personality-related endpoints into a new controller module: create a new
controller (e.g., PersonalityController) and move the methods
update_agent_personality and list_personality_presets out of SetupController
into that file, preserving route decorators and guards (put
"/agents/{agent_index:int}/personality" with require_ceo and get
"/personality-presets" with require_read_access). Import the same dependencies
used in those handlers (State/AppState, _check_setup_not_complete, _AGENT_LOCK,
get_existing_agents, _validate_agent_index, get_personality_preset,
PERSONALITY_PRESETS, logger, ApiResponse and response models like
UpdateAgentPersonalityRequest, SetupAgentSummary,
PersonalityPresetsListResponse, PersonalityPresetInfoResponse, and helper
agent_dict_to_summary), wire the new controller into the app/router registration
where SetupController is registered, and remove the moved methods from
src/synthorg/api/controllers/setup.py so the file falls under 800 lines; run
tests and update any imports or references to these methods if used elsewhere.
🪄 Autofix (Beta)
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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c1ced38c-2117-4966-9e1d-f1dcd6ce1942
📒 Files selected for processing (39)
README.mddocs/design/operations.mddocs/design/organization.mddocs/design/page-structure.mddocs/user_guide.mdsrc/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/setup_helpers.pysrc/synthorg/api/controllers/setup_models.pysrc/synthorg/observability/events/setup.pytests/unit/api/controllers/test_setup.pyweb/src/__tests__/pages/setup/WizardProgress.test.tsxweb/src/__tests__/stores/setup-wizard.test.tsweb/src/__tests__/utils/cost-estimator.property.test.tsweb/src/__tests__/utils/cost-estimator.test.tsweb/src/api/endpoints/setup.tsweb/src/api/types.tsweb/src/components/ui/stat-pill.tsxweb/src/pages/setup/AccountStep.tsxweb/src/pages/setup/AgentsStep.tsxweb/src/pages/setup/CompanyStep.tsxweb/src/pages/setup/CompleteStep.tsxweb/src/pages/setup/CostEstimatePanel.tsxweb/src/pages/setup/MiniOrgChart.tsxweb/src/pages/setup/ProviderAddForm.tsxweb/src/pages/setup/ProviderProbeResults.tsxweb/src/pages/setup/ProvidersStep.tsxweb/src/pages/setup/SetupAgentCard.tsxweb/src/pages/setup/SetupSummary.tsxweb/src/pages/setup/TemplateCard.tsxweb/src/pages/setup/TemplateCategoryGroup.tsxweb/src/pages/setup/TemplateCompareDrawer.tsxweb/src/pages/setup/TemplateCostBadge.tsxweb/src/pages/setup/TemplateStep.tsxweb/src/pages/setup/TemplateVariables.tsxweb/src/pages/setup/WizardModeStep.tsxweb/src/pages/setup/WizardProgress.tsxweb/src/pages/setup/WizardShell.tsxweb/src/stores/setup-wizard.tsweb/src/utils/cost-estimator.ts
💤 Files with no reviewable changes (7)
- web/src/pages/setup/TemplateCompareDrawer.tsx
- web/src/pages/setup/TemplateCostBadge.tsx
- web/src/tests/utils/cost-estimator.test.ts
- web/src/pages/setup/CostEstimatePanel.tsx
- web/src/pages/setup/TemplateCategoryGroup.tsx
- web/src/utils/cost-estimator.ts
- web/src/tests/utils/cost-estimator.property.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Dashboard Test
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (7)
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
web/src/**/*.{ts,tsx}: Use design tokens and semantic classes (text-foreground,bg-card,text-accent,text-success,bg-danger, etc.) or CSS variables (var(--so-accent)) -- NEVER hardcode hex values or rgba in.tsx/.tsfiles
Usefont-sansorfont-monofor typography (maps to Geist tokens) -- NEVER setfontFamilydirectly
Use density-aware spacing tokens (p-card,gap-section-gap,gap-grid-gap) or standard Tailwind spacing -- NEVER hardcode pixel values for layout spacing
Use token variables for shadows/borders (var(--so-shadow-card-hover),border-border,border-bright) -- NEVER hardcode values
Do NOT recreate status dots inline -- use<StatusBadge>; do NOT build card-with-header layouts from scratch -- use<SectionCard>; do NOT create metric displays with text-metric classes -- use<MetricCard>
Do NOT render initials circles manually -- use<Avatar>; do NOT create complex (>8 line) JSX inside.map()-- extract to a shared component
Use TypeScript 6.0 features: remove deprecatedbaseUrl, accept thatesModuleInteropis always true, explicitly listtypesif needed, use"bundler"or"nodenext"formoduleResolution
Never use real vendor names (OpenAI, Claude, GPT, Anthropic) in web dashboard code -- use generic names:example-provider,example-large-001, etc.
Keep functions under 50 lines and files under 800 lines
Handle errors explicitly, never silently swallow exceptions
Files:
web/src/components/ui/stat-pill.tsxweb/src/pages/setup/WizardProgress.tsxweb/src/__tests__/pages/setup/WizardProgress.test.tsxweb/src/pages/setup/CompanyStep.tsxweb/src/pages/setup/WizardModeStep.tsxweb/src/pages/setup/TemplateStep.tsxweb/src/pages/setup/TemplateVariables.tsxweb/src/pages/setup/WizardShell.tsxweb/src/api/endpoints/setup.tsweb/src/pages/setup/MiniOrgChart.tsxweb/src/pages/setup/AccountStep.tsxweb/src/pages/setup/SetupSummary.tsxweb/src/pages/setup/ProviderAddForm.tsxweb/src/pages/setup/ProvidersStep.tsxweb/src/pages/setup/AgentsStep.tsxweb/src/pages/setup/SetupAgentCard.tsxweb/src/pages/setup/TemplateCard.tsxweb/src/__tests__/stores/setup-wizard.test.tsweb/src/pages/setup/ProviderProbeResults.tsxweb/src/api/types.tsweb/src/pages/setup/CompleteStep.tsxweb/src/stores/setup-wizard.ts
web/src/components/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always reuse existing shared components from
web/src/components/ui/before creating new ones; export props as TypeScript interfaces; use design tokens exclusively; importcnfrom@/lib/utilsfor conditional class merging
Files:
web/src/components/ui/stat-pill.tsx
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Design spec pages in
docs/design/are mandatory references before implementing features -- ALWAYS read the relevant design page
Files:
docs/design/organization.mddocs/design/page-structure.mddocs/user_guide.mddocs/design/operations.md
docs/design/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
When approved deviations from the design spec occur during implementation, update the relevant
docs/design/page to reflect the new reality
Files:
docs/design/organization.mddocs/design/page-structure.mddocs/design/operations.md
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations-- Python 3.14 has PEP 649 native lazy annotations
Useexcept A, B:syntax without parentheses (PEP 758 except syntax) -- ruff enforces this on Python 3.14
All public functions must have type hints; mypy strict mode is enforced
Use Google-style docstrings on all public classes and functions (enforced by ruff D rules)
Never useimport logging/logging.getLogger()/print()in application code -- exception:observability/setup.pyandobservability/sinks.pymay use stdlib logging and print for bootstrap code
Always use structured logging with variable names as kwargs:logger.info(EVENT, key=value)-- never uselogger.info("msg %s", val)
Log all error paths at WARNING or ERROR with context before raising; log all state transitions at INFO; log DEBUG for object creation and internal flow
Create new objects instead of mutating existing ones; for non-Pydantic internal collections usecopy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcement
Fordict/listfields in frozen Pydantic models, usecopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization)
Use frozen Pydantic v2 models for config and identity; use separate mutable models (withmodel_copy(update=...)) for runtime state that evolves
Use@computed_fieldfor derived values instead of storing redundant fields; useNotBlankStrfor all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls) over barecreate_task
Keep functions under 50 lines and files under 800 lines
Line length must be 88 characters (enforced by ruff)
Handle errors explicitly, never silently swallow exceptions
Validate input at system boundaries (user input, external APIs, config files)
Files:
src/synthorg/observability/events/setup.pysrc/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/setup_models.pysrc/synthorg/api/controllers/setup_helpers.pytests/unit/api/controllers/test_setup.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic must importfrom synthorg.observability import get_loggerand createlogger = get_logger(__name__)
Use event name constants from domain-specific modules undersynthorg.observability.events(e.g.,API_REQUEST_STARTEDfromevents.api) instead of hardcoded event strings
ALWAYS read the relevantdocs/design/page before implementing any feature or planning any issue; the design spec is the starting point for architecture, data models, and behavior
If implementation deviates from the design spec, alert the user and explain why -- user decides whether to proceed or update the spec; never silently diverge
Files:
src/synthorg/observability/events/setup.pysrc/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/setup_models.pysrc/synthorg/api/controllers/setup_helpers.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Mark all tests with@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, or@pytest.mark.slow
Maintain 80% minimum code coverage (enforced in CI)
Never use real vendor names (Anthropic, OpenAI, Claude, GPT) -- use generic names:example-provider,example-large-001,example-medium-001,example-small-001, or test aliases liketest-provider,test-small-001
For timing-sensitive tests, mocktime.monotonic()andasyncio.sleep()to make them deterministic instead of widening timing margins; for tasks that must block indefinitely, useasyncio.Event().wait()instead ofasyncio.sleep(large_number)
Never skip, dismiss, or ignore flaky tests -- always fix them fully and fundamentally
Files:
tests/unit/api/controllers/test_setup.py
🧠 Learnings (37)
📚 Learning: 2026-03-28T16:54:20.164Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T16:54:20.164Z
Learning: Applies to web/src/components/ui/**/*.{ts,tsx} : Always reuse existing shared components from `web/src/components/ui/` before creating new ones; export props as TypeScript interfaces; use design tokens exclusively; import `cn` from `@/lib/utils` for conditional class merging
Applied to files:
web/src/components/ui/stat-pill.tsxweb/src/pages/setup/MiniOrgChart.tsxweb/src/pages/setup/CompleteStep.tsx
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : Package structure: src/synthorg/ organized as: api/ (REST+WebSocket, Litestar), auth/ (auth subpackage), backup/ (scheduled/manual backups), budget/ (cost tracking, CFO), cli/ (superseded by Go CLI), communication/ (message bus, meetings), config/ (YAML loading), core/ (domain models, resilience config), engine/ (orchestration, task state, coordination, approval gates, stagnation detection, context budget, compaction), hr/ (hiring, performance, promotion), memory/ (pluggable backend, Mem0, retrieval, consolidation), persistence/ (operational data, SQLite, settings), observability/ (logging, correlation, sinks), providers/ (LLM abstraction, LiteLLM, auth types, presets, runtime CRUD), settings/ (runtime-editable, typed definitions, encryption, config bridge), security/ (SecOps, rule engine, output scanning, progressive trust, autonomy levels), templates/ (company templates, personalities), tools/ (registry, built-in tools, git, sandbox, code_runner, MCP...
Applied to files:
README.mddocs/design/operations.mdsrc/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/setup_helpers.py
📚 Learning: 2026-03-26T15:18:16.848Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T15:18:16.848Z
Learning: Applies to src/synthorg/api/**/*.py : Litestar API must include setup wizard, auth/, auto-wiring, and lifecycle management
Applied to files:
README.mddocs/user_guide.mdsrc/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/setup_helpers.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to docker/Dockerfile.sandbox : Docker sandbox: `synthorg-sandbox` — Python 3.14 + Node.js + git, non-root (UID 10001), agent code execution sandbox
Applied to files:
README.md
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to docs/design/**/*.md : Design specification pages in `docs/design/` must be consulted before implementing features (7 pages: index, agents, organization, communication, engine, memory, operations)
Applied to files:
docs/design/organization.mddocs/design/page-structure.md
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to docs/design/*.md : Design spec pages: 7 pages in `docs/design/` — index, agents, organization, communication, engine, memory, operations
Applied to files:
docs/design/organization.mddocs/design/page-structure.md
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to src/synthorg/templates/**/*.py : Templates: pre-built company templates, personality presets, and builder.
Applied to files:
docs/design/organization.mdsrc/synthorg/api/controllers/setup_models.pyweb/src/stores/setup-wizard.tssrc/synthorg/api/controllers/setup_helpers.py
📚 Learning: 2026-03-28T16:54:20.164Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T16:54:20.164Z
Learning: Applies to web/src/**/*.stories.tsx : Use Storybook 10 ESM-only imports: `storybook/test` (not `storybook/test`), `storybook/actions` (not `storybook/addon-actions`); use `definePreview` from `storybook/react-vite`; set `parameters.a11y.test` for WCAG compliance enforcement
Applied to files:
web/src/__tests__/pages/setup/WizardProgress.test.tsxweb/src/pages/setup/WizardModeStep.tsxweb/src/pages/setup/WizardShell.tsxweb/src/pages/setup/AgentsStep.tsxweb/src/__tests__/stores/setup-wizard.test.tsweb/src/pages/setup/CompleteStep.tsx
📚 Learning: 2026-03-28T16:54:20.163Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T16:54:20.163Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`) instead of hardcoded event strings
Applied to files:
src/synthorg/observability/events/setup.py
📚 Learning: 2026-03-28T16:54:20.164Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T16:54:20.164Z
Learning: Applies to web/src/**/*.{ts,tsx} : Never use real vendor names (OpenAI, Claude, GPT, Anthropic) in web dashboard code -- use generic names: `example-provider`, `example-large-001`, etc.
Applied to files:
web/src/pages/setup/CompanyStep.tsxweb/src/stores/setup-wizard.ts
📚 Learning: 2026-03-28T16:54:20.164Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T16:54:20.164Z
Learning: Applies to web/src/components/ui/**/*.{tsx} : Create a `.stories.tsx` file alongside each new shared component with all states (default, hover, loading, error, empty) for Storybook documentation
Applied to files:
web/src/pages/setup/WizardModeStep.tsx
📚 Learning: 2026-03-28T16:54:20.164Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T16:54:20.164Z
Learning: Applies to web/src/**/*.{ts,tsx} : Use token variables for shadows/borders (`var(--so-shadow-card-hover)`, `border-border`, `border-bright`) -- NEVER hardcode values
Applied to files:
web/src/pages/setup/WizardModeStep.tsxweb/src/pages/setup/TemplateVariables.tsxweb/src/pages/setup/MiniOrgChart.tsx
📚 Learning: 2026-03-28T16:54:20.164Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T16:54:20.164Z
Learning: Applies to web/src/**/*.{ts,tsx} : Keep functions under 50 lines and files under 800 lines
Applied to files:
web/src/pages/setup/WizardModeStep.tsxweb/src/pages/setup/MiniOrgChart.tsxweb/src/stores/setup-wizard.ts
📚 Learning: 2026-03-21T11:08:01.542Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-21T11:08:01.542Z
Learning: Applies to **/*.{py,md} : Line length: 88 characters (ruff).
Applied to files:
web/src/pages/setup/WizardModeStep.tsxweb/src/stores/setup-wizard.ts
📚 Learning: 2026-03-28T16:54:20.164Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T16:54:20.164Z
Learning: Applies to web/src/**/*.{ts,tsx} : Use design tokens and semantic classes (`text-foreground`, `bg-card`, `text-accent`, `text-success`, `bg-danger`, etc.) or CSS variables (`var(--so-accent)`) -- NEVER hardcode hex values or rgba in `.tsx`/`.ts` files
Applied to files:
web/src/pages/setup/WizardModeStep.tsxweb/src/pages/setup/MiniOrgChart.tsx
📚 Learning: 2026-03-28T16:54:20.164Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T16:54:20.164Z
Learning: Applies to web/src/**/*.{ts,tsx} : Do NOT recreate status dots inline -- use `<StatusBadge>`; do NOT build card-with-header layouts from scratch -- use `<SectionCard>`; do NOT create metric displays with text-metric classes -- use `<MetricCard>`
Applied to files:
web/src/pages/setup/WizardModeStep.tsxweb/src/pages/setup/TemplateStep.tsxweb/src/pages/setup/MiniOrgChart.tsxweb/src/pages/setup/SetupSummary.tsxweb/src/pages/setup/TemplateCard.tsx
📚 Learning: 2026-03-28T16:54:20.164Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T16:54:20.164Z
Learning: Applies to web/src/**/*.{ts,tsx} : Do NOT render initials circles manually -- use `<Avatar>`; do NOT create complex (>8 line) JSX inside `.map()` -- extract to a shared component
Applied to files:
web/src/pages/setup/WizardShell.tsxweb/src/pages/setup/MiniOrgChart.tsxweb/src/pages/setup/SetupSummary.tsxweb/src/pages/setup/CompleteStep.tsx
📚 Learning: 2026-03-28T16:54:20.164Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T16:54:20.164Z
Learning: Applies to web/src/**/*.{ts,tsx} : Use density-aware spacing tokens (`p-card`, `gap-section-gap`, `gap-grid-gap`) or standard Tailwind spacing -- NEVER hardcode pixel values for layout spacing
Applied to files:
web/src/pages/setup/MiniOrgChart.tsx
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/api/**/*.py : API package (api/): Litestar REST + WebSocket with controllers, guards, channels, JWT + API key + WS ticket auth, approval gate integration, coordination endpoint, collaboration endpoint, settings endpoint, provider management endpoint (CRUD + test + presets), backup endpoint, RFC 9457 structured errors, AppState hot-reload slots, service auto-wiring (Phase 1 at construction, Phase 2 on startup), lifecycle helpers
Applied to files:
docs/design/operations.mdsrc/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/setup_helpers.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Documentation source in `docs/` (Markdown, built with Zensical). Design spec in `docs/design/` (7 pages: index, agents, organization, communication, engine, memory, operations). Architecture in `docs/architecture/` (overview, tech-stack, decision log). Roadmap in `docs/roadmap/`. Security in `docs/security.md`. Licensing in `docs/licensing.md`. Reference in `docs/reference/`. REST API reference in `docs/rest-api.md`. Library reference in `docs/api/` (auto-generated from docstrings). Custom templates in `docs/overrides/`. Config in `mkdocs.yml`.
Applied to files:
docs/design/operations.md
📚 Learning: 2026-03-15T21:20:09.993Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:20:09.993Z
Learning: Applies to web/src/components/** : Vue components organized by feature (agents/, approvals/, budget/, common/, dashboard/, layout/, messages/, org-chart/, tasks/).
Applied to files:
web/src/pages/setup/SetupSummary.tsx
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (via `model_copy(update=...)`) for runtime state that evolves
Applied to files:
src/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/setup_models.py
📚 Learning: 2026-03-16T20:14:00.937Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T20:14:00.937Z
Learning: Applies to **/*.py : Validate: at system boundaries (user input, external APIs, config files).
Applied to files:
src/synthorg/api/controllers/setup.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Handle errors explicitly, never silently swallow. Validate at system boundaries (user input, external APIs, config files).
Applied to files:
src/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/setup_models.py
📚 Learning: 2026-03-19T07:13:44.964Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:13:44.964Z
Learning: Applies to **/*.py : Validate at system boundaries (user input, external APIs, config files)
Applied to files:
src/synthorg/api/controllers/setup.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/api/**/*.py : REST API: Litestar framework, controllers with guards, channels for WebSocket, JWT + API key + WS ticket auth, approval gate integration, coordination endpoint, collaboration endpoint, settings endpoint. RFC 9457 structured errors (ErrorCategory, ErrorCode, ErrorDetail, ProblemDetail, CATEGORY_TITLES, category_title, category_type_uri, content negotiation).
Applied to files:
src/synthorg/api/controllers/setup.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/api/**/*.py : Use Litestar for REST + WebSocket API. Controllers, guards, channels, JWT + API key + WS ticket auth, RFC 9457 structured errors.
Applied to files:
src/synthorg/api/controllers/setup.py
📚 Learning: 2026-03-20T11:18:48.128Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T11:18:48.128Z
Learning: Applies to src/synthorg/api/**/*.py : Use Litestar for REST API and WebSocket API with JWT + API key + WS ticket authentication, RFC 9457 structured errors, and content negotiation.
Applied to files:
src/synthorg/api/controllers/setup.py
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to src/synthorg/providers/**/*.py : Providers: LLM provider abstraction (LiteLLM adapter), auth types (api_key/oauth/custom_header/none), presets (PROVIDER_PRESETS), runtime CRUD (ProviderManagementService with asyncio.Lock serialization), hot-reload via AppState swap.
Applied to files:
src/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/setup_helpers.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`. For derived values use `computed_field` instead of storing + validating redundant fields. Use `NotBlankStr` (from `core.types`) for all identifier/name fields — including optional (`NotBlankStr | None`) and tuple (`tuple[NotBlankStr, ...]`) variants — instead of manual whitespace validators.
Applied to files:
src/synthorg/api/controllers/setup_models.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; separate mutable-via-copy models (using `model_copy(update=...)`) for runtime state
Applied to files:
src/synthorg/api/controllers/setup_models.py
📚 Learning: 2026-03-28T16:54:20.164Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T16:54:20.164Z
Learning: Applies to src/synthorg/**/*.py : If implementation deviates from the design spec, alert the user and explain why -- user decides whether to proceed or update the spec; never silently diverge
Applied to files:
src/synthorg/api/controllers/setup_models.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions.
Applied to files:
src/synthorg/api/controllers/setup_models.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, and key function entry/exit
Applied to files:
src/synthorg/api/controllers/setup_models.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Applied to files:
src/synthorg/api/controllers/setup_models.py
📚 Learning: 2026-03-17T06:43:14.114Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:43:14.114Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions. Pure data models, enums, and re-exports do NOT need logging.
Applied to files:
src/synthorg/api/controllers/setup_models.py
📚 Learning: 2026-03-19T07:13:44.964Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:13:44.964Z
Learning: Applies to src/synthorg/hr/**/*.py : HR package (hr/): hiring, firing, onboarding, offboarding, agent registry, performance tracking (task metrics, collaboration scoring, LLM calibration, collaboration overrides, trend detection), promotion/demotion (criteria evaluation, approval strategies, model mapping)
Applied to files:
src/synthorg/api/controllers/setup_helpers.py
🔇 Additional comments (11)
web/src/components/ui/stat-pill.tsx (1)
3-5: Nice fix for empty-label pills.Making
labeloptional and only rendering the label span when it exists removes the stray leading gap without changing behavior for callers that still provide a label.Also applies to: 17-21
web/src/pages/setup/CompleteStep.tsx (1)
25-30: The failure path is already correctly exposed—no changes needed.The dialog does close on failed confirms. When
wizardCompleteSetup()throws,handleCompletecatches the error without rethrowing, causing the promise to resolve successfully. This allowsConfirmDialogto executeonOpenChange(false), closing the dialog. ThecompletionErrorfrom the store is then visible in the rendered alert below, so users see the failure state.web/src/__tests__/pages/setup/WizardProgress.test.tsx (1)
6-14: Fixture update aligns with the new wizard flow.The step order and completion defaults are consistent with the mode-aware, providers-before-agents setup progression.
README.md (1)
110-110: Setup wizard docs are updated correctly.This line now matches the branched setup behavior and step ordering implemented in the wizard.
web/src/api/types.ts (1)
1282-1293: New setup personality DTOs are clean and consistent.The request/response typing is clear and supports the new personality preset API surface well.
web/src/api/endpoints/setup.ts (1)
87-104: New setup personality endpoint wrappers look solid.The index validation and typed unwrapping follow the same robust pattern as adjacent setup API helpers.
web/src/pages/setup/AgentsStep.tsx (1)
36-50: Preset loading/error handling is now robust.Good fix on the fetch guard plus explicit retry UI for personality presets; this avoids re-fetch loops and gives users recovery control.
Also applies to: 139-152
web/src/pages/setup/WizardShell.tsx (1)
30-31: Mode-step shell integration is clean.Hiding progress/navigation on the mode gate while keeping step routing/store sync intact is implemented well.
Also applies to: 47-51, 99-135
docs/user_guide.md (1)
69-84: Nice doc refresh for the branched wizard flow.The updated step list and completion requirements line up with the guided/quick behavior described elsewhere in this PR.
web/src/stores/setup-wizard.ts (1)
325-348: Nice quick-mode reset.Clearing the template-derived state and skipped-step completion flags here closes the stale-template path cleanly and keeps
submitCompany()aligned with the active flow.web/src/__tests__/stores/setup-wizard.test.ts (1)
127-164: Good regression coverage on the mode-dependent flow.These cases exercise the dynamic
stepOrderbranches and the quick-mode guard onsetStep, which are the key store regressions from this change.
| // Read backend-configured min password length | ||
| useEffect(() => { | ||
| getSetupStatus() | ||
| .then((status) => { | ||
| setMinPasswordLength( | ||
| status.min_password_length ?? DEFAULT_MIN_PASSWORD_LENGTH, | ||
| ) | ||
| }) | ||
| .catch(() => { | ||
| // Fall back to default on error | ||
| }) | ||
| }, []) |
There was a problem hiding this comment.
Don't silently mask /setup/status contract failures.
min_password_length is a required field in the setup-status response, so the ?? DEFAULT_MIN_PASSWORD_LENGTH fallback plus the empty catch here turns a broken status fetch into a silent downgrade of the UI hint/validation. Please surface the fetch failure explicitly and read the field directly once the frontend type matches the backend contract.
As per coding guidelines, "Handle errors explicitly, never silently swallow exceptions"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/pages/setup/AccountStep.tsx` around lines 24 - 35, The current
useEffect swallows failures from getSetupStatus and falls back to
DEFAULT_MIN_PASSWORD_LENGTH, which masks contract errors; change it to read
status.min_password_length directly (no nullish fallback) and handle errors
explicitly by logging and surfacing them to UI/state (e.g., set an error state
or show an inline message) instead of an empty catch; update the useEffect that
calls getSetupStatus and the state setters (setMinPasswordLength) so contract
mismatches are visible, and remove the silent fallback to
DEFAULT_MIN_PASSWORD_LENGTH until the frontend type aligns with the backend.
| /** Layout constants. */ | ||
| const SMALL_TEAM_THRESHOLD = 5 | ||
| const LARGE_AVATAR_RADIUS = 16 | ||
| const SMALL_AVATAR_RADIUS = 14 | ||
| const LARGE_NODE_WIDTH = 110 | ||
| const SMALL_NODE_WIDTH = 90 | ||
| const AGENT_SPACING_GAP = 10 |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Stop baking chart geometry into local pixel constants.
The new layout math still hardcodes raw measurements in TSX (LARGE_AVATAR_RADIUS, SMALL_AVATAR_RADIUS, nodeWidth, hGap, vGap, etc.), so any future density change will drift this chart out of sync with the rest of the setup UI. Please move these values behind shared design tokens or centralized layout constants.
As per coding guidelines: "Use density-aware spacing tokens (p-card, gap-section-gap, gap-grid-gap) or standard Tailwind spacing -- NEVER hardcode pixel values for layout spacing".
Also applies to: 101-109
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/pages/setup/MiniOrgChart.tsx` around lines 20 - 26, The component
currently hardcodes layout pixels (LARGE_AVATAR_RADIUS, SMALL_AVATAR_RADIUS,
LARGE_NODE_WIDTH, SMALL_NODE_WIDTH, AGENT_SPACING_GAP and local derived values
like nodeWidth, hGap, vGap) — replace these hardcoded numbers with shared
density-aware design tokens or centralized layout constants (e.g., import p-card
/ gap-section-gap / gap-grid-gap or existing spacing tokens from the design
system or a shared constants file) and use Tailwind spacing tokens where
appropriate; update MiniOrgChart.tsx to import those tokens and compute avatar
radii/node width/gaps from them (and remove the local numeric constants), and
propagate the same change for the other occurrences that compute
nodeWidth/hGap/vGap so the chart follows global density settings.
| const maxAgentsInDept = Math.max(...departments.map((d) => d.agents.length), 1) | ||
| const deptWidths = departments.map((d) => | ||
| Math.max(NODE_WIDTH, d.agents.length * (AVATAR_RADIUS * 2 + 8)), | ||
| Math.max(nodeWidth, d.agents.length * (avatarRadius * 2 + AGENT_SPACING_GAP)), | ||
| ) | ||
| const totalWidth = deptWidths.reduce((sum, w) => sum + w + H_GAP, 0) - H_GAP | ||
| const svgWidth = Math.max(totalWidth + 40, 200) | ||
| const svgHeight = V_GAP * 2 + NODE_HEIGHT + maxAgentsInDept * (AVATAR_RADIUS * 2 + 4) + 20 | ||
| const totalWidth = deptWidths.reduce((sum, w) => sum + w + hGap, 0) - hGap | ||
| const svgWidth = Math.max(totalWidth + 40, 300) | ||
| const svgHeight = vGap * 2 + nodeHeight + maxAgentsInDept * (avatarRadius * 2 + 6) + 24 | ||
|
|
||
| // Dynamic height: scale with team, capped at bounds | ||
| const displayHeight = Math.min(Math.max(svgHeight, MIN_HEIGHT), MAX_HEIGHT) |
There was a problem hiding this comment.
Scale the chart height from total team size, not the largest department.
Every department still renders a single agent row, so maxAgentsInDept only adds blank vertical space. That makes a small org concentrated in one department render much taller than a larger org spread across several departments, which breaks the new “scale by team size” behavior. Drive displayHeight from agents.length (or actual rendered rows) and keep svgHeight tied to the real content bounds.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/pages/setup/MiniOrgChart.tsx` around lines 112 - 121, The height
calculation currently uses maxAgentsInDept which only reflects the largest
single department and causes small teams concentrated in one dept to inflate
height; compute the total number of agents (e.g., totalAgents =
departments.reduce((s,d)=>s + d.agents.length, 0)) or the actual number of
rendered rows and use that value instead of maxAgentsInDept when computing
svgHeight and displayHeight, while leaving deptWidths and svgHeight tied to the
real content bounds so displayHeight = clamp(svgHeight, MIN_HEIGHT, MAX_HEIGHT)
reflects total team size; update references to maxAgentsInDept in this file
(MiniOrgChart.tsx) accordingly.
| const [addingPreset, setAddingPreset] = useState<string | null>(null) | ||
|
|
||
| const localPresets = presets.filter((p) => p.auth_type === 'none') | ||
|
|
||
| if (localPresets.length === 0) return null | ||
|
|
||
| const handleAdd = async (presetName: string) => { | ||
| setAddingPreset(presetName) | ||
| try { | ||
| await onAddPreset(presetName) | ||
| } catch { | ||
| // Error already set in store (providersError) and rendered by ProvidersStep | ||
| } finally { | ||
| setAddingPreset(null) | ||
| } |
There was a problem hiding this comment.
Track pending adds per preset, not with a single string.
addingPreset only models one in-flight request. If a user clicks provider B while A is still adding, A's finally resets the flag to null, so B loses its loading state and can be submitted again while its first request is still pending.
Suggested fix
- const [addingPreset, setAddingPreset] = useState<string | null>(null)
+ const [addingPresets, setAddingPresets] = useState<ReadonlySet<string>>(
+ () => new Set(),
+ )
const handleAdd = async (presetName: string) => {
- setAddingPreset(presetName)
+ setAddingPresets((current) => {
+ const next = new Set(current)
+ next.add(presetName)
+ return next
+ })
try {
await onAddPreset(presetName)
} catch {
// Error already set in store (providersError) and rendered by ProvidersStep
} finally {
- setAddingPreset(null)
+ setAddingPresets((current) => {
+ const next = new Set(current)
+ next.delete(presetName)
+ return next
+ })
}
}
...
- adding={addingPreset === preset.name}
+ adding={addingPresets.has(preset.name)}Also applies to: 109-118
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/pages/setup/ProviderProbeResults.tsx` around lines 71 - 85, Currently
addingPreset (state and setAddingPreset) is a single string/null so concurrent
adds race and clear each other; change addingPreset to track per-preset pending
state (e.g., a Set<string> or Record<string, boolean> or string[]), update
handleAdd to add the preset name to that collection before awaiting onAddPreset
and remove it in finally, and update any UI checks that compare addingPreset ===
presetName to instead test collection.has(presetName) (apply the same change to
the other occurrence that uses addingPreset and setAddingPreset).
| const SELECTED_SHADOW = | ||
| 'shadow-[0_0_12px_color-mix(' + | ||
| 'in_srgb,var(--so-accent)_15%,transparent)]' | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace the hardcoded selected shadow with a tokenized shadow value.
The current class hardcodes shadow geometry/color mix. Use the shared shadow token instead.
♻️ Proposed fix
-const SELECTED_SHADOW =
- 'shadow-[0_0_12px_color-mix(' +
- 'in_srgb,var(--so-accent)_15%,transparent)]'
+const SELECTED_SHADOW = 'shadow-[var(--so-shadow-card-hover)]'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/pages/setup/WizardModeStep.tsx` around lines 17 - 20, The
SELECTED_SHADOW constant currently hardcodes the shadow geometry and color-mix;
replace its value with the shared shadow token (use var(--so-shadow-card-hover)
or the project's standard shadow variable) so it uses the tokenized shadow
instead of the literal string, and update any usage of SELECTED_SHADOW in
WizardModeStep.tsx to rely on that tokenized constant.
… fixes Comprehensive UX improvements to the first-run setup wizard: - Replace skip-wizard link with Guided/Quick mode choice step - Swap step order: Providers before Agents (model picker now works) - Remove all misleading cost estimates from every wizard step - Flatten template grid, add inline category tags per card - Fix StatPill leading space on empty labels - Make template variable labels currency-aware (dynamic, not hardcoded USD) - Dynamic org chart sizing (scales 140-500px based on team size) - Add personality preset editing (backend endpoints + dropdown UI) - Fix ProvidersStep rate limiting (no more "Too Many Requests") - Auto-discover models after provider creation (fixes "0 models" bug) - Add re-probe button for local provider detection - Prevent duplicate provider names with inline validation - Relax backend agent requirement for Quick Setup mode - Update design spec to reflect new wizard flow Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Pre-reviewed by 7 agents, 16 findings addressed: - Fix stale docstring on complete_setup (contradicted relaxed agent req) - Add status_code=HTTP_200_OK to personality endpoint for consistency - Auto-advance WizardModeStep after selection (was stuck with no nav) - Catch unhandled promise rejection in ProviderProbeResults.handleAdd - Remove non-functional Test Connection button (provider doesn't exist yet) - Display personalityPresetsError in AgentsStep - Remove incorrect template.tags.length as agent count in TemplateCard - Update user_guide.md for new wizard flow (mode gate, step order, no costs) - Delete orphaned cost-estimator.ts and its test files - Log actual error in createProviderFromPreset discovery catch - Log failed probes in reprobePresets (was silently dropping) - Add retry button for providersError in ProvidersStep - Add comment to CompleteStep bare catch explaining error flow - Guard setStep against steps not in current stepOrder - Mark PersonalityPresetInfo fields as readonly - Extract duplicate preset validation (kept as-is, minor maintenance risk) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… Gemini Critical fixes: - Persist full personality dict (not just preset name) on agent update - Change _check_has_agents to non-strict for Quick Setup completion - Extract shared preset validator to eliminate duplication Backend improvements: - Extract 500+ lines of helpers into setup_helpers.py (854 vs 1383 lines) - Add extra="forbid" to personality response models - Add 6 new tests for personality preset endpoints Frontend fixes: - Fix infinite retry loop on personality preset fetch failure - Clear template state when switching to Quick mode (prevents stale data) - Surface model discovery failure instead of silent swallow - Fix double-fetch on provider retry button - Fix 5 floating promise violations (void operator) - Import TemplateVariable from shared types instead of duplicating - Make StatPill label optional, remove redundant label="" props - Read min password length from API instead of hardcoding - Extract MiniOrgChart magic numbers into named constants - Extract WizardModeStep shadow class constant - Fix duplicate React key warning in CompanyStep agent list - Add retry button for personality presets error - Add 5 new store tests for wizard mode and step order Documentation: - Update README, organization.md, operations.md, page-structure.md to reflect new wizard flow, mode selection, and API endpoints Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Backend:
- Extract personality endpoints into SetupPersonalityController (setup.py
now 746 lines, under 800 limit)
- Move validate_agent_index to setup_helpers for cross-controller reuse
- Standardize {agent_index} placeholder in operations.md API table
Frontend:
- Log errors in AccountStep setup status fetch (was empty catch)
- Use SectionCard for CompanyStep agent preview (was hand-built card)
- Filter ProviderAddForm presets to auth_type=api_key only
- Extract TemplateGridItem from TemplateStep .map() block
- Extract DepartmentGroup from MiniOrgChart .map() block
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
603bdcd to
a082741
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/pages/setup/CompanyStep.tsx (1)
63-66:⚠️ Potential issue | 🟠 MajorQuick Setup still reads like a template-application step.
When
selectedTemplateisnullby design, this screen still tells the user to “customize/apply the template.” That makes the skip-template flow look broken even thoughsubmitCompany()should be creating the org without one. Branch the copy and primary CTA onselectedTemplateorwizardMode, and revisit the template-specific validation wording around Line 137 as well.💬 Suggested copy fix
+ const wizardMode = useSetupWizardStore((s) => s.wizardMode) + <div className="space-y-2"> <h2 className="text-lg font-semibold text-foreground">Configure Your Company</h2> <p className="text-sm text-muted-foreground"> - Name your organization and customize the template. + {wizardMode === 'quick' + ? 'Name your organization and finish the required setup.' + : 'Name your organization and customize the template.'} </p> </div>- {companyLoading ? 'Applying Template...' : 'Apply Template'} + {companyLoading + ? (selectedTemplate ? 'Applying Template...' : 'Creating Company...') + : (selectedTemplate ? 'Apply Template' : 'Create Company')}Also applies to: 133-143
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/setup/CompanyStep.tsx` around lines 63 - 66, The page copy and primary CTA must branch when no template is selected: in the CompanyStep component, use the selectedTemplate (or wizardMode) flag to render alternative heading, helper text, and primary button label for the “skip-template” flow (e.g., “Create your organization” / “Name your organization” vs. template-focused copy), and update the template-specific validation message shown by the same component (the text currently tied to template validation near the submitCompany() flow) so it no longer tells users to “customize/apply the template” when selectedTemplate is null; keep submitCompany() logic intact but ensure all UI copy and CTA strings are conditionally chosen based on selectedTemplate or wizardMode (adjust the render of the primary CTA and validation wording accordingly).
♻️ Duplicate comments (3)
web/src/pages/setup/WizardModeStep.tsx (1)
17-20: 🛠️ Refactor suggestion | 🟠 MajorUse the shared shadow token for the selected state.
SELECTED_SHADOWhardcodes the shadow recipe again, so this option card will drift from the rest of the setup UI when the shared token changes. Swap it to the standard shadow token instead.As per coding guidelines, "Use design token rules: colors via Tailwind semantic classes (`text-foreground`, `bg-card`, `text-accent`, `text-success`, `bg-danger`) or CSS variables (`var(--so-accent)`), typography via `font-sans` or `font-mono`, spacing via density-aware tokens (`p-card`, `gap-section-gap`, `gap-grid-gap`) or standard Tailwind, shadows/borders via token variables (`var(--so-shadow-card-hover)`, `border-border`, `border-bright`). Never hardcode hex values, set fontFamily directly, or hardcode pixel values for layout spacing."♻️ Suggested refactor
-const SELECTED_SHADOW = - 'shadow-[0_0_12px_color-mix(' + - 'in_srgb,var(--so-accent)_15%,transparent)]' +const SELECTED_SHADOW = 'shadow-[var(--so-shadow-card-hover)]'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/setup/WizardModeStep.tsx` around lines 17 - 20, Replace the hardcoded SELECTED_SHADOW recipe with the shared shadow token: stop using the SELECTED_SHADOW constant and instead apply the standard design token (e.g. var(--so-shadow-card-hover) or the equivalent shared token used elsewhere) for the selected state in WizardModeStep; update any usage of SELECTED_SHADOW to reference the shared token name so the option card inherits the global shadow token changes.web/src/pages/setup/AccountStep.tsx (1)
24-35:⚠️ Potential issue | 🟠 MajorDon't leave the password policy on the hardcoded fallback.
min_password_lengthis already required onSetupStatusResponse, but this effect still leaves the UI onDEFAULT_MIN_PASSWORD_LENGTHwhenever/setup/statusfails or drifts. If an install is configured above 12, the client will accept a password thatauthSetup()then rejects. Surface a blocking inline error here and readstatus.min_password_lengthdirectly once the fetch succeeds.💡 Suggested fix
const [error, setError] = useState<string | null>(null) const [loading, setLoading] = useState(false) + const [policyError, setPolicyError] = useState<string | null>(null) const [minPasswordLength, setMinPasswordLength] = useState(DEFAULT_MIN_PASSWORD_LENGTH) useEffect(() => { - getSetupStatus() + void getSetupStatus() .then((status) => { - setMinPasswordLength( - status.min_password_length ?? DEFAULT_MIN_PASSWORD_LENGTH, - ) + setMinPasswordLength(status.min_password_length) + setPolicyError(null) }) .catch((err) => { console.error('AccountStep: failed to fetch setup status:', err) + setPolicyError( + 'Unable to load the password policy. Please refresh and try again.', + ) }) }, [])+ {policyError && ( + <div + role="alert" + className="rounded-md border border-danger/30 bg-danger/5 px-3 py-2 text-sm text-danger" + > + {policyError} + </div> + )} - <Button onClick={handleSubmit} disabled={loading} className="w-full"> + <Button + onClick={handleSubmit} + disabled={loading || policyError !== null} + className="w-full" + >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/setup/AccountStep.tsx` around lines 24 - 35, The effect that calls getSetupStatus currently falls back to DEFAULT_MIN_PASSWORD_LENGTH on failure which can allow the UI to accept passwords shorter than the server policy; instead, in the useEffect that calls getSetupStatus, stop using the hardcoded fallback (DEFAULT_MIN_PASSWORD_LENGTH), set the min via setMinPasswordLength strictly from status.min_password_length when the fetch succeeds, and on error set a new local error state (e.g., setupStatusError) that the AccountStep render checks to show a blocking inline error and disables the submit path that calls authSetup() until getSetupStatus() succeeds; update the code paths referencing DEFAULT_MIN_PASSWORD_LENGTH, useEffect, getSetupStatus, setMinPasswordLength and authSetup to ensure the UI only validates against the server-provided min_password_length.web/src/pages/setup/ProviderProbeResults.tsx (1)
71-86:⚠️ Potential issue | 🟠 MajorTrack pending adds per preset, not with a single string.
addingPresetonly models one in-flight request. If a user clicks provider B while A is still adding, A'sfinallyresets the flag tonull, so B loses its loading state and can be submitted again while its first request is still pending.🔧 Suggested fix using a Set
- const [addingPreset, setAddingPreset] = useState<string | null>(null) + const [addingPresets, setAddingPresets] = useState<ReadonlySet<string>>( + () => new Set(), + ) const handleAdd = async (presetName: string) => { - setAddingPreset(presetName) + setAddingPresets((current) => { + const next = new Set(current) + next.add(presetName) + return next + }) try { await onAddPreset(presetName) } catch { // Error already set in store (providersError) and rendered by ProvidersStep } finally { - setAddingPreset(null) + setAddingPresets((current) => { + const next = new Set(current) + next.delete(presetName) + return next + }) } }Then update line 116:
- adding={addingPreset === preset.name} + adding={addingPresets.has(preset.name)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/setup/ProviderProbeResults.tsx` around lines 71 - 86, The current addingPreset state (addingPreset / setAddingPreset) only models a single in-flight add; change it to track multiple pending presets (e.g., a Set<string>) so each preset maintains its own loading state: replace the string|null state with a Set (or similar collection), update handleAdd to add the presetName to the Set before awaiting onAddPreset and remove it in finally (use the functional updater form to avoid races), and update any UI checks that compare addingPreset === presetName to instead check set.has(presetName) (also update localPresets/presets usage as needed). Ensure references to addingPreset, setAddingPreset, and handleAdd are updated consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/api/controllers/setup_helpers.py`:
- Around line 326-353: The code in read_name_locales() currently slices
entry.value and only catches JSONDecodeError, allowing TypeError from non-string
entry.value to propagate; centralize parsing by sending entry.value into
parse_locale_json() (or update parse_locale_json()) to catch both
JSONDecodeError and TypeError and return None on failure, log using
SETUP_NAME_LOCALES_CORRUPTED with safe raw preview only after confirming
entry.value is a str, and ensure read_name_locales() validates that the parsed
result is a list of strings (no truthy non-string elements) before calling
resolve_locales(); update handling in read_name_locales(), parse_locale_json(),
and any callers of resolve_locales() to use this validated list and to return
False/None on invalid input.
- Around line 558-579: persist_company_settings performs three independent
awaits on SettingsService.set which can leave a partially written company if one
call fails; change this to use an atomic batch/transaction API (e.g., add and
call SettingsService.set_many or a transactional method like
SettingsService.transaction or SettingsService.run_in_transaction to set
company_name, description, and departments in one commit) or wrap the three sets
in explicit transaction handling with rollback on error; ensure you log failures
at WARNING/ERROR with context (including company_name and which fields failed)
before re-raising the exception so callers don’t observe a half-written company
via check_has_company().
In `@src/synthorg/api/controllers/setup_models.py`:
- Around line 12-44: The _normalize_and_validate_preset helper currently raises
ValueError for unknown presets without logging; update it to log a warning with
context before raising: import or use the module logger (consistent with
get_personality_preset) and call logger.warning including the invalid raw value
and available keys (PERSONALITY_PRESETS) immediately before the ValueError is
raised in the branch where key not in PERSONALITY_PRESETS so the warning mirrors
the raised message and provides context for debugging.
In `@web/src/__tests__/pages/setup/WizardProgress.test.tsx`:
- Around line 6-17: Add a fast-check property test in WizardProgress.test.tsx
that uses fc.assert and fc.property to verify an invariant across randomized
inputs: for example, generate permutations of the stepOrder array (or arbitrary
subsets of WizardStep using the WizardStep union) and arbitrary completion maps
based on defaultStepsCompleted, then render the WizardProgress (or call its
label/completion helpers) and assert that every step in the generated order
either renders a label from the known stepOrder set and its completed state
equals the value in the generated completion map; reference WizardStep,
stepOrder, defaultStepsCompleted and the WizardProgress rendering helpers in the
test so the property ties generated inputs to the component invariants. Ensure
the test imports fast-check (fc.assert, fc.property) and uses only valid
WizardStep values when building permutations or maps.
In `@web/src/pages/setup/MiniOrgChart.tsx`:
- Around line 90-111: Extract the magic numbers used for truncating department
names in DepartmentGroup into named constants (e.g., MAX_DISPLAY_LENGTH and
TRUNCATE_AT) and replace the inline literals in the conditional and slice
expressions that reference pos.dept.name; update the JSX expression that
currently uses "pos.dept.name.length > 14 ? pos.dept.name.slice(0, 12) + '..' :
pos.dept.name" to use those constants so the truncation thresholds are clear and
maintainable.
- Around line 50-57: The connector line uses a hardcoded vertical offset (deptY
+ 18) which should be derived from the node height; update MiniOrgChart to
accept a nodeHeight prop (or use the existing nodeHeight value) and replace the
literal 18 with nodeHeight / 2 (or the appropriate expression based on how node
Y is computed), then use that derived value when computing y1 for the <line>
(referencing deptY, agentY, radius) so the connector moves correctly when
nodeHeight changes.
In `@web/src/stores/setup-wizard.ts`:
- Around line 610-628: Extract the duplicate Promise.allSettled + aggregation
logic used in probeAllPresets and reprobePresets into a helper (e.g.,
runProbeAll) that accepts presets and a label; have runProbeAll call probePreset
for each preset, collect results into a Record<string, ProbePresetResponse>, and
log errors using the provided label; then update probeAllPresets to call
runProbeAll(presets, 'probe') and reprobePresets to clear probeResults, set
probing true, call runProbeAll(presets, 'reprobe'), and set probeResults and
probing false — reference functions/fields: reprobePresets, probeAllPresets,
runProbeAll (new), probePreset, probeResults, probing.
---
Outside diff comments:
In `@web/src/pages/setup/CompanyStep.tsx`:
- Around line 63-66: The page copy and primary CTA must branch when no template
is selected: in the CompanyStep component, use the selectedTemplate (or
wizardMode) flag to render alternative heading, helper text, and primary button
label for the “skip-template” flow (e.g., “Create your organization” / “Name
your organization” vs. template-focused copy), and update the template-specific
validation message shown by the same component (the text currently tied to
template validation near the submitCompany() flow) so it no longer tells users
to “customize/apply the template” when selectedTemplate is null; keep
submitCompany() logic intact but ensure all UI copy and CTA strings are
conditionally chosen based on selectedTemplate or wizardMode (adjust the render
of the primary CTA and validation wording accordingly).
---
Duplicate comments:
In `@web/src/pages/setup/AccountStep.tsx`:
- Around line 24-35: The effect that calls getSetupStatus currently falls back
to DEFAULT_MIN_PASSWORD_LENGTH on failure which can allow the UI to accept
passwords shorter than the server policy; instead, in the useEffect that calls
getSetupStatus, stop using the hardcoded fallback (DEFAULT_MIN_PASSWORD_LENGTH),
set the min via setMinPasswordLength strictly from status.min_password_length
when the fetch succeeds, and on error set a new local error state (e.g.,
setupStatusError) that the AccountStep render checks to show a blocking inline
error and disables the submit path that calls authSetup() until getSetupStatus()
succeeds; update the code paths referencing DEFAULT_MIN_PASSWORD_LENGTH,
useEffect, getSetupStatus, setMinPasswordLength and authSetup to ensure the UI
only validates against the server-provided min_password_length.
In `@web/src/pages/setup/ProviderProbeResults.tsx`:
- Around line 71-86: The current addingPreset state (addingPreset /
setAddingPreset) only models a single in-flight add; change it to track multiple
pending presets (e.g., a Set<string>) so each preset maintains its own loading
state: replace the string|null state with a Set (or similar collection), update
handleAdd to add the presetName to the Set before awaiting onAddPreset and
remove it in finally (use the functional updater form to avoid races), and
update any UI checks that compare addingPreset === presetName to instead check
set.has(presetName) (also update localPresets/presets usage as needed). Ensure
references to addingPreset, setAddingPreset, and handleAdd are updated
consistently.
In `@web/src/pages/setup/WizardModeStep.tsx`:
- Around line 17-20: Replace the hardcoded SELECTED_SHADOW recipe with the
shared shadow token: stop using the SELECTED_SHADOW constant and instead apply
the standard design token (e.g. var(--so-shadow-card-hover) or the equivalent
shared token used elsewhere) for the selected state in WizardModeStep; update
any usage of SELECTED_SHADOW to reference the shared token name so the option
card inherits the global shadow token changes.
🪄 Autofix (Beta)
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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 31523dfa-78b2-4f90-86b2-0c5cca88a2a5
📒 Files selected for processing (41)
README.mddocs/design/operations.mddocs/design/organization.mddocs/design/page-structure.mddocs/user_guide.mdsrc/synthorg/api/controllers/__init__.pysrc/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/setup_helpers.pysrc/synthorg/api/controllers/setup_models.pysrc/synthorg/api/controllers/setup_personality.pysrc/synthorg/observability/events/setup.pytests/unit/api/controllers/test_setup.pyweb/src/__tests__/pages/setup/WizardProgress.test.tsxweb/src/__tests__/stores/setup-wizard.test.tsweb/src/__tests__/utils/cost-estimator.property.test.tsweb/src/__tests__/utils/cost-estimator.test.tsweb/src/api/endpoints/setup.tsweb/src/api/types.tsweb/src/components/ui/stat-pill.tsxweb/src/pages/setup/AccountStep.tsxweb/src/pages/setup/AgentsStep.tsxweb/src/pages/setup/CompanyStep.tsxweb/src/pages/setup/CompleteStep.tsxweb/src/pages/setup/CostEstimatePanel.tsxweb/src/pages/setup/MiniOrgChart.tsxweb/src/pages/setup/ProviderAddForm.tsxweb/src/pages/setup/ProviderProbeResults.tsxweb/src/pages/setup/ProvidersStep.tsxweb/src/pages/setup/SetupAgentCard.tsxweb/src/pages/setup/SetupSummary.tsxweb/src/pages/setup/TemplateCard.tsxweb/src/pages/setup/TemplateCategoryGroup.tsxweb/src/pages/setup/TemplateCompareDrawer.tsxweb/src/pages/setup/TemplateCostBadge.tsxweb/src/pages/setup/TemplateStep.tsxweb/src/pages/setup/TemplateVariables.tsxweb/src/pages/setup/WizardModeStep.tsxweb/src/pages/setup/WizardProgress.tsxweb/src/pages/setup/WizardShell.tsxweb/src/stores/setup-wizard.tsweb/src/utils/cost-estimator.ts
💤 Files with no reviewable changes (7)
- web/src/pages/setup/TemplateCompareDrawer.tsx
- web/src/tests/utils/cost-estimator.property.test.ts
- web/src/pages/setup/CostEstimatePanel.tsx
- web/src/pages/setup/TemplateCostBadge.tsx
- web/src/pages/setup/TemplateCategoryGroup.tsx
- web/src/tests/utils/cost-estimator.test.ts
- web/src/utils/cost-estimator.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Dashboard Test
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Backend
- GitHub Check: Build Web
- GitHub Check: Build Sandbox
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always reuse existing components from
web/src/components/ui/before creating new ones. Do not recreate status dots inline, build card-with-header layouts from scratch, create metric displays manually, render initials circles manually, create complex (>8 line) JSX inside.map()blocks, or usergba()with hardcoded values instead of design token variables.
Files:
web/src/pages/setup/WizardProgress.tsxweb/src/pages/setup/CompleteStep.tsxweb/src/api/endpoints/setup.tsweb/src/components/ui/stat-pill.tsxweb/src/__tests__/pages/setup/WizardProgress.test.tsxweb/src/pages/setup/TemplateStep.tsxweb/src/pages/setup/AccountStep.tsxweb/src/pages/setup/ProvidersStep.tsxweb/src/api/types.tsweb/src/pages/setup/WizardShell.tsxweb/src/pages/setup/TemplateCard.tsxweb/src/pages/setup/CompanyStep.tsxweb/src/pages/setup/TemplateVariables.tsxweb/src/pages/setup/AgentsStep.tsxweb/src/pages/setup/SetupSummary.tsxweb/src/pages/setup/MiniOrgChart.tsxweb/src/pages/setup/SetupAgentCard.tsxweb/src/pages/setup/ProviderAddForm.tsxweb/src/pages/setup/ProviderProbeResults.tsxweb/src/pages/setup/WizardModeStep.tsxweb/src/__tests__/stores/setup-wizard.test.tsweb/src/stores/setup-wizard.ts
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use design token rules: colors via Tailwind semantic classes (
text-foreground,bg-card,text-accent,text-success,bg-danger) or CSS variables (var(--so-accent)), typography viafont-sansorfont-mono, spacing via density-aware tokens (p-card,gap-section-gap,gap-grid-gap) or standard Tailwind, shadows/borders via token variables (var(--so-shadow-card-hover),border-border,border-bright). Never hardcode hex values, set fontFamily directly, or hardcode pixel values for layout spacing.
Files:
web/src/pages/setup/WizardProgress.tsxweb/src/pages/setup/CompleteStep.tsxweb/src/api/endpoints/setup.tsweb/src/components/ui/stat-pill.tsxweb/src/__tests__/pages/setup/WizardProgress.test.tsxweb/src/pages/setup/TemplateStep.tsxweb/src/pages/setup/AccountStep.tsxweb/src/pages/setup/ProvidersStep.tsxweb/src/api/types.tsweb/src/pages/setup/WizardShell.tsxweb/src/pages/setup/TemplateCard.tsxweb/src/pages/setup/CompanyStep.tsxweb/src/pages/setup/TemplateVariables.tsxweb/src/pages/setup/AgentsStep.tsxweb/src/pages/setup/SetupSummary.tsxweb/src/pages/setup/MiniOrgChart.tsxweb/src/pages/setup/SetupAgentCard.tsxweb/src/pages/setup/ProviderAddForm.tsxweb/src/pages/setup/ProviderProbeResults.tsxweb/src/pages/setup/WizardModeStep.tsxweb/src/__tests__/stores/setup-wizard.test.tsweb/src/stores/setup-wizard.ts
web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use property-based testing with fast-check (
fc.assert+fc.property). Run tests withnpm --prefix web run testfor Vitest with--detect-async-leaksflag.
Files:
web/src/pages/setup/WizardProgress.tsxweb/src/pages/setup/CompleteStep.tsxweb/src/api/endpoints/setup.tsweb/src/components/ui/stat-pill.tsxweb/src/__tests__/pages/setup/WizardProgress.test.tsxweb/src/pages/setup/TemplateStep.tsxweb/src/pages/setup/AccountStep.tsxweb/src/pages/setup/ProvidersStep.tsxweb/src/api/types.tsweb/src/pages/setup/WizardShell.tsxweb/src/pages/setup/TemplateCard.tsxweb/src/pages/setup/CompanyStep.tsxweb/src/pages/setup/TemplateVariables.tsxweb/src/pages/setup/AgentsStep.tsxweb/src/pages/setup/SetupSummary.tsxweb/src/pages/setup/MiniOrgChart.tsxweb/src/pages/setup/SetupAgentCard.tsxweb/src/pages/setup/ProviderAddForm.tsxweb/src/pages/setup/ProviderProbeResults.tsxweb/src/pages/setup/WizardModeStep.tsxweb/src/__tests__/stores/setup-wizard.test.tsweb/src/stores/setup-wizard.ts
**/*.{md,mdx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always read the relevant
docs/design/page before implementing any feature or planning any issue. The design spec is the starting point for architecture, data models, and behavior. If implementation deviates from spec, alert the user and explain why -- user decides whether to proceed or update the spec. Never silently diverge. When approved deviations occur, update the relevantdocs/design/page to reflect the new reality.
Files:
README.mddocs/design/organization.mddocs/user_guide.mddocs/design/page-structure.mddocs/design/operations.md
docs/design/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Design spec pages are mandatory reading before implementing features. Design specs pages are the starting point for architecture, data models, and behavior. When implementation deviates from spec, alert the user with reasoning. Update the relevant design page when approved deviations occur.
Files:
docs/design/organization.mddocs/design/page-structure.mddocs/design/operations.md
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations-- Python 3.14+ has PEP 649 native lazy annotations. Useexcept A, B:(no parentheses) per PEP 758 except syntax as enforced by ruff.
All public functions and classes require type hints and Google-style docstrings (enforced by ruff D rules). Use mypy strict mode.
Functions must be less than 50 lines, files less than 800 lines. Handle errors explicitly, never silently swallow. Validate at system boundaries (user input, external APIs, config files).
Files:
src/synthorg/observability/events/setup.pysrc/synthorg/api/controllers/__init__.pysrc/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/setup_personality.pytests/unit/api/controllers/test_setup.pysrc/synthorg/api/controllers/setup_models.pysrc/synthorg/api/controllers/setup_helpers.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic must have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__). Use event name constants fromsynthorg.observability.events.<domain>modules (e.g.,API_REQUEST_STARTED,TOOL_INVOKE_START). Always use structured kwargs:logger.info(EVENT, key=value). Log all error paths at WARNING or ERROR with context before raising, all state transitions at INFO, and DEBUG for object creation and key function flow. Pure data models, enums, and re-exports do not need logging.
Use immutability patterns: create new objects instead of mutating existing ones. For non-Pydantic internal collections (registries,BaseTool), usecopy.deepcopy()at construction plusMappingProxyTypewrapping for read-only enforcement. Fordict/listfields in frozen Pydantic models, usecopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization).
Use Pydantic v2 models (BaseModel,model_validator,computed_field,ConfigDict). Use@computed_fieldfor derived values instead of storing redundant fields (e.g.,TokenUsage.total_tokens). UseNotBlankStrfromcore.typesfor all identifier/name fields (including optionalNotBlankStr | Noneand tuple variants) instead of manual whitespace validators.
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations (e.g., multiple tool invocations, parallel agent calls) in new code instead of barecreate_task. Favor structured concurrency.
Separate frozen Pydantic models for config/identity from mutable-via-copy models for runtime state that evolves (e.g., agent execution state, task progress). Never mix static config fields with mutable runtime fields in one model.
Never useimport loggingorlogging.getLogger()in application code. Exception:observability/setup.pyandobservability/sinks.pymay use stdlibloggingandprint(..., file=sys.stderr)for bootst...
Files:
src/synthorg/observability/events/setup.pysrc/synthorg/api/controllers/__init__.pysrc/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/setup_personality.pysrc/synthorg/api/controllers/setup_models.pysrc/synthorg/api/controllers/setup_helpers.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Line length: 88 characters (ruff enforces).
Files:
src/synthorg/observability/events/setup.pysrc/synthorg/api/controllers/__init__.pysrc/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/setup_personality.pysrc/synthorg/api/controllers/setup_models.pysrc/synthorg/api/controllers/setup_helpers.py
src/synthorg/api/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use RFC 9457 errors in the REST API. Implement role-based access control (RBAC) guards in the API layer.
Files:
src/synthorg/api/controllers/__init__.pysrc/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/setup_personality.pysrc/synthorg/api/controllers/setup_models.pysrc/synthorg/api/controllers/setup_helpers.py
web/src/components/ui/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
New shared components in
web/src/components/ui/must have a corresponding.stories.tsxfile with all states (default, hover, loading, error, empty), exported props as a TypeScript interface, design tokens exclusively (no hardcoded colors, fonts, or spacing), and usecn()from@/lib/utilsfor conditional class merging.
Files:
web/src/components/ui/stat-pill.tsx
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use pytest markers:@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slow. Maintain 80% minimum coverage. Useasyncio_mode = 'auto'(no manual@pytest.mark.asyncio). Set 30-second global timeout per test. Run tests with-n autofor parallelism via pytest-xdist; never run sequentially. Use Hypothesis for property-based testing with@givenand@settings,HYPOTHESIS_PROFILEenv var (ci=50 examples, dev=1000 examples). Never skip flaky tests -- fix them fundamentally by mockingtime.monotonic(),asyncio.sleep(), or usingasyncio.Event().wait()for blocking operations.
Use vendor-agnostic names in tests:test-provider,test-small-001,large/medium/smallaliases instead of real vendor names (Anthropic, OpenAI, Claude, GPT, etc.). Vendor names appear only in operations design page,.claude/files, third-party imports, and provider presets.
Files:
tests/unit/api/controllers/test_setup.py
🧠 Learnings (29)
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : Package structure: src/synthorg/ organized as: api/ (REST+WebSocket, Litestar), auth/ (auth subpackage), backup/ (scheduled/manual backups), budget/ (cost tracking, CFO), cli/ (superseded by Go CLI), communication/ (message bus, meetings), config/ (YAML loading), core/ (domain models, resilience config), engine/ (orchestration, task state, coordination, approval gates, stagnation detection, context budget, compaction), hr/ (hiring, performance, promotion), memory/ (pluggable backend, Mem0, retrieval, consolidation), persistence/ (operational data, SQLite, settings), observability/ (logging, correlation, sinks), providers/ (LLM abstraction, LiteLLM, auth types, presets, runtime CRUD), settings/ (runtime-editable, typed definitions, encryption, config bridge), security/ (SecOps, rule engine, output scanning, progressive trust, autonomy levels), templates/ (company templates, personalities), tools/ (registry, built-in tools, git, sandbox, code_runner, MCP...
Applied to files:
README.mdsrc/synthorg/api/controllers/__init__.pysrc/synthorg/api/controllers/setup.pydocs/design/operations.md
📚 Learning: 2026-03-26T15:18:16.848Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T15:18:16.848Z
Learning: Applies to src/synthorg/api/**/*.py : Litestar API must include setup wizard, auth/, auto-wiring, and lifecycle management
Applied to files:
README.mdsrc/synthorg/api/controllers/__init__.pydocs/user_guide.mdsrc/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/setup_personality.pysrc/synthorg/api/controllers/setup_helpers.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to docker/Dockerfile.sandbox : Docker sandbox: `synthorg-sandbox` — Python 3.14 + Node.js + git, non-root (UID 10001), agent code execution sandbox
Applied to files:
README.md
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to docs/design/**/*.md : Design specification pages in `docs/design/` must be consulted before implementing features (7 pages: index, agents, organization, communication, engine, memory, operations)
Applied to files:
docs/design/organization.mddocs/design/operations.md
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to src/synthorg/templates/**/*.py : Templates: pre-built company templates, personality presets, and builder.
Applied to files:
docs/design/organization.mdsrc/synthorg/api/controllers/setup_models.pyweb/src/stores/setup-wizard.ts
📚 Learning: 2026-03-28T18:10:43.542Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T18:10:43.542Z
Learning: Applies to **/*.{ts,tsx} : Always reuse existing components from `web/src/components/ui/` before creating new ones. Do not recreate status dots inline, build card-with-header layouts from scratch, create metric displays manually, render initials circles manually, create complex (>8 line) JSX inside `.map()` blocks, or use `rgba()` with hardcoded values instead of design token variables.
Applied to files:
web/src/pages/setup/CompleteStep.tsxweb/src/pages/setup/TemplateStep.tsxweb/src/pages/setup/TemplateCard.tsxweb/src/pages/setup/CompanyStep.tsxweb/src/pages/setup/MiniOrgChart.tsxweb/src/pages/setup/WizardModeStep.tsx
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/api/**/*.py : API package (api/): Litestar REST + WebSocket with controllers, guards, channels, JWT + API key + WS ticket auth, approval gate integration, coordination endpoint, collaboration endpoint, settings endpoint, provider management endpoint (CRUD + test + presets), backup endpoint, RFC 9457 structured errors, AppState hot-reload slots, service auto-wiring (Phase 1 at construction, Phase 2 on startup), lifecycle helpers
Applied to files:
src/synthorg/api/controllers/__init__.pysrc/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/setup_personality.pydocs/design/operations.mdsrc/synthorg/api/controllers/setup_helpers.py
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to src/synthorg/providers/**/*.py : Providers: LLM provider abstraction (LiteLLM adapter), auth types (api_key/oauth/custom_header/none), presets (PROVIDER_PRESETS), runtime CRUD (ProviderManagementService with asyncio.Lock serialization), hot-reload via AppState swap.
Applied to files:
src/synthorg/api/controllers/__init__.pysrc/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/setup_helpers.py
📚 Learning: 2026-03-16T20:14:00.937Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T20:14:00.937Z
Learning: Applies to **/*.py : Validate: at system boundaries (user input, external APIs, config files).
Applied to files:
src/synthorg/api/controllers/setup.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Handle errors explicitly, never silently swallow. Validate at system boundaries (user input, external APIs, config files).
Applied to files:
src/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/setup_models.py
📚 Learning: 2026-03-19T07:13:44.964Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:13:44.964Z
Learning: Applies to **/*.py : Validate at system boundaries (user input, external APIs, config files)
Applied to files:
src/synthorg/api/controllers/setup.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/api/**/*.py : REST API: Litestar framework, controllers with guards, channels for WebSocket, JWT + API key + WS ticket auth, approval gate integration, coordination endpoint, collaboration endpoint, settings endpoint. RFC 9457 structured errors (ErrorCategory, ErrorCode, ErrorDetail, ProblemDetail, CATEGORY_TITLES, category_title, category_type_uri, content negotiation).
Applied to files:
src/synthorg/api/controllers/setup.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/api/**/*.py : Use Litestar for REST + WebSocket API. Controllers, guards, channels, JWT + API key + WS ticket auth, RFC 9457 structured errors.
Applied to files:
src/synthorg/api/controllers/setup.py
📚 Learning: 2026-03-20T11:18:48.128Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T11:18:48.128Z
Learning: Applies to src/synthorg/api/**/*.py : Use Litestar for REST API and WebSocket API with JWT + API key + WS ticket authentication, RFC 9457 structured errors, and content negotiation.
Applied to files:
src/synthorg/api/controllers/setup.py
📚 Learning: 2026-03-15T21:20:09.993Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:20:09.993Z
Learning: Applies to web/src/components/** : Vue components organized by feature (agents/, approvals/, budget/, common/, dashboard/, layout/, messages/, org-chart/, tasks/).
Applied to files:
web/src/pages/setup/SetupSummary.tsx
📚 Learning: 2026-03-28T18:10:43.543Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T18:10:43.543Z
Learning: Applies to web/src/**/*.{ts,tsx} : Use design token rules: colors via Tailwind semantic classes (`text-foreground`, `bg-card`, `text-accent`, `text-success`, `bg-danger`) or CSS variables (`var(--so-accent)`), typography via `font-sans` or `font-mono`, spacing via density-aware tokens (`p-card`, `gap-section-gap`, `gap-grid-gap`) or standard Tailwind, shadows/borders via token variables (`var(--so-shadow-card-hover)`, `border-border`, `border-bright`). Never hardcode hex values, set fontFamily directly, or hardcode pixel values for layout spacing.
Applied to files:
web/src/pages/setup/MiniOrgChart.tsxweb/src/pages/setup/WizardModeStep.tsx
📚 Learning: 2026-03-28T18:10:43.543Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T18:10:43.543Z
Learning: Applies to web/src/components/ui/*.{ts,tsx} : New shared components in `web/src/components/ui/` must have a corresponding `.stories.tsx` file with all states (default, hover, loading, error, empty), exported props as a TypeScript interface, design tokens exclusively (no hardcoded colors, fonts, or spacing), and use `cn()` from `@/lib/utils` for conditional class merging.
Applied to files:
web/src/pages/setup/MiniOrgChart.tsxweb/src/pages/setup/WizardModeStep.tsx
📚 Learning: 2026-03-28T18:10:43.543Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T18:10:43.543Z
Learning: Applies to tests/**/*.py : Use vendor-agnostic names in tests: `test-provider`, `test-small-001`, `large`/`medium`/`small` aliases instead of real vendor names (Anthropic, OpenAI, Claude, GPT, etc.). Vendor names appear only in operations design page, `.claude/` files, third-party imports, and provider presets.
Applied to files:
tests/unit/api/controllers/test_setup.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to docs/design/*.md : Design spec pages: 7 pages in `docs/design/` — index, agents, organization, communication, engine, memory, operations
Applied to files:
docs/design/operations.md
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Documentation source in `docs/` (Markdown, built with Zensical). Design spec in `docs/design/` (7 pages: index, agents, organization, communication, engine, memory, operations). Architecture in `docs/architecture/` (overview, tech-stack, decision log). Roadmap in `docs/roadmap/`. Security in `docs/security.md`. Licensing in `docs/licensing.md`. Reference in `docs/reference/`. REST API reference in `docs/rest-api.md`. Library reference in `docs/api/` (auto-generated from docstrings). Custom templates in `docs/overrides/`. Config in `mkdocs.yml`.
Applied to files:
docs/design/operations.md
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (via `model_copy(update=...)`) for runtime state that evolves
Applied to files:
src/synthorg/api/controllers/setup_models.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; separate mutable-via-copy models (using `model_copy(update=...)`) for runtime state
Applied to files:
src/synthorg/api/controllers/setup_models.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`. For derived values use `computed_field` instead of storing + validating redundant fields. Use `NotBlankStr` (from `core.types`) for all identifier/name fields — including optional (`NotBlankStr | None`) and tuple (`tuple[NotBlankStr, ...]`) variants — instead of manual whitespace validators.
Applied to files:
src/synthorg/api/controllers/setup_models.py
📚 Learning: 2026-03-28T18:10:43.543Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T18:10:43.543Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 models (`BaseModel`, `model_validator`, `computed_field`, `ConfigDict`). Use `computed_field` for derived values instead of storing redundant fields (e.g., `TokenUsage.total_tokens`). Use `NotBlankStr` from `core.types` for all identifier/name fields (including optional `NotBlankStr | None` and tuple variants) instead of manual whitespace validators.
Applied to files:
src/synthorg/api/controllers/setup_models.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Applied to files:
src/synthorg/api/controllers/setup_models.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions.
Applied to files:
src/synthorg/api/controllers/setup_models.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, and key function entry/exit
Applied to files:
src/synthorg/api/controllers/setup_models.py
📚 Learning: 2026-03-21T11:08:01.542Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-21T11:08:01.542Z
Learning: Applies to **/*.{py,md} : Line length: 88 characters (ruff).
Applied to files:
web/src/pages/setup/WizardModeStep.tsxweb/src/stores/setup-wizard.ts
📚 Learning: 2026-03-28T18:10:43.543Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T18:10:43.543Z
Learning: Applies to src/**/*.py : Line length: 88 characters (ruff enforces).
Applied to files:
web/src/pages/setup/WizardModeStep.tsxweb/src/stores/setup-wizard.ts
🔇 Additional comments (41)
web/src/pages/setup/MiniOrgChart.tsx (4)
20-26: Hardcoded layout constants remain unfixed.The pixel values for avatar radii, node widths, and spacing gap were previously flagged for replacement with shared design tokens or centralized layout constants. This concern remains unaddressed.
168-177: Height calculation still usesmaxAgentsInDeptinstead of total team size.This was previously flagged:
svgHeightis driven by the largest department's agent count rather than the total number of agents. This causes a small team concentrated in one department to render taller than a larger team spread across departments, which doesn't match the intended "scale by team size" behavior.
1-13: LGTM!Imports are minimal and the interfaces are well-typed with appropriate readonly modifier on the agents array.
191-227: Good use of semantic design tokens and component extraction.The container uses appropriate semantic classes (
border-border,bg-card,p-4) and the rendering logic is cleanly structured withDepartmentGrouphandling the complexity. The SVG accessibility attributes (role="img",aria-label) are a nice touch.web/src/pages/setup/WizardProgress.tsx (1)
10-19: Step-label mapping is correctly updated for the new wizard flow.Adding
modeand keeping the labels aligned with the reordered step sequence keeps progress rendering consistent across Guided/Quick paths.src/synthorg/observability/events/setup.py (1)
100-104: New setup observability events are consistent and well-scoped.The added constants follow the module’s existing naming and typing pattern and cleanly support the personality endpoints.
src/synthorg/api/controllers/__init__.py (1)
25-27: Controller wiring is complete (import + registration + export).
SetupPersonalityControlleris correctly integrated into the controllers package surface and router feed.Also applies to: 53-53, 81-81
web/src/pages/setup/CompleteStep.tsx (1)
1-1: This cleanup is coherent with the new completion/error handling path.Import simplification and the catch-path comment now match store-managed
completionErrorrendering behavior.Also applies to: 29-29
web/src/components/ui/stat-pill.tsx (1)
3-5: Optional-label rendering is a solid fix for the empty-label UI glitch.Conditionally omitting the label span removes the leading-space artifact while keeping pill value behavior intact.
Also applies to: 17-21
docs/design/organization.md (1)
336-351: Design-spec update is aligned with the new setup wizard behavior.The Guided/Quick branching, providers-before-agents ordering, and quick-mode scope are clearly documented.
README.md (1)
110-110: README setup flow text is updated correctly for Guided vs Quick onboarding.The sequence and purpose of provider-before-agent configuration are communicated clearly for first-run users.
web/src/pages/setup/WizardModeStep.tsx (1)
23-31:⚠️ Potential issue | 🟡 MinorExpose the selected mode to assistive tech.
These buttons behave like a two-state selector, but the current state is only visual. Add
aria-pressed={selected}at minimum, or model the pair as a radio group, so screen-reader users can tell which mode is active before activating it.🦾 Minimal accessibility fix
<button type="button" onClick={onClick} + aria-pressed={selected} className={cn(> Likely an incorrect or invalid review comment.web/src/pages/setup/ProvidersStep.tsx (3)
56-63: Well-structured single-mount fetch pattern.The
fetchedRefguard ensures providers and presets are fetched exactly once on mount, avoiding duplicate requests on re-renders. This correctly addresses the previous double-fetch concern.
93-96: Manual reprobe correctly maintains guard state.Setting
probeAttemptedRef.current = truebefore callingreprobePresets()prevents the auto-probe effect (lines 67-72) from triggering redundantly after a user-initiated reprobe.
129-137: Retry button fix correctly applied.The retry button now calls
fetchProviders()directly without resettingfetchedRef.current, which prevents the previous double-fetch issue where resetting the ref would trigger both the button's fetch and the useEffect.web/src/pages/setup/AgentsStep.tsx (2)
36-50: Infinite retry loop correctly prevented.The guard condition
!personalityPresetsErrorstops the effect from re-fetching after a failure. Combined with the store's implementation that setspersonalityPresetsError: nullbefore each attempt (seeweb/src/stores/setup-wizard.ts:499), clicking the retry button clears the error and allows the useEffect or button handler to retry appropriately.
139-152: Retry button for presets provides consistent UX.Good addition matching the agents error pattern. The warning styling (vs. danger for agents) correctly signals that presets are optional while still offering recovery.
web/src/pages/setup/SetupSummary.tsx (1)
74-74: Clean replacement of cost estimate metric.Replacing "Est. Monthly Cost" with "Providers" count aligns with the PR's removal of cost-estimation UI while maintaining a useful summary metric.
web/src/pages/setup/TemplateVariables.tsx (1)
12-16: Word-boundary regex prevents unintended replacements.Using
\bUSD\bensures only standalone "USD" is replaced, not substrings like "USDA" or "nonUSD". The early return whencurrencyis falsy is a good guard.src/synthorg/api/controllers/setup.py (2)
24-68: Helper extraction keeps controller focused and under line limit.Moving utilities to
setup_helpers.pyaddresses the file-size concern from past reviews. The underscore-prefixed imports clearly signal internal usage.
726-731: Agent check correctly relaxed for Quick Setup.The check now logs
SETUP_NO_AGENTSat INFO level with context (note="allowed_for_quick_setup") instead of raisingApiValidationError. This aligns with the PR's Quick Setup flow where agents are optional.docs/design/page-structure.md (2)
146-148: Documentation accurately reflects the new wizard flow and API surface.The mode selection gate, both flow paths (Guided/Quick), and new personality preset endpoints are correctly documented. This keeps the design spec in sync with implementation.
222-222: Route table correctly shows conditional account step for both flows.The
account (conditional)notation in both Guided and Quick paths addresses the previous review concern about fresh-install scenarios requiring account creation before mode selection.docs/user_guide.md (1)
69-84: User guide comprehensively covers the new wizard flow.The documentation clearly explains the Guided/Quick mode choice, updated step order (Providers before Agents), new features (auto-detection, personality presets), and relaxed completion requirements. This provides users with accurate guidance for both setup paths.
web/src/__tests__/stores/setup-wizard.test.ts (2)
127-164: Comprehensive test coverage for wizard mode switching.Tests cover all key behaviors: step order changes for quick/guided modes, template state cleanup on mode switch, step validation against current flow, and account step inclusion when needed. Good defensive testing for the mode-gated navigation logic.
100-110: Navigation tests correctly updated for new step order.The test now validates the mode→template→company→providers sequence, ensuring
canNavigateTo('providers')requires all prior steps (mode, template, company) to be complete.web/src/pages/setup/ProviderAddForm.tsx (2)
21-24: LGTM! Auth type filtering now correctly restricts to API key presets.The filter now correctly limits selectable presets to
auth_type === 'api_key', ensuring the form only shows presets that can be configured with the API key field it renders.
26-29: LGTM! Provider name uniqueness validation is well implemented.The
nameConflictcheck properly validates against existing provider names, guardshandleAdd, and disables the submit button. Theinoperator check with trimmed name is correct.web/src/pages/setup/SetupAgentCard.tsx (2)
70-76: LGTM! Async callback properly wrapped to avoid returning promise.The
onChangehandler now correctly wraps the async call withvoidinside a block statement, preventing the promise from being returned toSelectFieldwhich expects a synchronous callback.
32-39: LGTM! Personality options correctly memoized.The
useMemoproperly maps preset names to options with human-readable labels (underscores replaced with spaces) and is correctly dependent onpersonalityPresets.tests/unit/api/controllers/test_setup.py (2)
454-479: LGTM! Test correctly validates Quick Setup mode completion without agents.The test properly:
- Seeds company name in settings
- Registers a stub provider to satisfy the provider requirement
- Validates 201 response with
setup_complete=True- Cleans up all modified settings in the finally block
Based on learnings: "Use vendor-agnostic names in tests:
test-provider,test-small-001" - the test correctly usestest-provider.
1566-1680: LGTM! Comprehensive test coverage for new personality endpoints.Tests cover all key scenarios:
- Happy path with persistence verification
- Invalid preset rejection (400)
- Out-of-range agent index (404)
- Conflict when setup already complete (409)
- Preset listing returns non-empty list with correct field shapes
src/synthorg/api/controllers/setup_personality.py (2)
79-114: LGTM! Properly acquires lock for agent mutation.The endpoint correctly follows the safe pattern from other agent-mutating endpoints:
- Acquires
_AGENT_LOCKbefore reading/writing agents- Validates index within the lock
- Creates a new list immutably (no in-place mutation)
- Persists atomically
- Logs after lock release (safe since
agentsis a local copy)
116-151: LGTM! Read-only endpoint with appropriate access control and logging.The
list_personality_presetsendpoint:
- Uses
require_read_accessguard (appropriate for read-only)- Builds response from sorted preset keys
- Logs at DEBUG level (appropriate for non-state-changing read)
docs/design/operations.md (1)
1111-1111: LGTM! Documentation updated with consistent route semantics.The endpoint list now:
- Uses
{agent_index}consistently across all agent routes- Includes the zero-based/unstable-index note applying to the whole route group
- Documents new personality-related endpoints
- Correctly states that agents are optional for Quick Setup mode completion
web/src/pages/setup/ProviderProbeResults.tsx (1)
99-107: LGTM! Re-scan button properly implemented.The refresh button:
- Disables while probing
- Shows spinning animation during probe
- Has appropriate aria-label for accessibility
- Correctly uses
void onReprobe()to ignore the promise resultsrc/synthorg/api/controllers/setup_models.py (2)
281-299: LGTM! Request model with proper validation.
UpdateAgentPersonalityRequestcorrectly:
- Uses
frozen=Trueandextra="forbid"- Validates via
_normalize_and_validate_presetwithout fallback (preset is required)- Uses
NotBlankStrfor the personality_preset field
302-325: LGTM! Response models are properly frozen.Both
PersonalityPresetInfoResponseandPersonalityPresetsListResponseuse:
frozen=Truefor immutabilityextra="forbid"to reject unknown fields- Appropriate field types (
NotBlankStr,tuple)web/src/stores/setup-wizard.ts (3)
325-348: LGTM! Template-derived state correctly cleared on quick mode switch.The
setWizardModeimplementation properly:
- Clears
selectedTemplate,comparedTemplates, andtemplateVariableswhen switching to quick mode- Marks
template,agents, andthemesteps as incomplete- Preserves state when staying in or switching to guided mode
This addresses the concern about stale
selectedTemplatebeing sent during Quick Setup submission.
544-571: LGTM! Discovery failures now properly surfaced to UI.The post-creation model discovery:
- Attempts discovery only when
models.length === 0- On failure, sets
providersErrorwith a user-friendly message- Keeps the provider in the store (creation succeeded)
- Suggests using the re-probe button as recovery action
277-286: LGTM! Step navigation now guards against invalid steps.The early return when
targetIdx === -1prevents navigation to steps not in the current flow (e.g., navigating to 'agents' in quick mode), which is good defensive programming.
| try: | ||
| entry = await settings_svc.get_entry( | ||
| "company", | ||
| "name_locales", | ||
| ) | ||
| except MemoryError, RecursionError: | ||
| raise | ||
| except SettingNotFoundError: | ||
| return False | ||
| except Exception: | ||
| logger.warning( | ||
| SETUP_STATUS_SETTINGS_UNAVAILABLE, | ||
| setting="name_locales", | ||
| exc_info=True, | ||
| ) | ||
| return False | ||
| if entry.source != SettingSource.DATABASE or not entry.value: | ||
| return False | ||
| try: | ||
| parsed = json.loads(entry.value) | ||
| except json.JSONDecodeError, TypeError: | ||
| logger.warning( | ||
| SETUP_NAME_LOCALES_CORRUPTED, | ||
| reason="invalid_json_or_type", | ||
| raw=entry.value[:200] if entry.value else None, | ||
| ) | ||
| return False | ||
| return isinstance(parsed, list) and len(parsed) > 0 |
There was a problem hiding this comment.
Corrupted name_locales values still escape the fallback path.
Line 346 handles TypeError, but Line 350 can immediately re-raise on truthy non-string values by slicing entry.value[:200]. Separately, parse_locale_json() only catches JSONDecodeError, so read_name_locales() still propagates TypeError instead of returning None as documented. Please centralize both paths on one parser and validate list items before resolve_locales() runs.
🛠️ Suggested fix
async def check_has_name_locales(
settings_svc: SettingsService,
) -> bool:
@@
- try:
- parsed = json.loads(entry.value)
- except json.JSONDecodeError, TypeError:
- logger.warning(
- SETUP_NAME_LOCALES_CORRUPTED,
- reason="invalid_json_or_type",
- raw=entry.value[:200] if entry.value else None,
- )
- return False
- return isinstance(parsed, list) and len(parsed) > 0
+ parsed = parse_locale_json(entry.value)
+ return bool(parsed)
@@
def parse_locale_json(raw: str) -> list[str] | None:
@@
try:
parsed = json.loads(raw)
- except json.JSONDecodeError:
+ except json.JSONDecodeError, TypeError:
logger.warning(
SETUP_NAME_LOCALES_CORRUPTED,
- reason="invalid_json",
- raw=raw[:200] if raw else None,
+ reason="invalid_json_or_type",
+ raw=raw[:200] if isinstance(raw, str) and raw else repr(raw)[:200],
)
return None
if not isinstance(parsed, list):
logger.warning(
SETUP_NAME_LOCALES_CORRUPTED,
reason="expected_list",
actual_type=type(parsed).__name__,
)
return None
+ if not all(isinstance(locale, str) and locale.strip() for locale in parsed):
+ logger.warning(
+ SETUP_NAME_LOCALES_CORRUPTED,
+ reason="invalid_list_items",
+ )
+ return None
return parsedAs per coding guidelines "Validate at system boundaries (user input, external APIs, config files)."
Also applies to: 431-452, 455-496
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/api/controllers/setup_helpers.py` around lines 326 - 353, The
code in read_name_locales() currently slices entry.value and only catches
JSONDecodeError, allowing TypeError from non-string entry.value to propagate;
centralize parsing by sending entry.value into parse_locale_json() (or update
parse_locale_json()) to catch both JSONDecodeError and TypeError and return None
on failure, log using SETUP_NAME_LOCALES_CORRUPTED with safe raw preview only
after confirming entry.value is a str, and ensure read_name_locales() validates
that the parsed result is a list of strings (no truthy non-string elements)
before calling resolve_locales(); update handling in read_name_locales(),
parse_locale_json(), and any callers of resolve_locales() to use this validated
list and to return False/None on invalid input.
| async def persist_company_settings( | ||
| settings_svc: SettingsService, | ||
| company_name: str, | ||
| description: str | None, | ||
| departments_json: str, | ||
| ) -> None: | ||
| """Write company name, description, and departments.""" | ||
| await settings_svc.set( | ||
| "company", | ||
| "company_name", | ||
| company_name, | ||
| ) | ||
| await settings_svc.set( | ||
| "company", | ||
| "description", | ||
| description or "", | ||
| ) | ||
| await settings_svc.set( | ||
| "company", | ||
| "departments", | ||
| departments_json or "[]", | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
service_file="$(fd 'service\.py$' src/synthorg/settings | head -n 1)"
echo "== SettingsService file =="
echo "$service_file"
echo
echo "== Write-related methods on SettingsService =="
rg -n -C4 --type=py \
'class SettingsService|async def (set|set_many|update|bulk|transaction)|def (set|set_many|update|bulk|transaction)' \
"$service_file"
echo
echo "== Existing batch/transaction helpers in settings/persistence layers =="
rg -n -C2 --type=py \
'set_many|bulk|atomic|transaction' \
src/synthorg/settings src/synthorg/persistenceRepository: Aureliolo/synthorg
Length of output: 1421
🏁 Script executed:
# Check the full SettingsService API and set() implementation
cat -n src/synthorg/settings/service.py | head -n 500 | tail -n 300Repository: Aureliolo/synthorg
Length of output: 13313
🏁 Script executed:
# Check if there's transaction support in the database layer
rg -n "transaction|atomic" src/synthorg/persistence --type=py | head -20Repository: Aureliolo/synthorg
Length of output: 171
🏁 Script executed:
# Look at the check_has_company function to understand the consistency check
rg -n "def check_has_company|async def check_has_company" -A 10 src/synthorgRepository: Aureliolo/synthorg
Length of output: 905
🏁 Script executed:
# Check the actual file context around the problematic function
cat -n src/synthorg/api/controllers/setup_helpers.py | sed -n '550,585p'Repository: Aureliolo/synthorg
Length of output: 1137
🏁 Script executed:
cat -n src/synthorg/api/controllers/setup_helpers.py | sed -n '183,210p'Repository: Aureliolo/synthorg
Length of output: 1082
🏁 Script executed:
cat -n src/synthorg/settings/service.py | sed -n '466,520p'Repository: Aureliolo/synthorg
Length of output: 2319
🏁 Script executed:
# Check the repository layer for transaction support
fd 'repository' src/synthorg/settings --type=py | head -5Repository: Aureliolo/synthorg
Length of output: 231
🏁 Script executed:
# Look for DB transaction or atomic write support
rg -n "async def set|execute|insert|UPDATE|BEGIN|COMMIT|transaction" src/synthorg/persistence --type=py -B2 -A8 | head -50Repository: Aureliolo/synthorg
Length of output: 4359
🏁 Script executed:
# Check full set() implementation in repository
cat -n src/synthorg/persistence/sqlite/settings_repo.py | sed -n '99,130p'Repository: Aureliolo/synthorg
Length of output: 1322
🏁 Script executed:
# Check if the DB connection supports transactions
rg -n "class.*Repo|class.*Connection|transaction|begin|commit|rollback" src/synthorg/persistence/sqlite --max-count=30Repository: Aureliolo/synthorg
Length of output: 3648
🏁 Script executed:
# Check caller of persist_company_settings to see if error handling is at that level
rg -n "persist_company_settings" src/synthorg --context=5Repository: Aureliolo/synthorg
Length of output: 2536
🏁 Script executed:
# Check the caller (setup.py) around line 259 to see if there's error handling
cat -n src/synthorg/api/controllers/setup.py | sed -n '240,280p'Repository: Aureliolo/synthorg
Length of output: 1816
🏁 Script executed:
# Check if persist_company_settings has any error handling or logging
cat -n src/synthorg/api/controllers/setup_helpers.py | sed -n '558,600p'Repository: Aureliolo/synthorg
Length of output: 1492
🏁 Script executed:
# Verify if SettingsService has any batch method we might have missed
rg -n "def set_many|async def set_many|def set_batch|async def set_batch|class SettingsService" src/synthorg/settings/service.py | head -20Repository: Aureliolo/synthorg
Length of output: 88
Add transaction or batch write API for persisting related company settings.
Lines 565–579 execute three independent database writes with separate commits. If the second or third set() call fails, company_name is already persisted and check_has_company() returns True (since it only checks for the existence of company_name at database source). Subsequent setup steps then observe a half-written company record.
Add either:
- A batch/transactional
set_many()method toSettingsServicethat writes all three fields in a single transaction, or - Explicit transaction handling with rollback logic and error logging before raising.
Per coding guidelines, log the error path at WARNING or ERROR with context before raising.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/api/controllers/setup_helpers.py` around lines 558 - 579,
persist_company_settings performs three independent awaits on
SettingsService.set which can leave a partially written company if one call
fails; change this to use an atomic batch/transaction API (e.g., add and call
SettingsService.set_many or a transactional method like
SettingsService.transaction or SettingsService.run_in_transaction to set
company_name, description, and departments in one commit) or wrap the three sets
in explicit transaction handling with rollback on error; ensure you log failures
at WARNING/ERROR with context (including company_name and which fields failed)
before re-raising the exception so callers don’t observe a half-written company
via check_has_company().
| def _normalize_and_validate_preset( | ||
| raw: object, | ||
| fallback: str = "", | ||
| ) -> str: | ||
| """Normalize *raw* to a valid personality preset key. | ||
|
|
||
| Args: | ||
| raw: Raw preset value from the request payload. | ||
| fallback: Default key when *raw* is falsy. | ||
|
|
||
| Returns: | ||
| Normalized lowercase preset key. | ||
|
|
||
| Raises: | ||
| ValueError: If the resolved key is not in ``PERSONALITY_PRESETS``. | ||
| """ | ||
| from synthorg.templates.presets import ( # noqa: PLC0415 | ||
| PERSONALITY_PRESETS, | ||
| ) | ||
|
|
||
| if not raw or not str(raw).strip(): | ||
| if not fallback: | ||
| msg = "personality_preset is required" | ||
| raise ValueError(msg) | ||
| key = fallback | ||
| else: | ||
| key = str(raw).strip().lower() | ||
|
|
||
| if key not in PERSONALITY_PRESETS: | ||
| available = sorted(PERSONALITY_PRESETS) | ||
| msg = f"Unknown personality preset {raw!r}. Available: {available}" | ||
| raise ValueError(msg) | ||
| return key |
There was a problem hiding this comment.
Add warning log for unknown preset before raising.
The helper validates correctly but doesn't log a warning for unknown presets, unlike the reference get_personality_preset() function. Per coding guidelines: "All error paths must log at WARNING or ERROR with context before raising."
🛡️ Suggested fix to add logging
+from synthorg.observability import get_logger
+from synthorg.observability.events.setup import (
+ SETUP_PERSONALITY_PRESET_INVALID,
+)
+
+logger = get_logger(__name__)
+
def _normalize_and_validate_preset(
raw: object,
fallback: str = "",
) -> str:
...
if key not in PERSONALITY_PRESETS:
available = sorted(PERSONALITY_PRESETS)
msg = f"Unknown personality preset {raw!r}. Available: {available}"
+ logger.warning(
+ SETUP_PERSONALITY_PRESET_INVALID,
+ preset_name=raw,
+ available=available,
+ )
raise ValueError(msg)
return key🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/api/controllers/setup_models.py` around lines 12 - 44, The
_normalize_and_validate_preset helper currently raises ValueError for unknown
presets without logging; update it to log a warning with context before raising:
import or use the module logger (consistent with get_personality_preset) and
call logger.warning including the invalid raw value and available keys
(PERSONALITY_PRESETS) immediately before the ValueError is raised in the branch
where key not in PERSONALITY_PRESETS so the warning mirrors the raised message
and provides context for debugging.
| const stepOrder: WizardStep[] = ['template', 'company', 'providers', 'agents', 'theme', 'complete'] | ||
|
|
||
| const defaultStepsCompleted: Record<WizardStep, boolean> = { | ||
| account: false, | ||
| mode: false, | ||
| template: false, | ||
| company: false, | ||
| agents: false, | ||
| providers: false, | ||
| agents: false, | ||
| theme: false, | ||
| complete: false, | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify whether fast-check properties are present in this test file and sibling setup tests.
rg -n --type=ts --type=tsx 'fc\.(assert|property)\s*\(' web/src/__tests__/pages/setup/WizardProgress.test.tsx
rg -n --type=ts --type=tsx 'fc\.(assert|property)\s*\(' web/src/__tests__/pages/setupRepository: Aureliolo/synthorg
Length of output: 89
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the test file to understand its structure
echo "=== WizardProgress.test.tsx Content ==="
cat -n web/src/__tests__/pages/setup/WizardProgress.test.tsx
echo ""
echo "=== Searching for fast-check in this test file ==="
rg 'fc\.' web/src/__tests__/pages/setup/WizardProgress.test.tsx || echo "No fast-check found"
echo ""
echo "=== Checking for fast-check imports across setup test directory ==="
rg 'fast-check' web/src/__tests__/pages/setup/ || echo "No fast-check imports found in setup tests"
echo ""
echo "=== Checking fast-check usage in other test files in the codebase ==="
rg 'fc\.' web/src/__tests__/ --max-count=5 || echo "No fast-check found in test files"Repository: Aureliolo/synthorg
Length of output: 16321
Add property-based tests for wizard-step invariants.
This test file uses only example-based assertions but the coding guideline requires property-based testing with fast-check for all web/**/*.{ts,tsx,js,jsx} files. Add at least one fc.assert() + fc.property() test—for example, to verify label rendering invariants across valid WizardStep[] permutations or completion state transitions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/__tests__/pages/setup/WizardProgress.test.tsx` around lines 6 - 17,
Add a fast-check property test in WizardProgress.test.tsx that uses fc.assert
and fc.property to verify an invariant across randomized inputs: for example,
generate permutations of the stepOrder array (or arbitrary subsets of WizardStep
using the WizardStep union) and arbitrary completion maps based on
defaultStepsCompleted, then render the WizardProgress (or call its
label/completion helpers) and assert that every step in the generated order
either renders a label from the known stepOrder set and its completed state
equals the value in the generated completion map; reference WizardStep,
stepOrder, defaultStepsCompleted and the WizardProgress rendering helpers in the
test so the property ties generated inputs to the component invariants. Ensure
the test imports fast-check (fc.assert, fc.property) and uses only valid
WizardStep values when building permutations or maps.
- parse_locale_json: catch TypeError alongside JSONDecodeError - MiniOrgChart: extract truncation constants, derive connector offset - Store: deduplicate probe logic into shared runProbeAll helper Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
web/src/pages/setup/MiniOrgChart.tsx (3)
177-186:⚠️ Potential issue | 🟠 MajorHeight scaling still depends on
maxAgentsInDeptinstead of rendered content/team size.The current
svgHeightformula grows with the largest department count, but this chart renders one agent row per department. That can add large blank vertical space and make the chart appear compressed. KeepsvgHeighttied to actual content bounds, and scaledisplayHeightfrom total team size.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/setup/MiniOrgChart.tsx` around lines 177 - 186, The svgHeight is incorrectly based on maxAgentsInDept (largest department) which creates excessive vertical gaps; update the sizing to use the actual rendered rows and overall team size: replace maxAgentsInDept in the svgHeight calculation with departments.length (since one agent row is rendered per department), and compute a totalTeamSize (e.g., totalAgents = departments.reduce((s,d)=>s + d.agents.length, 0) or use departments.length for per-row height) to drive displayHeight scaling; adjust displayHeight to Math.min(Math.max(svgHeightDerivedFromRowsOrTotalTeamSize, MIN_HEIGHT), MAX_HEIGHT) and remove dependence on maxAgentsInDept. Ensure you update references to maxAgentsInDept, svgHeight, and displayHeight accordingly.
50-51:⚠️ Potential issue | 🟡 MinorConnector anchor remains coupled to a duplicated constant (
18).
DEPT_NODE_HALF_HEIGHTduplicatesnodeHeight / 2logic. IfnodeHeightchanges, connector alignment can silently drift. Derive the anchor directly fromnodeHeightand thread it throughAgentNode.Also applies to: 59-60, 172-172
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/setup/MiniOrgChart.tsx` around lines 50 - 51, DEPT_NODE_HALF_HEIGHT duplicates nodeHeight/2 and can drift when nodeHeight changes; remove the hard-coded constant and compute the half-height from the nodeHeight value instead, then pass that computed value into the components that need it (thread it through the AgentNode props and any connector/layout functions currently referencing DEPT_NODE_HALF_HEIGHT). Update usages in MiniOrgChart (where DEPT_NODE_HALF_HEIGHT is declared and referenced), AgentNode (add/accept a halfHeight or nodeHeight prop and use it for anchor calculations), and any connector rendering logic to use the passed-in derived value so connectors remain correctly aligned when nodeHeight changes.
20-27:⚠️ Potential issue | 🟠 MajorLayout spacing is still hardcoded in pixels in setup UI geometry.
AGENT_SPACING_GAP,hGap, andvGapare still pixel literals in this page. These should come from shared spacing tokens/centralized design constants so density changes don’t drift this chart from the rest of setup UI.As per coding guidelines: "Use design token rules... spacing via density-aware tokens ... Never ... hardcode pixel values for layout spacing."
Also applies to: 166-175
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/setup/MiniOrgChart.tsx` around lines 20 - 27, Replace hardcoded pixel spacing with the shared design spacing tokens: swap AGENT_SPACING_GAP and any local pixel literals used to compute hGap and vGap (and the similar values around the SMALL_/LARGE_* geometry) to read from the centralized spacing/density tokens (e.g., import the project's spacing token or density-aware spacing utility and use its small/medium/large values) so spacing adapts with density; update references to AGENT_SPACING_GAP, hGap, and vGap to compute from those tokens (and map SMALL_AGENT/LARGE variants to appropriate token names) rather than numeric px literals, and ensure the same change is applied to the similar values in the block around the previous 166-175 logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/api/controllers/setup_helpers.py`:
- Around line 143-158: The two except blocks handling QueryError and Exception
in the function that logs SETUP_STATUS_SETTINGS_UNAVAILABLE are redundant;
replace them with a single "except Exception as exc:" handler that performs the
same logger.warning call (with context="admin_count", error=str(exc),
exc_info=True) and returns True, removing the specific QueryError branch (unless
QueryError needs special handling elsewhere).
In `@web/src/stores/setup-wizard.ts`:
- Around line 35-61: The function runProbeAll is defined between import blocks
which breaks module organization; move the entire runProbeAll declaration so all
import statements (including DEFAULT_CURRENCY and CurrencyCode) are grouped at
the top of the file and the function appears after them. Locate the runProbeAll
function (which calls probePreset and returns
Promise<Record<string,ProbePresetResponse>>) and cut/paste it below the last
existing import, ensuring any referenced symbols (probePreset,
ProbePresetResponse) remain in scope and no duplicate imports are introduced.
---
Duplicate comments:
In `@web/src/pages/setup/MiniOrgChart.tsx`:
- Around line 177-186: The svgHeight is incorrectly based on maxAgentsInDept
(largest department) which creates excessive vertical gaps; update the sizing to
use the actual rendered rows and overall team size: replace maxAgentsInDept in
the svgHeight calculation with departments.length (since one agent row is
rendered per department), and compute a totalTeamSize (e.g., totalAgents =
departments.reduce((s,d)=>s + d.agents.length, 0) or use departments.length for
per-row height) to drive displayHeight scaling; adjust displayHeight to
Math.min(Math.max(svgHeightDerivedFromRowsOrTotalTeamSize, MIN_HEIGHT),
MAX_HEIGHT) and remove dependence on maxAgentsInDept. Ensure you update
references to maxAgentsInDept, svgHeight, and displayHeight accordingly.
- Around line 50-51: DEPT_NODE_HALF_HEIGHT duplicates nodeHeight/2 and can drift
when nodeHeight changes; remove the hard-coded constant and compute the
half-height from the nodeHeight value instead, then pass that computed value
into the components that need it (thread it through the AgentNode props and any
connector/layout functions currently referencing DEPT_NODE_HALF_HEIGHT). Update
usages in MiniOrgChart (where DEPT_NODE_HALF_HEIGHT is declared and referenced),
AgentNode (add/accept a halfHeight or nodeHeight prop and use it for anchor
calculations), and any connector rendering logic to use the passed-in derived
value so connectors remain correctly aligned when nodeHeight changes.
- Around line 20-27: Replace hardcoded pixel spacing with the shared design
spacing tokens: swap AGENT_SPACING_GAP and any local pixel literals used to
compute hGap and vGap (and the similar values around the SMALL_/LARGE_*
geometry) to read from the centralized spacing/density tokens (e.g., import the
project's spacing token or density-aware spacing utility and use its
small/medium/large values) so spacing adapts with density; update references to
AGENT_SPACING_GAP, hGap, and vGap to compute from those tokens (and map
SMALL_AGENT/LARGE variants to appropriate token names) rather than numeric px
literals, and ensure the same change is applied to the similar values in the
block around the previous 166-175 logic.
🪄 Autofix (Beta)
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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 22851eb8-c3c9-48ac-83c3-781b63b4fff6
📒 Files selected for processing (3)
src/synthorg/api/controllers/setup_helpers.pyweb/src/pages/setup/MiniOrgChart.tsxweb/src/stores/setup-wizard.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: Dashboard Test
- GitHub Check: Test (Python 3.14)
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always reuse existing components from
web/src/components/ui/before creating new ones. Do not recreate status dots inline, build card-with-header layouts from scratch, create metric displays manually, render initials circles manually, create complex (>8 line) JSX inside.map()blocks, or usergba()with hardcoded values instead of design token variables.
Files:
web/src/pages/setup/MiniOrgChart.tsxweb/src/stores/setup-wizard.ts
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use design token rules: colors via Tailwind semantic classes (
text-foreground,bg-card,text-accent,text-success,bg-danger) or CSS variables (var(--so-accent)), typography viafont-sansorfont-mono, spacing via density-aware tokens (p-card,gap-section-gap,gap-grid-gap) or standard Tailwind, shadows/borders via token variables (var(--so-shadow-card-hover),border-border,border-bright). Never hardcode hex values, set fontFamily directly, or hardcode pixel values for layout spacing.
Files:
web/src/pages/setup/MiniOrgChart.tsxweb/src/stores/setup-wizard.ts
web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use property-based testing with fast-check (
fc.assert+fc.property). Run tests withnpm --prefix web run testfor Vitest with--detect-async-leaksflag.
Files:
web/src/pages/setup/MiniOrgChart.tsxweb/src/stores/setup-wizard.ts
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations-- Python 3.14+ has PEP 649 native lazy annotations. Useexcept A, B:(no parentheses) per PEP 758 except syntax as enforced by ruff.
All public functions and classes require type hints and Google-style docstrings (enforced by ruff D rules). Use mypy strict mode.
Functions must be less than 50 lines, files less than 800 lines. Handle errors explicitly, never silently swallow. Validate at system boundaries (user input, external APIs, config files).
Files:
src/synthorg/api/controllers/setup_helpers.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic must have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__). Use event name constants fromsynthorg.observability.events.<domain>modules (e.g.,API_REQUEST_STARTED,TOOL_INVOKE_START). Always use structured kwargs:logger.info(EVENT, key=value). Log all error paths at WARNING or ERROR with context before raising, all state transitions at INFO, and DEBUG for object creation and key function flow. Pure data models, enums, and re-exports do not need logging.
Use immutability patterns: create new objects instead of mutating existing ones. For non-Pydantic internal collections (registries,BaseTool), usecopy.deepcopy()at construction plusMappingProxyTypewrapping for read-only enforcement. Fordict/listfields in frozen Pydantic models, usecopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization).
Use Pydantic v2 models (BaseModel,model_validator,computed_field,ConfigDict). Use@computed_fieldfor derived values instead of storing redundant fields (e.g.,TokenUsage.total_tokens). UseNotBlankStrfromcore.typesfor all identifier/name fields (including optionalNotBlankStr | Noneand tuple variants) instead of manual whitespace validators.
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations (e.g., multiple tool invocations, parallel agent calls) in new code instead of barecreate_task. Favor structured concurrency.
Separate frozen Pydantic models for config/identity from mutable-via-copy models for runtime state that evolves (e.g., agent execution state, task progress). Never mix static config fields with mutable runtime fields in one model.
Never useimport loggingorlogging.getLogger()in application code. Exception:observability/setup.pyandobservability/sinks.pymay use stdlibloggingandprint(..., file=sys.stderr)for bootst...
Files:
src/synthorg/api/controllers/setup_helpers.py
src/synthorg/api/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use RFC 9457 errors in the REST API. Implement role-based access control (RBAC) guards in the API layer.
Files:
src/synthorg/api/controllers/setup_helpers.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Line length: 88 characters (ruff enforces).
Files:
src/synthorg/api/controllers/setup_helpers.py
🧠 Learnings (12)
📚 Learning: 2026-03-28T18:10:43.542Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T18:10:43.542Z
Learning: Applies to **/*.{ts,tsx} : Always reuse existing components from `web/src/components/ui/` before creating new ones. Do not recreate status dots inline, build card-with-header layouts from scratch, create metric displays manually, render initials circles manually, create complex (>8 line) JSX inside `.map()` blocks, or use `rgba()` with hardcoded values instead of design token variables.
Applied to files:
web/src/pages/setup/MiniOrgChart.tsx
📚 Learning: 2026-03-28T18:10:43.543Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T18:10:43.543Z
Learning: Applies to web/src/**/*.{ts,tsx} : Use design token rules: colors via Tailwind semantic classes (`text-foreground`, `bg-card`, `text-accent`, `text-success`, `bg-danger`) or CSS variables (`var(--so-accent)`), typography via `font-sans` or `font-mono`, spacing via density-aware tokens (`p-card`, `gap-section-gap`, `gap-grid-gap`) or standard Tailwind, shadows/borders via token variables (`var(--so-shadow-card-hover)`, `border-border`, `border-bright`). Never hardcode hex values, set fontFamily directly, or hardcode pixel values for layout spacing.
Applied to files:
web/src/pages/setup/MiniOrgChart.tsx
📚 Learning: 2026-03-28T18:10:43.543Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T18:10:43.543Z
Learning: Applies to web/src/components/ui/*.{ts,tsx} : New shared components in `web/src/components/ui/` must have a corresponding `.stories.tsx` file with all states (default, hover, loading, error, empty), exported props as a TypeScript interface, design tokens exclusively (no hardcoded colors, fonts, or spacing), and use `cn()` from `@/lib/utils` for conditional class merging.
Applied to files:
web/src/pages/setup/MiniOrgChart.tsx
📚 Learning: 2026-03-21T11:08:01.542Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-21T11:08:01.542Z
Learning: Applies to **/*.{py,md} : Line length: 88 characters (ruff).
Applied to files:
web/src/stores/setup-wizard.ts
📚 Learning: 2026-03-28T18:10:43.543Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T18:10:43.543Z
Learning: Applies to src/**/*.py : Line length: 88 characters (ruff enforces).
Applied to files:
web/src/stores/setup-wizard.ts
📚 Learning: 2026-03-15T21:49:53.264Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:49:53.264Z
Learning: Fix everything valid — never skip when review agents find valid issues (including pre-existing issues in surrounding code, suggestions, and findings adjacent to the PR's changes). No deferring, no 'out of scope' skipping.
Applied to files:
web/src/stores/setup-wizard.ts
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to src/synthorg/templates/**/*.py : Templates: pre-built company templates, personality presets, and builder.
Applied to files:
web/src/stores/setup-wizard.ts
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/api/**/*.py : API package (api/): Litestar REST + WebSocket with controllers, guards, channels, JWT + API key + WS ticket auth, approval gate integration, coordination endpoint, collaboration endpoint, settings endpoint, provider management endpoint (CRUD + test + presets), backup endpoint, RFC 9457 structured errors, AppState hot-reload slots, service auto-wiring (Phase 1 at construction, Phase 2 on startup), lifecycle helpers
Applied to files:
src/synthorg/api/controllers/setup_helpers.py
📚 Learning: 2026-03-26T15:18:16.848Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T15:18:16.848Z
Learning: Applies to src/synthorg/api/**/*.py : Litestar API must include setup wizard, auth/, auto-wiring, and lifecycle management
Applied to files:
src/synthorg/api/controllers/setup_helpers.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : Package structure: src/synthorg/ organized as: api/ (REST+WebSocket, Litestar), auth/ (auth subpackage), backup/ (scheduled/manual backups), budget/ (cost tracking, CFO), cli/ (superseded by Go CLI), communication/ (message bus, meetings), config/ (YAML loading), core/ (domain models, resilience config), engine/ (orchestration, task state, coordination, approval gates, stagnation detection, context budget, compaction), hr/ (hiring, performance, promotion), memory/ (pluggable backend, Mem0, retrieval, consolidation), persistence/ (operational data, SQLite, settings), observability/ (logging, correlation, sinks), providers/ (LLM abstraction, LiteLLM, auth types, presets, runtime CRUD), settings/ (runtime-editable, typed definitions, encryption, config bridge), security/ (SecOps, rule engine, output scanning, progressive trust, autonomy levels), templates/ (company templates, personalities), tools/ (registry, built-in tools, git, sandbox, code_runner, MCP...
Applied to files:
src/synthorg/api/controllers/setup_helpers.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Handle errors explicitly, never silently swallow. Validate at system boundaries (user input, external APIs, config files).
Applied to files:
src/synthorg/api/controllers/setup_helpers.py
📚 Learning: 2026-03-16T20:14:00.937Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T20:14:00.937Z
Learning: Applies to **/*.py : Validate: at system boundaries (user input, external APIs, config files).
Applied to files:
src/synthorg/api/controllers/setup_helpers.py
🔇 Additional comments (21)
web/src/pages/setup/MiniOrgChart.tsx (1)
27-30: Good refactor: truncation thresholds are now explicit constants.Extracting
DEPT_LABEL_MAX_DISPLAYandDEPT_LABEL_TRUNCATE_ATimproves readability and future tuning safety.Also applies to: 117-119
web/src/stores/setup-wizard.ts (7)
63-109: LGTM!The step order logic cleanly handles both admin and mode variants. The readonly arrays and helper function provide good encapsulation.
160-163: LGTM!New personality preset state and actions follow established patterns in the store.
Also applies to: 189-189, 211-212, 220-220
234-295: LGTM!Initial state correctly defaults to guided mode starting at the mode selection step, with all new fields properly initialized.
302-373: LGTM!Navigation logic is well-implemented:
setStepcorrectly guards against invalid steps for the current flow.setWizardModeproperly resets template-derived state when switching to quick mode, preventing stale data from being submitted.
510-532: LGTM!Personality preset actions follow established store patterns with proper loading/error state management.
569-596: LGTM!Discovery failures are now properly surfaced via
providersErrorwith actionable guidance, addressing the previous concern about silently proceeding with zero models.
615-627: LGTM!Good refactor extracting the shared probe logic into
runProbeAll. The helper reduces duplication and provides consistent error logging.src/synthorg/api/controllers/setup_helpers.py (13)
344-353: Unsafe slicing in exception handler can still raise.Line 350 attempts
entry.value[:200]whenentry.valueis truthy. Ifentry.valueis a non-string type (e.g.,intordict) that caused theTypeErrorfromjson.loads, the slicing operation will raise anotherTypeError, escaping the handler.🛡️ Proposed fix
except json.JSONDecodeError, TypeError: logger.warning( SETUP_NAME_LOCALES_CORRUPTED, reason="invalid_json_or_type", - raw=entry.value[:200] if entry.value else None, + raw=entry.value[:200] if isinstance(entry.value, str) and entry.value else repr(entry.value)[:200], ) return False
431-452: Return typelist[str]not enforced — list item validation missing.The function returns
list[str] | Nonebut only validates thatparsedis a list, not that all items are strings. Downstream code likeresolve_localesmay fail on non-string items.🛡️ Proposed fix to validate list items
if not isinstance(parsed, list): logger.warning( SETUP_NAME_LOCALES_CORRUPTED, reason="expected_list", actual_type=type(parsed).__name__, ) return None + if not all(isinstance(item, str) for item in parsed): + logger.warning( + SETUP_NAME_LOCALES_CORRUPTED, + reason="invalid_list_items", + ) + return None return parsed
558-579: Non-atomic writes can leave partial company state.The three independent
set()calls can leave a half-written company record if the second or third call fails. Sincecheck_has_company()only verifiescompany_nameexistence, subsequent steps may observe inconsistent state.
1-63: LGTM!Module structure is clean. Proper use of
get_logger(__name__),TYPE_CHECKINGfor deferred imports, and derivingDEFAULT_MIN_PASSWORD_LENGTHfromAuthConfig.model_fieldsto prevent silent divergence.
65-81: LGTM!Clear validation with appropriate logging before raising, and descriptive error messages for both empty and non-empty agent lists.
83-131: LGTM!Non-fatal error handling is appropriate — setup completion should succeed even if runtime re-init fails, with proper warning logs for operator visibility. The late import avoids circular dependencies cleanly.
162-227: LGTM!Both functions follow consistent error handling patterns with appropriate fail-open behavior and logging.
229-312: LGTM!Thorough validation with clear error paths. The locale validation correctly handles sentinel mixing, invalid codes, and duplicates with appropriate logging.
356-396: LGTM!Good defensive pattern —
max(parsed, DEFAULT_MIN_PASSWORD_LENGTH)ensures the minimum is always enforced, andraw_pw_valueis captured before parsing to provide context in error logs.
410-428: LGTM!The
AGENT_LOCKcorrectly serializes the read-modify-write sequence. The function appropriately delegates error handling to callers.
455-529: LGTM!
read_name_localescorrectly delegates toparse_locale_json, andis_setup_completeappropriately logs at ERROR level before propagating unexpected exceptions.
532-555: LGTM!Clean
NamedTupledesign with sensible defaults for the no-template case.
582-620: LGTM!Clean error translation from domain exceptions to API-friendly errors with appropriate warning logs including template name and error context.
- Consolidate duplicate QueryError/Exception handlers in check_needs_admin - Move runProbeAll after all imports (was splitting import block) - Derive AgentNode connector offset from nodeHeight prop (was hardcoded 18) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
src/synthorg/api/controllers/setup_helpers.py (2)
333-344:⚠️ Potential issue | 🟠 MajorCentralize
name_localesparsing and reject invalid list items.
check_has_name_locales()still bypassesparse_locale_json()and slicesentry.valuebefore confirming it is astr, so a truthy non-string setting can raise from the exception handler itself.parse_locale_json()also accepts any JSON list, which lets[123]or[""]count as configured locales and then flow intoresolve_locales()with the wrong element type.🛠️ Suggested fix
async def check_has_name_locales( @@ - try: - parsed = json.loads(entry.value) - except json.JSONDecodeError, TypeError: - logger.warning( - SETUP_NAME_LOCALES_CORRUPTED, - reason="invalid_json_or_type", - raw=entry.value[:200] if entry.value else None, - ) - return False - return isinstance(parsed, list) and len(parsed) > 0 + parsed = parse_locale_json(entry.value) + return bool(parsed) @@ def parse_locale_json(raw: str) -> list[str] | None: @@ - return parsed + if not all(isinstance(locale, str) and locale.strip() for locale in parsed): + logger.warning( + SETUP_NAME_LOCALES_CORRUPTED, + reason="invalid_list_items", + ) + return None + return parsedAs per coding guidelines "Validate at system boundaries (user input, external APIs, config files)."
Also applies to: 422-443, 478-487
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/api/controllers/setup_helpers.py` around lines 333 - 344, check_has_name_locales currently parses JSON itself and slices entry.value without ensuring it's a string, and parse_locale_json allows lists with non-string/empty items (e.g. [123] or [""]). Change check_has_name_locales to call parse_locale_json(entry.value) (after confirming entry.value is a str) instead of json.loads directly, and update parse_locale_json to validate the parsed value is a non-empty list of non-empty strings (reject numbers, empty strings, and other types); return False and log SETUP_NAME_LOCALES_CORRUPTED with reason when validation fails. Apply the same replacement/validation pattern used in check_has_name_locales to the other similar blocks mentioned (the regions around 422-443 and 478-487) so all name_locales parsing is centralized and consistent (functions to look for: check_has_name_locales, parse_locale_json, resolve_locales, SettingSource.DATABASE, entry.value).
549-570:⚠️ Potential issue | 🟠 MajorPersist the company block atomically.
These three
set()calls commit independently. If the second or third write fails,company_nameis already persisted andcheck_has_company()starts returningTrue, leaving the wizard in a half-written state. Wrap them in a single transaction/batch API, or add rollback plus error logging before re-raising.As per coding guidelines "Log all error paths at WARNING or ERROR with context before raising."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/api/controllers/setup_helpers.py` around lines 549 - 570, persist_company_settings currently issues three independent SettingsService.set calls so a failure leaves company_name persisted and check_has_company() true; change it to perform the three writes atomically (use the SettingsService transaction/batch API if available) or, if no transaction API exists, wrap the three set() calls in a try/except that on exception logs an ERROR/WARNING with context and rolls back the partial write(s) by deleting or resetting the previously-written keys (company_name, description, departments) before re-raising; update references in the function persist_company_settings and use SettingsService.set and check_has_company() symbols to locate the affected code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/api/controllers/setup_helpers.py`:
- Around line 157-170: The current try/except around settings_svc.get_entry
treats a missing "setup_complete" entry as an unexpected error and logs
SETUP_STATUS_SETTINGS_UNAVAILABLE; change it to explicitly handle the
SettingNotFoundError (the same case handled by is_setup_complete()) by returning
False (or the "not complete" equivalent) instead of warning, while keeping the
existing except Exception block for real errors; update the except clauses
around settings_svc.get_entry to catch SettingNotFoundError (and re-raise
MemoryError/RecursionError as before), and only log
SETUP_STATUS_SETTINGS_UNAVAILABLE for other exceptions.
In `@web/src/pages/setup/MiniOrgChart.tsx`:
- Around line 209-218: The root circle radius is hard-coded in two places in
MiniOrgChart (the <circle> element using r={10} and the connector using rootY +
10); extract a single ROOT_RADIUS constant (e.g., const ROOT_RADIUS = 10) near
the top of the component and replace r={10} with r={ROOT_RADIUS} and rootY + 10
with rootY + ROOT_RADIUS in the deptPositions map/line connector so the value is
centralized and cannot drift.
- Around line 77-78: In MiniOrgChart (or the component that renders the SVG
text), replace the hardcoded numeric font sizes by extracting named constants
(e.g., FONT_SIZE_LARGE and FONT_SIZE_SMALL) alongside the existing layout
constants and use them in the ternary expression currently using 10 and 8;
update the JSX to fontSize={radius > 14 ? FONT_SIZE_LARGE : FONT_SIZE_SMALL} so
the size relationship is explicit and maintainable and ensure the new constants
are exported/declared near other layout constants for discoverability.
In `@web/src/stores/setup-wizard.ts`:
- Around line 589-594: The current providersError message is misleading because
it tells the user to use the "re-probe button" even though reprobePresets() only
scans for new local provider availability and does not re-run model discovery
for an existing provider; update the error text in the set({ providersError: ...
}) call to instruct the user to either "ensure the local provider is running
with models available, then refresh the providers list" or "use a Retry model
discovery action for this provider" and, if desired, add/wire a new retry action
(e.g., retryModelDiscovery(providerName) or similar) that triggers model
discovery for the specific provider instead of calling reprobePresets().
---
Duplicate comments:
In `@src/synthorg/api/controllers/setup_helpers.py`:
- Around line 333-344: check_has_name_locales currently parses JSON itself and
slices entry.value without ensuring it's a string, and parse_locale_json allows
lists with non-string/empty items (e.g. [123] or [""]). Change
check_has_name_locales to call parse_locale_json(entry.value) (after confirming
entry.value is a str) instead of json.loads directly, and update
parse_locale_json to validate the parsed value is a non-empty list of non-empty
strings (reject numbers, empty strings, and other types); return False and log
SETUP_NAME_LOCALES_CORRUPTED with reason when validation fails. Apply the same
replacement/validation pattern used in check_has_name_locales to the other
similar blocks mentioned (the regions around 422-443 and 478-487) so all
name_locales parsing is centralized and consistent (functions to look for:
check_has_name_locales, parse_locale_json, resolve_locales,
SettingSource.DATABASE, entry.value).
- Around line 549-570: persist_company_settings currently issues three
independent SettingsService.set calls so a failure leaves company_name persisted
and check_has_company() true; change it to perform the three writes atomically
(use the SettingsService transaction/batch API if available) or, if no
transaction API exists, wrap the three set() calls in a try/except that on
exception logs an ERROR/WARNING with context and rolls back the partial write(s)
by deleting or resetting the previously-written keys (company_name, description,
departments) before re-raising; update references in the function
persist_company_settings and use SettingsService.set and check_has_company()
symbols to locate the affected code.
🪄 Autofix (Beta)
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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: aa5387a9-5053-40b2-9892-21c77df47560
📒 Files selected for processing (3)
src/synthorg/api/controllers/setup_helpers.pyweb/src/pages/setup/MiniOrgChart.tsxweb/src/stores/setup-wizard.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Dashboard Test
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always reuse existing components from
web/src/components/ui/before creating new ones. Do not recreate status dots inline, build card-with-header layouts from scratch, create metric displays manually, render initials circles manually, create complex (>8 line) JSX inside.map()blocks, or usergba()with hardcoded values instead of design token variables.
Files:
web/src/pages/setup/MiniOrgChart.tsxweb/src/stores/setup-wizard.ts
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use design token rules: colors via Tailwind semantic classes (
text-foreground,bg-card,text-accent,text-success,bg-danger) or CSS variables (var(--so-accent)), typography viafont-sansorfont-mono, spacing via density-aware tokens (p-card,gap-section-gap,gap-grid-gap) or standard Tailwind, shadows/borders via token variables (var(--so-shadow-card-hover),border-border,border-bright). Never hardcode hex values, set fontFamily directly, or hardcode pixel values for layout spacing.
Files:
web/src/pages/setup/MiniOrgChart.tsxweb/src/stores/setup-wizard.ts
web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use property-based testing with fast-check (
fc.assert+fc.property). Run tests withnpm --prefix web run testfor Vitest with--detect-async-leaksflag.
Files:
web/src/pages/setup/MiniOrgChart.tsxweb/src/stores/setup-wizard.ts
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations-- Python 3.14+ has PEP 649 native lazy annotations. Useexcept A, B:(no parentheses) per PEP 758 except syntax as enforced by ruff.
All public functions and classes require type hints and Google-style docstrings (enforced by ruff D rules). Use mypy strict mode.
Functions must be less than 50 lines, files less than 800 lines. Handle errors explicitly, never silently swallow. Validate at system boundaries (user input, external APIs, config files).
Files:
src/synthorg/api/controllers/setup_helpers.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic must have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__). Use event name constants fromsynthorg.observability.events.<domain>modules (e.g.,API_REQUEST_STARTED,TOOL_INVOKE_START). Always use structured kwargs:logger.info(EVENT, key=value). Log all error paths at WARNING or ERROR with context before raising, all state transitions at INFO, and DEBUG for object creation and key function flow. Pure data models, enums, and re-exports do not need logging.
Use immutability patterns: create new objects instead of mutating existing ones. For non-Pydantic internal collections (registries,BaseTool), usecopy.deepcopy()at construction plusMappingProxyTypewrapping for read-only enforcement. Fordict/listfields in frozen Pydantic models, usecopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization).
Use Pydantic v2 models (BaseModel,model_validator,computed_field,ConfigDict). Use@computed_fieldfor derived values instead of storing redundant fields (e.g.,TokenUsage.total_tokens). UseNotBlankStrfromcore.typesfor all identifier/name fields (including optionalNotBlankStr | Noneand tuple variants) instead of manual whitespace validators.
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations (e.g., multiple tool invocations, parallel agent calls) in new code instead of barecreate_task. Favor structured concurrency.
Separate frozen Pydantic models for config/identity from mutable-via-copy models for runtime state that evolves (e.g., agent execution state, task progress). Never mix static config fields with mutable runtime fields in one model.
Never useimport loggingorlogging.getLogger()in application code. Exception:observability/setup.pyandobservability/sinks.pymay use stdlibloggingandprint(..., file=sys.stderr)for bootst...
Files:
src/synthorg/api/controllers/setup_helpers.py
src/synthorg/api/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use RFC 9457 errors in the REST API. Implement role-based access control (RBAC) guards in the API layer.
Files:
src/synthorg/api/controllers/setup_helpers.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Line length: 88 characters (ruff enforces).
Files:
src/synthorg/api/controllers/setup_helpers.py
🧠 Learnings (12)
📚 Learning: 2026-03-28T18:10:43.542Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T18:10:43.542Z
Learning: Applies to **/*.{ts,tsx} : Always reuse existing components from `web/src/components/ui/` before creating new ones. Do not recreate status dots inline, build card-with-header layouts from scratch, create metric displays manually, render initials circles manually, create complex (>8 line) JSX inside `.map()` blocks, or use `rgba()` with hardcoded values instead of design token variables.
Applied to files:
web/src/pages/setup/MiniOrgChart.tsx
📚 Learning: 2026-03-28T18:10:43.543Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T18:10:43.543Z
Learning: Applies to web/src/**/*.{ts,tsx} : Use design token rules: colors via Tailwind semantic classes (`text-foreground`, `bg-card`, `text-accent`, `text-success`, `bg-danger`) or CSS variables (`var(--so-accent)`), typography via `font-sans` or `font-mono`, spacing via density-aware tokens (`p-card`, `gap-section-gap`, `gap-grid-gap`) or standard Tailwind, shadows/borders via token variables (`var(--so-shadow-card-hover)`, `border-border`, `border-bright`). Never hardcode hex values, set fontFamily directly, or hardcode pixel values for layout spacing.
Applied to files:
web/src/pages/setup/MiniOrgChart.tsx
📚 Learning: 2026-03-28T18:10:43.543Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T18:10:43.543Z
Learning: Applies to web/src/components/ui/*.{ts,tsx} : New shared components in `web/src/components/ui/` must have a corresponding `.stories.tsx` file with all states (default, hover, loading, error, empty), exported props as a TypeScript interface, design tokens exclusively (no hardcoded colors, fonts, or spacing), and use `cn()` from `@/lib/utils` for conditional class merging.
Applied to files:
web/src/pages/setup/MiniOrgChart.tsx
📚 Learning: 2026-03-21T11:08:01.542Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-21T11:08:01.542Z
Learning: Applies to **/*.{py,md} : Line length: 88 characters (ruff).
Applied to files:
web/src/stores/setup-wizard.ts
📚 Learning: 2026-03-28T18:10:43.543Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T18:10:43.543Z
Learning: Applies to src/**/*.py : Line length: 88 characters (ruff enforces).
Applied to files:
web/src/stores/setup-wizard.ts
📚 Learning: 2026-03-15T21:49:53.264Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:49:53.264Z
Learning: Fix everything valid — never skip when review agents find valid issues (including pre-existing issues in surrounding code, suggestions, and findings adjacent to the PR's changes). No deferring, no 'out of scope' skipping.
Applied to files:
web/src/stores/setup-wizard.ts
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to src/synthorg/templates/**/*.py : Templates: pre-built company templates, personality presets, and builder.
Applied to files:
web/src/stores/setup-wizard.ts
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/api/**/*.py : API package (api/): Litestar REST + WebSocket with controllers, guards, channels, JWT + API key + WS ticket auth, approval gate integration, coordination endpoint, collaboration endpoint, settings endpoint, provider management endpoint (CRUD + test + presets), backup endpoint, RFC 9457 structured errors, AppState hot-reload slots, service auto-wiring (Phase 1 at construction, Phase 2 on startup), lifecycle helpers
Applied to files:
src/synthorg/api/controllers/setup_helpers.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : Package structure: src/synthorg/ organized as: api/ (REST+WebSocket, Litestar), auth/ (auth subpackage), backup/ (scheduled/manual backups), budget/ (cost tracking, CFO), cli/ (superseded by Go CLI), communication/ (message bus, meetings), config/ (YAML loading), core/ (domain models, resilience config), engine/ (orchestration, task state, coordination, approval gates, stagnation detection, context budget, compaction), hr/ (hiring, performance, promotion), memory/ (pluggable backend, Mem0, retrieval, consolidation), persistence/ (operational data, SQLite, settings), observability/ (logging, correlation, sinks), providers/ (LLM abstraction, LiteLLM, auth types, presets, runtime CRUD), settings/ (runtime-editable, typed definitions, encryption, config bridge), security/ (SecOps, rule engine, output scanning, progressive trust, autonomy levels), templates/ (company templates, personalities), tools/ (registry, built-in tools, git, sandbox, code_runner, MCP...
Applied to files:
src/synthorg/api/controllers/setup_helpers.py
📚 Learning: 2026-03-26T15:18:16.848Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T15:18:16.848Z
Learning: Applies to src/synthorg/api/**/*.py : Litestar API must include setup wizard, auth/, auto-wiring, and lifecycle management
Applied to files:
src/synthorg/api/controllers/setup_helpers.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Handle errors explicitly, never silently swallow. Validate at system boundaries (user input, external APIs, config files).
Applied to files:
src/synthorg/api/controllers/setup_helpers.py
📚 Learning: 2026-03-16T20:14:00.937Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T20:14:00.937Z
Learning: Applies to **/*.py : Validate: at system boundaries (user input, external APIs, config files).
Applied to files:
src/synthorg/api/controllers/setup_helpers.py
🔇 Additional comments (12)
web/src/stores/setup-wizard.ts (7)
38-61: Well-structured helper extraction.The
runProbeAllhelper cleanly consolidates thePromise.allSettledpattern, handles per-preset failure logging, and is correctly placed after all imports. This addresses the prior review feedback effectively.
97-109: Step order logic is clear and correct.The
getStepOrderfunction cleanly mapsneedsAdmin×modecombinations to the appropriate step sequences. The four constant arrays provide good documentation of each flow.
302-311: LGTM!The guard at line 305 correctly prevents navigation to steps outside the current flow, returning early without state changes. This pairs well with
canNavigateTofor UI-level gating.
350-373: Template state cleanup is correctly implemented.When switching to
quickmode, the action properly clearsselectedTemplate,comparedTemplates,templateVariables, and resets the relevantstepsCompletedflags. This prevents stale template data from being sent during Quick Setup completion.
510-532: Personality preset actions follow established store patterns.Both
updateAgentPersonalityandfetchPersonalityPresetscorrectly mirror the existing agent action patterns with proper error handling and state updates.
615-627: Probe methods correctly refactored to use shared helper.Both
probeAllPresetsandreprobePresetsnow delegate torunProbeAll, eliminating the duplicatedPromise.allSettledpattern. ThereprobePresetsvariant correctly clears stale results before starting the new probe cycle.
247-255: Initial state is correctly configured for the guided flow.The defaults are consistent:
currentStep: 'mode',stepOrder: GUIDED_STEP_ORDER, andwizardMode: 'guided'. WhensetNeedsAdmin(true)is called, the step correctly shifts to'account'with the appropriate step order.web/src/pages/setup/MiniOrgChart.tsx (5)
20-26: Hardcoded pixel constants remain unaddressed.The layout constants (
LARGE_AVATAR_RADIUS,SMALL_AVATAR_RADIUS,LARGE_NODE_WIDTH,SMALL_NODE_WIDTH,AGENT_SPACING_GAP) are still raw pixel values. As per coding guidelines, these should use design tokens or centralized layout constants to stay in sync with the rest of the setup UI.
176-185: Height scaling still usesmaxAgentsInDeptrather than total team size.The
svgHeightcalculation on line 182 derives frommaxAgentsInDept, which only reflects the largest single department. This was previously flagged: a small org concentrated in one department renders taller than a larger org spread across departments, which contradicts the "scale by team size" intent.
1-13: LGTM!Clean imports and well-typed interfaces.
32-39: LGTM!Clean helper function with proper edge case handling.
86-143: Good component extraction.The
DepartmentGroupcomponent cleanly encapsulates department rendering logic. The truncation now uses named constants (DEPT_LABEL_MAX_DISPLAY,DEPT_LABEL_TRUNCATE_AT), and the eslint-disable comment provides valid justification for using index as key tiebreaker.Consider also extracting the department font sizes (lines 113:
11/9) alongside the initials font sizes for consistency.
| try: | ||
| entry = await settings_svc.get_entry( | ||
| "api", | ||
| "setup_complete", | ||
| ) | ||
| except MemoryError, RecursionError: | ||
| raise | ||
| except Exception: | ||
| logger.warning( | ||
| SETUP_STATUS_SETTINGS_UNAVAILABLE, | ||
| exc_info=True, | ||
| ) | ||
| return True | ||
| else: |
There was a problem hiding this comment.
Treat a missing setup_complete entry as an expected state.
is_setup_complete() already maps SettingNotFoundError to "not complete". Here the same condition falls into the warning path, so fresh or partially initialized installs will emit SETUP_STATUS_SETTINGS_UNAVAILABLE even though setup is simply still pending.
🛠️ Suggested fix
except MemoryError, RecursionError:
raise
+ except SettingNotFoundError:
+ return True
except Exception:
logger.warning(
SETUP_STATUS_SETTINGS_UNAVAILABLE,
exc_info=True,
)
return True🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/api/controllers/setup_helpers.py` around lines 157 - 170, The
current try/except around settings_svc.get_entry treats a missing
"setup_complete" entry as an unexpected error and logs
SETUP_STATUS_SETTINGS_UNAVAILABLE; change it to explicitly handle the
SettingNotFoundError (the same case handled by is_setup_complete()) by returning
False (or the "not complete" equivalent) instead of warning, while keeping the
existing except Exception block for real errors; update the except clauses
around settings_svc.get_entry to catch SettingNotFoundError (and re-raise
MemoryError/RecursionError as before), and only log
SETUP_STATUS_SETTINGS_UNAVAILABLE for other exceptions.
- check_needs_setup: handle SettingNotFoundError explicitly (no warning) - check_has_name_locales: delegate to shared parse_locale_json - MiniOrgChart: extract ROOT_RADIUS, FONT_SIZE_LARGE/SMALL constants - Store: fix misleading "re-probe button" error message Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/synthorg/api/controllers/setup_helpers.py (2)
543-564:⚠️ Potential issue | 🟠 MajorMake
persist_company_settings()atomic.These three awaits commit independently. If a call fails after Line 550,
company_nameis already stored andcheck_has_company()will treat the company step as complete on a half-written record. Please batch these writes behind a transactional settings API, or add explicit rollback handling here, and log the failure context before re-raising.As per coding guidelines "Log all error paths at WARNING or ERROR with context before raising".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/api/controllers/setup_helpers.py` around lines 543 - 564, persist_company_settings currently performs three independent awaits on SettingsService.set so a partial failure can leave a half-written company record; make these writes atomic by using a transactional API on SettingsService (e.g., begin_transaction/commit/rollback) or, if that API doesn't exist, perform all three writes in a try block and on exception log an ERROR with context (include company_name and departments_json) then rollback previous writes by reverting keys ("company_name","description","departments") to their prior values or empty defaults before re-raising; reference persist_company_settings, SettingsService, and its set method when locating and implementing the change.
416-437:⚠️ Potential issue | 🟠 MajorReject invalid list items in
parse_locale_json().Line 437 currently accepts any JSON list, so corrupted values like
[1, ""]makecheck_has_name_locales()report success and can still flow intoresolve_locales()viaread_name_locales(). Reject non-string or blank entries before returningparsed.🛠️ Proposed fix
def parse_locale_json(raw: str) -> list[str] | None: @@ if not isinstance(parsed, list): logger.warning( SETUP_NAME_LOCALES_CORRUPTED, reason="expected_list", actual_type=type(parsed).__name__, ) return None + if not all(isinstance(locale, str) and locale.strip() for locale in parsed): + logger.warning( + SETUP_NAME_LOCALES_CORRUPTED, + reason="invalid_list_items", + ) + return None return parsedAs per coding guidelines "Validate at system boundaries (user input, external APIs, config files)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/api/controllers/setup_helpers.py` around lines 416 - 437, parse_locale_json currently accepts any JSON list and returns it even if items are non-strings or blank; update parse_locale_json to validate that each item in the parsed list is a non-empty string and reject the whole payload (return None) if any item is not a string or is an empty/whitespace-only string, logging via SETUP_NAME_LOCALES_CORRUPTED with a reason like "invalid_list_items" and include a short sample/actual value via logger.warning; this ensures downstream callers (check_has_name_locales, read_name_locales, resolve_locales) only see well-formed locale lists.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/pages/setup/MiniOrgChart.tsx`:
- Line 118: Extract the hardcoded department label font sizes into the same
constants used for agent initials (e.g., FONT_SIZE_LARGE and FONT_SIZE_SMALL)
and replace the inline fontSize={isSmallTeam ? 11 : 9} with
fontSize={isSmallTeam ? FONT_SIZE_SMALL : FONT_SIZE_LARGE} (or the reverse
mapping that matches your sizing convention) in MiniOrgChart.tsx; add the new
constants near the existing FONT_SIZE_LARGE / FONT_SIZE_SMALL declarations so
both agent initials and department labels use the shared constants for
consistency.
In `@web/src/stores/setup-wizard.ts`:
- Around line 350-372: setWizardMode currently swaps stepOrder but leaves
currentStep unchanged, which can leave currentStep pointing at a step that
doesn't exist in the new stepOrder (e.g., switching to 'quick'); update
setWizardMode to compute the new stepOrder via getStepOrder(needsAdmin, mode)
and then ensure currentStep is set to a valid step from that new stepOrder
(e.g., if s.currentStep is not included in stepOrder, set currentStep to
stepOrder[0]); keep the existing quick-mode clearing logic for
selectedTemplate/comparedTemplates/templateVariables and update stepsCompleted
as before, but ensure you reference setWizardMode, currentStep, stepOrder,
getStepOrder and stepsCompleted when making the change.
- Around line 573-596: The code only treats exceptions from discoverModels() or
getProvider() as failures, but if both succeed and refreshed.models remains
empty we must still mark this as an error; after calling getProvider(name) check
refreshed.models (or refreshed.models.length) and if zero set the same
providersError state and do not silently accept the provider. Update the branch
that currently sets providers to instead validate refreshed.models > 0 before
calling set((s) => ({ providers: { ...s.providers, [name]: refreshed } })), and
if empty set providersError with a message referencing name/presetName
explaining model discovery returned no models.
---
Duplicate comments:
In `@src/synthorg/api/controllers/setup_helpers.py`:
- Around line 543-564: persist_company_settings currently performs three
independent awaits on SettingsService.set so a partial failure can leave a
half-written company record; make these writes atomic by using a transactional
API on SettingsService (e.g., begin_transaction/commit/rollback) or, if that API
doesn't exist, perform all three writes in a try block and on exception log an
ERROR with context (include company_name and departments_json) then rollback
previous writes by reverting keys ("company_name","description","departments")
to their prior values or empty defaults before re-raising; reference
persist_company_settings, SettingsService, and its set method when locating and
implementing the change.
- Around line 416-437: parse_locale_json currently accepts any JSON list and
returns it even if items are non-strings or blank; update parse_locale_json to
validate that each item in the parsed list is a non-empty string and reject the
whole payload (return None) if any item is not a string or is an
empty/whitespace-only string, logging via SETUP_NAME_LOCALES_CORRUPTED with a
reason like "invalid_list_items" and include a short sample/actual value via
logger.warning; this ensures downstream callers (check_has_name_locales,
read_name_locales, resolve_locales) only see well-formed locale lists.
🪄 Autofix (Beta)
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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d7e2f1db-1025-4f39-85b2-e17d1b9a78f3
📒 Files selected for processing (3)
src/synthorg/api/controllers/setup_helpers.pyweb/src/pages/setup/MiniOrgChart.tsxweb/src/stores/setup-wizard.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Dashboard Test
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Sandbox
- GitHub Check: Build Backend
- GitHub Check: Build Web
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always reuse existing components from
web/src/components/ui/before creating new ones. Do not recreate status dots inline, build card-with-header layouts from scratch, create metric displays manually, render initials circles manually, create complex (>8 line) JSX inside.map()blocks, or usergba()with hardcoded values instead of design token variables.
Files:
web/src/pages/setup/MiniOrgChart.tsxweb/src/stores/setup-wizard.ts
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use design token rules: colors via Tailwind semantic classes (
text-foreground,bg-card,text-accent,text-success,bg-danger) or CSS variables (var(--so-accent)), typography viafont-sansorfont-mono, spacing via density-aware tokens (p-card,gap-section-gap,gap-grid-gap) or standard Tailwind, shadows/borders via token variables (var(--so-shadow-card-hover),border-border,border-bright). Never hardcode hex values, set fontFamily directly, or hardcode pixel values for layout spacing.
Files:
web/src/pages/setup/MiniOrgChart.tsxweb/src/stores/setup-wizard.ts
web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use property-based testing with fast-check (
fc.assert+fc.property). Run tests withnpm --prefix web run testfor Vitest with--detect-async-leaksflag.
Files:
web/src/pages/setup/MiniOrgChart.tsxweb/src/stores/setup-wizard.ts
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations-- Python 3.14+ has PEP 649 native lazy annotations. Useexcept A, B:(no parentheses) per PEP 758 except syntax as enforced by ruff.
All public functions and classes require type hints and Google-style docstrings (enforced by ruff D rules). Use mypy strict mode.
Functions must be less than 50 lines, files less than 800 lines. Handle errors explicitly, never silently swallow. Validate at system boundaries (user input, external APIs, config files).
Files:
src/synthorg/api/controllers/setup_helpers.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic must have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__). Use event name constants fromsynthorg.observability.events.<domain>modules (e.g.,API_REQUEST_STARTED,TOOL_INVOKE_START). Always use structured kwargs:logger.info(EVENT, key=value). Log all error paths at WARNING or ERROR with context before raising, all state transitions at INFO, and DEBUG for object creation and key function flow. Pure data models, enums, and re-exports do not need logging.
Use immutability patterns: create new objects instead of mutating existing ones. For non-Pydantic internal collections (registries,BaseTool), usecopy.deepcopy()at construction plusMappingProxyTypewrapping for read-only enforcement. Fordict/listfields in frozen Pydantic models, usecopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization).
Use Pydantic v2 models (BaseModel,model_validator,computed_field,ConfigDict). Use@computed_fieldfor derived values instead of storing redundant fields (e.g.,TokenUsage.total_tokens). UseNotBlankStrfromcore.typesfor all identifier/name fields (including optionalNotBlankStr | Noneand tuple variants) instead of manual whitespace validators.
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations (e.g., multiple tool invocations, parallel agent calls) in new code instead of barecreate_task. Favor structured concurrency.
Separate frozen Pydantic models for config/identity from mutable-via-copy models for runtime state that evolves (e.g., agent execution state, task progress). Never mix static config fields with mutable runtime fields in one model.
Never useimport loggingorlogging.getLogger()in application code. Exception:observability/setup.pyandobservability/sinks.pymay use stdlibloggingandprint(..., file=sys.stderr)for bootst...
Files:
src/synthorg/api/controllers/setup_helpers.py
src/synthorg/api/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use RFC 9457 errors in the REST API. Implement role-based access control (RBAC) guards in the API layer.
Files:
src/synthorg/api/controllers/setup_helpers.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Line length: 88 characters (ruff enforces).
Files:
src/synthorg/api/controllers/setup_helpers.py
🧠 Learnings (10)
📚 Learning: 2026-03-28T18:10:43.542Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T18:10:43.542Z
Learning: Applies to **/*.{ts,tsx} : Always reuse existing components from `web/src/components/ui/` before creating new ones. Do not recreate status dots inline, build card-with-header layouts from scratch, create metric displays manually, render initials circles manually, create complex (>8 line) JSX inside `.map()` blocks, or use `rgba()` with hardcoded values instead of design token variables.
Applied to files:
web/src/pages/setup/MiniOrgChart.tsx
📚 Learning: 2026-03-28T18:10:43.543Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T18:10:43.543Z
Learning: Applies to web/src/**/*.{ts,tsx} : Use design token rules: colors via Tailwind semantic classes (`text-foreground`, `bg-card`, `text-accent`, `text-success`, `bg-danger`) or CSS variables (`var(--so-accent)`), typography via `font-sans` or `font-mono`, spacing via density-aware tokens (`p-card`, `gap-section-gap`, `gap-grid-gap`) or standard Tailwind, shadows/borders via token variables (`var(--so-shadow-card-hover)`, `border-border`, `border-bright`). Never hardcode hex values, set fontFamily directly, or hardcode pixel values for layout spacing.
Applied to files:
web/src/pages/setup/MiniOrgChart.tsx
📚 Learning: 2026-03-28T18:10:43.543Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T18:10:43.543Z
Learning: Applies to web/src/components/ui/*.{ts,tsx} : New shared components in `web/src/components/ui/` must have a corresponding `.stories.tsx` file with all states (default, hover, loading, error, empty), exported props as a TypeScript interface, design tokens exclusively (no hardcoded colors, fonts, or spacing), and use `cn()` from `@/lib/utils` for conditional class merging.
Applied to files:
web/src/pages/setup/MiniOrgChart.tsx
📚 Learning: 2026-03-21T11:08:01.542Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-21T11:08:01.542Z
Learning: Applies to **/*.{py,md} : Line length: 88 characters (ruff).
Applied to files:
web/src/stores/setup-wizard.ts
📚 Learning: 2026-03-28T18:10:43.543Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T18:10:43.543Z
Learning: Applies to src/**/*.py : Line length: 88 characters (ruff enforces).
Applied to files:
web/src/stores/setup-wizard.ts
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to src/synthorg/templates/**/*.py : Templates: pre-built company templates, personality presets, and builder.
Applied to files:
web/src/stores/setup-wizard.ts
📚 Learning: 2026-03-26T15:18:16.848Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T15:18:16.848Z
Learning: Applies to src/synthorg/api/**/*.py : Litestar API must include setup wizard, auth/, auto-wiring, and lifecycle management
Applied to files:
src/synthorg/api/controllers/setup_helpers.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/api/**/*.py : API package (api/): Litestar REST + WebSocket with controllers, guards, channels, JWT + API key + WS ticket auth, approval gate integration, coordination endpoint, collaboration endpoint, settings endpoint, provider management endpoint (CRUD + test + presets), backup endpoint, RFC 9457 structured errors, AppState hot-reload slots, service auto-wiring (Phase 1 at construction, Phase 2 on startup), lifecycle helpers
Applied to files:
src/synthorg/api/controllers/setup_helpers.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Handle errors explicitly, never silently swallow. Validate at system boundaries (user input, external APIs, config files).
Applied to files:
src/synthorg/api/controllers/setup_helpers.py
📚 Learning: 2026-03-16T20:14:00.937Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T20:14:00.937Z
Learning: Applies to **/*.py : Validate: at system boundaries (user input, external APIs, config files).
Applied to files:
src/synthorg/api/controllers/setup_helpers.py
🔇 Additional comments (6)
web/src/pages/setup/MiniOrgChart.tsx (4)
15-35: Good extraction of previously flagged constants.The extraction of
ROOT_RADIUS,FONT_SIZE_LARGE,FONT_SIZE_SMALL, and truncation constants addresses several previous review comments. The constants are well-documented with JSDoc comments.Note: The broader concern about hardcoded pixel values (vs. density-aware design tokens) was raised in a previous review and remains open.
,
181-190: Height calculation concern was previously raised.The use of
maxAgentsInDeptfor scalingsvgHeight(rather than total team size) was flagged in a previous review as potentially causing inconsistent scaling behavior. That concern remains open.
46-89: LGTM!The
AgentNodecomponent is well-structured with properly typed props. The use ofdeptHalfHeightinstead of a hardcoded offset addresses the previous "fragile coupling" concern, and font size selection correctly uses the extracted constants.
204-243: LGTM on the SVG rendering structure.The SVG layout is well-organized with proper semantic Tailwind classes (
fill-accent,stroke-border,bg-card,border-border), accessibility attributes (role="img",aria-label), and clean component composition viaDepartmentGroup.src/synthorg/api/controllers/setup_helpers.py (1)
406-413: Good reuse of the shared agent lock.Keeping the write on Lines 406-411 inside
AGENT_LOCKmatchessrc/synthorg/api/controllers/setup_personality.py:79-104, so template creation and personality edits cannot interleave on the sharedcompany/agentsblob.web/src/stores/setup-wizard.ts (1)
302-310: Good hardening for mode-specific navigation.Rejecting steps that are not in the active
stepOrdercloses the obvious quick-mode navigation hole cleanly.
- MiniOrgChart: extract dept label font size constants - setWizardMode: reset currentStep when not in new step order - Store: warn when model discovery succeeds but returns 0 models Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
web/src/stores/setup-wizard.ts (2)
350-379:⚠️ Potential issue | 🟠 MajorClear already-created company state when mode changes.
If guided setup has already called
submitCompany(), switching to quick leavescompanyResponse,agents, and thecompany/providerscompletion flags intact. That lets the user finish the quick flow with template-derived agents from the old guided run still applied. Either disallow mode changes after company creation or clear the created company/agent state and force the company step to run again.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/stores/setup-wizard.ts` around lines 350 - 379, The setWizardMode function must clear any previously-created company/agent state when switching modes so template-derived agents from a prior guided run aren't reused; update setWizardMode to, when mode changes (especially to 'quick'), reset companyResponse, agents, and the related stepsCompleted flags (at least company and providers) to force the company step to run again (or alternatively disallow mode changes if submitCompany() has completed); reference setWizardMode, submitCompany, companyResponse, agents, and stepsCompleted in the change so the new state reset logic is placed in the set((s) => { ... }) return object alongside selectedTemplate/comparedTemplates/templateVariables.
577-613:⚠️ Potential issue | 🟠 MajorSurface discovery failure as an actual action failure.
providersErroris set here, but the promise still resolves even when discovery failed or the refreshed provider still has no models. Any caller awaitingcreateProviderFromPreset()has no reliable success signal and can still advance the wizard as if provider setup completed. Throw after persisting the error, or return an explicit status that the caller must inspect.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/stores/setup-wizard.ts` around lines 577 - 613, In createProviderFromPreset, the code currently only sets providersError when model discovery fails or when refreshed.models.length === 0 but still resolves the promise; modify both places (the refreshed.models.length === 0 branch after getProvider(name) and the catch(discoveryErr) branch around discoverModels(name, presetName)) to, after calling set({ providersError: ... }), also reject the operation by throwing an Error (use the same error message you persisted) so callers awaiting createProviderFromPreset receive a failure; ensure you include the discoveryErr message when throwing in the catch branch and keep existing set(...) calls so the UI state is updated before the throw.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/pages/setup/MiniOrgChart.tsx`:
- Line 190: The svgHeight calculation uses a hard-coded 6 for agent vertical
spacing and an unexplained +24; replace the magic numbers by introducing a named
constant (e.g., AGENT_VERTICAL_GAP = 6) and a padding constant (e.g.,
SVG_BOTTOM_PADDING = 24) and use those in the formula for svgHeight (reference
symbols: svgHeight, AGENT_SPACING_GAP, nodeHeight, avatarRadius,
maxAgentsInDept) so vertical spacing is explicit and consistent with horizontal
spacing, and make the bottom padding intention clear.
In `@web/src/stores/setup-wizard.ts`:
- Around line 517-528: The updateAgentPersonality handler can apply out-of-order
API responses and revert the UI; modify updateAgentPersonality to ignore stale
responses by tracking a per-agent request id (e.g., add a requestId counter on
each agent or a separate map keyed by index) before calling
apiUpdateAgentPersonality, attach that id to the response handling, and only
call set(...) to replace the agent when the response id matches the latest for
that agent; alternatively implement cancellation by storing and aborting a
previous AbortController for the agent before issuing a new fetch. Ensure you
reference updateAgentPersonality, apiUpdateAgentPersonality and the agents array
when adding the per-agent request id or AbortController logic.
---
Duplicate comments:
In `@web/src/stores/setup-wizard.ts`:
- Around line 350-379: The setWizardMode function must clear any
previously-created company/agent state when switching modes so template-derived
agents from a prior guided run aren't reused; update setWizardMode to, when mode
changes (especially to 'quick'), reset companyResponse, agents, and the related
stepsCompleted flags (at least company and providers) to force the company step
to run again (or alternatively disallow mode changes if submitCompany() has
completed); reference setWizardMode, submitCompany, companyResponse, agents, and
stepsCompleted in the change so the new state reset logic is placed in the
set((s) => { ... }) return object alongside
selectedTemplate/comparedTemplates/templateVariables.
- Around line 577-613: In createProviderFromPreset, the code currently only sets
providersError when model discovery fails or when refreshed.models.length === 0
but still resolves the promise; modify both places (the refreshed.models.length
=== 0 branch after getProvider(name) and the catch(discoveryErr) branch around
discoverModels(name, presetName)) to, after calling set({ providersError: ...
}), also reject the operation by throwing an Error (use the same error message
you persisted) so callers awaiting createProviderFromPreset receive a failure;
ensure you include the discoveryErr message when throwing in the catch branch
and keep existing set(...) calls so the UI state is updated before the throw.
🪄 Autofix (Beta)
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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9cd49c09-c9ef-457e-8813-ac7c069fce6d
📒 Files selected for processing (2)
web/src/pages/setup/MiniOrgChart.tsxweb/src/stores/setup-wizard.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Build Web
- GitHub Check: Build Sandbox
- GitHub Check: Build Backend
- GitHub Check: Dashboard Test
- GitHub Check: Test (Python 3.14)
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always reuse existing components from
web/src/components/ui/before creating new ones. Do not recreate status dots inline, build card-with-header layouts from scratch, create metric displays manually, render initials circles manually, create complex (>8 line) JSX inside.map()blocks, or usergba()with hardcoded values instead of design token variables.
Files:
web/src/pages/setup/MiniOrgChart.tsxweb/src/stores/setup-wizard.ts
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use design token rules: colors via Tailwind semantic classes (
text-foreground,bg-card,text-accent,text-success,bg-danger) or CSS variables (var(--so-accent)), typography viafont-sansorfont-mono, spacing via density-aware tokens (p-card,gap-section-gap,gap-grid-gap) or standard Tailwind, shadows/borders via token variables (var(--so-shadow-card-hover),border-border,border-bright). Never hardcode hex values, set fontFamily directly, or hardcode pixel values for layout spacing.
Files:
web/src/pages/setup/MiniOrgChart.tsxweb/src/stores/setup-wizard.ts
web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use property-based testing with fast-check (
fc.assert+fc.property). Run tests withnpm --prefix web run testfor Vitest with--detect-async-leaksflag.
Files:
web/src/pages/setup/MiniOrgChart.tsxweb/src/stores/setup-wizard.ts
🧠 Learnings (7)
📚 Learning: 2026-03-28T18:10:43.542Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T18:10:43.542Z
Learning: Applies to **/*.{ts,tsx} : Always reuse existing components from `web/src/components/ui/` before creating new ones. Do not recreate status dots inline, build card-with-header layouts from scratch, create metric displays manually, render initials circles manually, create complex (>8 line) JSX inside `.map()` blocks, or use `rgba()` with hardcoded values instead of design token variables.
Applied to files:
web/src/pages/setup/MiniOrgChart.tsx
📚 Learning: 2026-03-15T21:20:09.993Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:20:09.993Z
Learning: Applies to web/src/components/** : Vue components organized by feature (agents/, approvals/, budget/, common/, dashboard/, layout/, messages/, org-chart/, tasks/).
Applied to files:
web/src/pages/setup/MiniOrgChart.tsx
📚 Learning: 2026-03-28T18:10:43.543Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T18:10:43.543Z
Learning: Applies to web/src/**/*.{ts,tsx} : Use design token rules: colors via Tailwind semantic classes (`text-foreground`, `bg-card`, `text-accent`, `text-success`, `bg-danger`) or CSS variables (`var(--so-accent)`), typography via `font-sans` or `font-mono`, spacing via density-aware tokens (`p-card`, `gap-section-gap`, `gap-grid-gap`) or standard Tailwind, shadows/borders via token variables (`var(--so-shadow-card-hover)`, `border-border`, `border-bright`). Never hardcode hex values, set fontFamily directly, or hardcode pixel values for layout spacing.
Applied to files:
web/src/pages/setup/MiniOrgChart.tsx
📚 Learning: 2026-03-28T18:10:43.543Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T18:10:43.543Z
Learning: Applies to web/src/components/ui/*.{ts,tsx} : New shared components in `web/src/components/ui/` must have a corresponding `.stories.tsx` file with all states (default, hover, loading, error, empty), exported props as a TypeScript interface, design tokens exclusively (no hardcoded colors, fonts, or spacing), and use `cn()` from `@/lib/utils` for conditional class merging.
Applied to files:
web/src/pages/setup/MiniOrgChart.tsx
📚 Learning: 2026-03-21T11:08:01.542Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-21T11:08:01.542Z
Learning: Applies to **/*.{py,md} : Line length: 88 characters (ruff).
Applied to files:
web/src/stores/setup-wizard.ts
📚 Learning: 2026-03-28T18:10:43.543Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T18:10:43.543Z
Learning: Applies to src/**/*.py : Line length: 88 characters (ruff enforces).
Applied to files:
web/src/stores/setup-wizard.ts
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to src/synthorg/templates/**/*.py : Templates: pre-built company templates, personality presets, and builder.
Applied to files:
web/src/stores/setup-wizard.ts
🔇 Additional comments (5)
web/src/pages/setup/MiniOrgChart.tsx (4)
15-38: Well-structured constants with proper documentation.The extracted constants with JSDoc comments address previous feedback. The naming convention is clear and consistent (e.g.,
LARGE_*vsSMALL_*prefixes,DEPT_FONT_SIZE_*for department-specific sizing).
49-92: AgentNode improvements look good.The component is now properly parameterized with
radiusanddeptHalfHeight, eliminating the hardcoded connector offset. The font size selection uses the extracted constants correctly.
94-151: Good component extraction for DepartmentGroup.The refactoring into a separate component improves readability. The
.map()inside correctly delegates to theAgentNodecomponent rather than inlining complex JSX. The ESLint disable comment on line 137 appropriately documents why array index is needed as a key tiebreaker.
207-243: SVG rendering correctly uses extracted constants.The root node uses
ROOT_RADIUSconsistently for both the circle radius and connector offset. Semantic Tailwind classes (fill-accent,stroke-border,border-border,bg-card) are used throughout.web/src/stores/setup-wizard.ts (1)
302-311: Good guard on flow-specific navigation.Ignoring steps that are absent from the active
stepOrdercleanly prevents invalid quick-mode transitions.
| async updateAgentPersonality(index, preset) { | ||
| set({ agentsError: null }) | ||
| try { | ||
| const updated = await apiUpdateAgentPersonality(index, { personality_preset: preset }) | ||
| set((s) => ({ | ||
| agents: s.agents.map((a, i) => i === index ? updated : a), | ||
| })) | ||
| } catch (err) { | ||
| console.error('setup-wizard: updateAgentPersonality failed:', err) | ||
| set({ agentsError: getErrorMessage(err) }) | ||
| } | ||
| }, |
There was a problem hiding this comment.
Ignore stale personality-update responses.
This applies whichever response arrives last. With an editable preset dropdown, overlapping requests can return out of order and revert the UI to an older choice. Gate the state update behind a per-agent request id, or cancel the previous request before sending a new one.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/stores/setup-wizard.ts` around lines 517 - 528, The
updateAgentPersonality handler can apply out-of-order API responses and revert
the UI; modify updateAgentPersonality to ignore stale responses by tracking a
per-agent request id (e.g., add a requestId counter on each agent or a separate
map keyed by index) before calling apiUpdateAgentPersonality, attach that id to
the response handling, and only call set(...) to replace the agent when the
response id matches the latest for that agent; alternatively implement
cancellation by storing and aborting a previous AbortController for the agent
before issuing a new fetch. Ensure you reference updateAgentPersonality,
apiUpdateAgentPersonality and the agents array when adding the per-agent request
id or AbortController logic.
AGENT_VERTICAL_GAP (6) and SVG_BOTTOM_PADDING (24) replace the last two inline magic numbers in the MiniOrgChart layout formula. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/pages/setup/MiniOrgChart.tsx (1)
75-90: 🛠️ Refactor suggestion | 🟠 MajorPromote the initials avatar to shared UI instead of keeping it in-page.
AgentNodeis still drawing the avatar circle and initials directly. Please reuse the shared avatar/initials primitive fromweb/src/components/ui/, or move this SVG-capable version there and consume it here, so setup keeps one source of truth for initials rendering.Based on learnings, "Always reuse existing components from
web/src/components/ui/before creating new ones. Do not recreate status dots inline, build card-with-header layouts from scratch, create metric displays manually, render initials circles manually, create complex (>8 line) JSX inside.map()blocks, or usergba()with hardcoded values instead of design token variables."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/setup/MiniOrgChart.tsx` around lines 75 - 90, AgentNode in MiniOrgChart currently renders the avatar circle and initials inline; replace that with the shared SVG-capable avatar/initials primitive from web/src/components/ui/ (or move an SVG-capable variant into that directory and import it) so there is a single source of truth. Locate the JSX in MiniOrgChart/AgentNode that draws the <circle> and <text> for initials (using agentX, agentY, radius, FONT_SIZE_*), remove that inline SVG rendering, import and render the shared Avatar/Initials component instead, passing props for size, initials (agent.name), role/status if supported, and any theme/className; if the shared component lacks SVG support, refactor it to accept coordinates/size or expose a prop to render as SVG and then update AgentNode to consume it. Ensure you remove duplicate styling (fill-card, stroke-accent) and use design-token props from the shared UI component rather than inline styles.
♻️ Duplicate comments (2)
web/src/pages/setup/MiniOrgChart.tsx (2)
20-42: 🛠️ Refactor suggestion | 🟠 MajorNice extraction, but the layout values are still outside the design system.
This refactor centralizes the numbers, but the chart still hardcodes its spacing and sizing in TSX (
LARGE_AVATAR_RADIUS,LARGE_NODE_WIDTH,hGap,vGap,svgWidth + 40,rootY = 16). That means setup density changes still will not propagate here. Please source these measurements from shared spacing/design tokens or a shared chart-layout config instead of keeping them file-local.As per coding guidelines, "Never hardcode hex values, set fontFamily directly, or hardcode pixel values for layout spacing."
Also applies to: 183-185, 193-194, 209-209
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/setup/MiniOrgChart.tsx` around lines 20 - 42, The layout hardcodes (LARGE_AVATAR_RADIUS, SMALL_AVATAR_RADIUS, LARGE_NODE_WIDTH, SMALL_NODE_WIDTH, AGENT_SPACING_GAP, AGENT_VERTICAL_GAP, SVG_BOTTOM_PADDING, plus local variables hGap, vGap, svgWidth + 40, rootY) and must be replaced with shared design tokens or a centralized chart-layout config; update MiniOrgChart to import and use the shared spacing/font/size tokens (or a chartLayout object) instead of file-local constants, map the existing constants to the corresponding token names, replace usages of hGap/vGap/svgWidth/rootY with values from that config, and remove any remaining hardcoded pixel values so spacing will follow the global design system.
133-149:⚠️ Potential issue | 🟠 MajorScale the height from actual rendered rows or total team size.
Line 137 still places every agent on one row, so Line 194's
maxAgentsInDeptterm only adds blank vertical space. That makes one dense department render taller than a larger org split across departments, and it forces unnecessary downscaling at the 500px cap. Drive the scaling input fromagents.length(or actual rendered rows if wrapping is added later) and keepsvgHeighttied to the content bounds.Also applies to: 188-197
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/setup/MiniOrgChart.tsx` around lines 133 - 149, The department layout currently uses maxAgentsInDept to add blank vertical space and scale down content; instead compute scaling from the actual number of agents/rows for each department (use pos.dept.agents.length or a derived rowsCount = Math.ceil(pos.dept.agents.length / agentsPerRow) if you add wrapping) and feed that into the height/scale math (replace uses of maxAgentsInDept in the svgHeight/scale calculation). Update the code that positions AgentNode (where agentX/agentY, avatarRadius, vGap, nodeHeight are used) so agentY spacing is based on rowsCount * (nodeHeight + vGap) and ensure svgHeight is tied to the computed content bounds (total rows across departments) rather than the maximum department size to avoid extra blank space and over-downscaling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@web/src/pages/setup/MiniOrgChart.tsx`:
- Around line 75-90: AgentNode in MiniOrgChart currently renders the avatar
circle and initials inline; replace that with the shared SVG-capable
avatar/initials primitive from web/src/components/ui/ (or move an SVG-capable
variant into that directory and import it) so there is a single source of truth.
Locate the JSX in MiniOrgChart/AgentNode that draws the <circle> and <text> for
initials (using agentX, agentY, radius, FONT_SIZE_*), remove that inline SVG
rendering, import and render the shared Avatar/Initials component instead,
passing props for size, initials (agent.name), role/status if supported, and any
theme/className; if the shared component lacks SVG support, refactor it to
accept coordinates/size or expose a prop to render as SVG and then update
AgentNode to consume it. Ensure you remove duplicate styling (fill-card,
stroke-accent) and use design-token props from the shared UI component rather
than inline styles.
---
Duplicate comments:
In `@web/src/pages/setup/MiniOrgChart.tsx`:
- Around line 20-42: The layout hardcodes (LARGE_AVATAR_RADIUS,
SMALL_AVATAR_RADIUS, LARGE_NODE_WIDTH, SMALL_NODE_WIDTH, AGENT_SPACING_GAP,
AGENT_VERTICAL_GAP, SVG_BOTTOM_PADDING, plus local variables hGap, vGap,
svgWidth + 40, rootY) and must be replaced with shared design tokens or a
centralized chart-layout config; update MiniOrgChart to import and use the
shared spacing/font/size tokens (or a chartLayout object) instead of file-local
constants, map the existing constants to the corresponding token names, replace
usages of hGap/vGap/svgWidth/rootY with values from that config, and remove any
remaining hardcoded pixel values so spacing will follow the global design
system.
- Around line 133-149: The department layout currently uses maxAgentsInDept to
add blank vertical space and scale down content; instead compute scaling from
the actual number of agents/rows for each department (use pos.dept.agents.length
or a derived rowsCount = Math.ceil(pos.dept.agents.length / agentsPerRow) if you
add wrapping) and feed that into the height/scale math (replace uses of
maxAgentsInDept in the svgHeight/scale calculation). Update the code that
positions AgentNode (where agentX/agentY, avatarRadius, vGap, nodeHeight are
used) so agentY spacing is based on rowsCount * (nodeHeight + vGap) and ensure
svgHeight is tied to the computed content bounds (total rows across departments)
rather than the maximum department size to avoid extra blank space and
over-downscaling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1b05b9af-6509-4249-8adc-c78d6db1d1d0
📒 Files selected for processing (1)
web/src/pages/setup/MiniOrgChart.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Dashboard Test
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Web
- GitHub Check: Build Sandbox
- GitHub Check: Build Backend
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always reuse existing components from
web/src/components/ui/before creating new ones. Do not recreate status dots inline, build card-with-header layouts from scratch, create metric displays manually, render initials circles manually, create complex (>8 line) JSX inside.map()blocks, or usergba()with hardcoded values instead of design token variables.
Files:
web/src/pages/setup/MiniOrgChart.tsx
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use design token rules: colors via Tailwind semantic classes (
text-foreground,bg-card,text-accent,text-success,bg-danger) or CSS variables (var(--so-accent)), typography viafont-sansorfont-mono, spacing via density-aware tokens (p-card,gap-section-gap,gap-grid-gap) or standard Tailwind, shadows/borders via token variables (var(--so-shadow-card-hover),border-border,border-bright). Never hardcode hex values, set fontFamily directly, or hardcode pixel values for layout spacing.
Files:
web/src/pages/setup/MiniOrgChart.tsx
web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use property-based testing with fast-check (
fc.assert+fc.property). Run tests withnpm --prefix web run testfor Vitest with--detect-async-leaksflag.
Files:
web/src/pages/setup/MiniOrgChart.tsx
🧠 Learnings (4)
📚 Learning: 2026-03-28T18:10:43.542Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T18:10:43.542Z
Learning: Applies to **/*.{ts,tsx} : Always reuse existing components from `web/src/components/ui/` before creating new ones. Do not recreate status dots inline, build card-with-header layouts from scratch, create metric displays manually, render initials circles manually, create complex (>8 line) JSX inside `.map()` blocks, or use `rgba()` with hardcoded values instead of design token variables.
Applied to files:
web/src/pages/setup/MiniOrgChart.tsx
📚 Learning: 2026-03-15T21:20:09.993Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:20:09.993Z
Learning: Applies to web/src/components/** : Vue components organized by feature (agents/, approvals/, budget/, common/, dashboard/, layout/, messages/, org-chart/, tasks/).
Applied to files:
web/src/pages/setup/MiniOrgChart.tsx
📚 Learning: 2026-03-28T18:10:43.543Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T18:10:43.543Z
Learning: Applies to web/src/**/*.{ts,tsx} : Use design token rules: colors via Tailwind semantic classes (`text-foreground`, `bg-card`, `text-accent`, `text-success`, `bg-danger`) or CSS variables (`var(--so-accent)`), typography via `font-sans` or `font-mono`, spacing via density-aware tokens (`p-card`, `gap-section-gap`, `gap-grid-gap`) or standard Tailwind, shadows/borders via token variables (`var(--so-shadow-card-hover)`, `border-border`, `border-bright`). Never hardcode hex values, set fontFamily directly, or hardcode pixel values for layout spacing.
Applied to files:
web/src/pages/setup/MiniOrgChart.tsx
📚 Learning: 2026-03-28T18:10:43.543Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T18:10:43.543Z
Learning: Applies to web/src/components/ui/*.{ts,tsx} : New shared components in `web/src/components/ui/` must have a corresponding `.stories.tsx` file with all states (default, hover, loading, error, empty), exported props as a TypeScript interface, design tokens exclusively (no hardcoded colors, fonts, or spacing), and use `cn()` from `@/lib/utils` for conditional class merging.
Applied to files:
web/src/pages/setup/MiniOrgChart.tsx
🤖 I have created a release *beep* *boop* --- #MAJOR CHANGES; We got a somewhat working webui :) ## [0.5.0](v0.4.9...v0.5.0) (2026-03-30) ### Features * add analytics trends and budget forecast API endpoints ([#798](#798)) ([16b61f5](16b61f5)) * add department policies to default templates ([#852](#852)) ([7a41548](7a41548)) * add remaining activity event types (task_started, tool_used, delegation, cost_incurred) ([#832](#832)) ([4252fac](4252fac)) * agent performance, activity, and history API endpoints ([#811](#811)) ([9b75c1d](9b75c1d)) * Agent Profiles and Detail pages (biography, career, performance) ([#874](#874)) ([62d7880](62d7880)) * app shell, Storybook, and CI/CD pipeline ([#819](#819)) ([d4dde90](d4dde90)) * Approvals page with risk grouping, urgency indicators, batch actions ([#889](#889)) ([4e9673d](4e9673d)) * Budget Panel page (P&L dashboard, breakdown charts, forecast) ([#890](#890)) ([b63b0f1](b63b0f1)) * build infrastructure layer (API client, auth, WebSocket) ([#815](#815)) ([9f01d3e](9f01d3e)) * CLI global options infrastructure, UI modes, exit codes, env vars ([#891](#891)) ([fef4fc5](fef4fc5)) * CodeMirror editor and theme preferences toggle ([#905](#905), [#807](#807)) ([#909](#909)) ([41fbedc](41fbedc)) * Company page (department/agent management) ([#888](#888)) ([cfb88b0](cfb88b0)) * comprehensive hint coverage across all CLI commands ([#900](#900)) ([937974e](937974e)) * config system extensions, per-command flags for init/start/stop/status/logs ([#895](#895)) ([32f83fe](32f83fe)) * configurable currency system replacing hardcoded USD ([#854](#854)) ([b372551](b372551)) * Dashboard page (metric cards, activity feed, budget burn) ([#861](#861)) ([7d519d5](7d519d5)) * department health, provider status, and activity feed endpoints ([#818](#818)) ([6d5f196](6d5f196)) * design tokens and core UI components ([#833](#833)) ([ed887f2](ed887f2)) * extend approval, meeting, and budget API responses ([#834](#834)) ([31472bf](31472bf)) * frontend polish -- real-time UX, accessibility, responsive, performance ([#790](#790), [#792](#792), [#791](#791), [#793](#793)) ([#917](#917)) ([f04a537](f04a537)) * implement human roles and access control levels ([#856](#856)) ([d6d8a06](d6d8a06)) * implement semantic conflict detection in workspace merge ([#860](#860)) ([d97283b](d97283b)) * interaction components and animation patterns ([#853](#853)) ([82d4b01](82d4b01)) * Login page + first-run bootstrap + Company page ([#789](#789), [#888](#888)) ([#896](#896)) ([8758e8d](8758e8d)) * Meetings page with timeline viz, token bars, contribution formatting ([#788](#788)) ([#904](#904)) ([b207f46](b207f46)) * Messages page with threading, channel badges, sender indicators ([#787](#787)) ([#903](#903)) ([28293ad](28293ad)) * Org Chart force-directed view and drag-drop reassignment ([#872](#872), [#873](#873)) ([#912](#912)) ([a68a938](a68a938)) * Org Chart page (living nodes, status, CRUD, department health) ([#870](#870)) ([0acbdae](0acbdae)) * per-command flags for remaining commands, auto-behavior wiring, help/discoverability ([#897](#897)) ([3f7afa2](3f7afa2)) * Providers page with backend rework -- health, CRUD, subscription auth ([#893](#893)) ([9f8dd98](9f8dd98)) * scaffold React + Vite + TypeScript + Tailwind project ([#799](#799)) ([bd151aa](bd151aa)) * Settings page with search, dependency indicators, grouped rendering ([#784](#784)) ([#902](#902)) ([a7b9870](a7b9870)) * Setup Wizard rebuild with template comparison, cost estimator, theme customization ([#879](#879)) ([ae8b50b](ae8b50b)) * setup wizard UX -- template filters, card metadata, provider form reuse ([#910](#910)) ([7f04676](7f04676)) * setup wizard UX overhaul -- mode choice, step reorder, provider fixes ([#907](#907)) ([ee964c4](ee964c4)) * structured ModelRequirement in template agent configs ([#795](#795)) ([7433548](7433548)) * Task Board page (rich Kanban, filtering, dependency viz) ([#871](#871)) ([04a19b0](04a19b0)) ### Bug Fixes * align frontend types with backend and debounce WS refetches ([#916](#916)) ([134c11b](134c11b)) * auto-cleanup targets newly pulled images instead of old ones ([#884](#884)) ([50e6591](50e6591)) * correct wipe backup-skip flow and harden error handling ([#808](#808)) ([c05860f](c05860f)) * improve provider setup in wizard, subscription auth, dashboard bugs ([#914](#914)) ([87bf8e6](87bf8e6)) * improve update channel detection and add config get command ([#814](#814)) ([6b137f0](6b137f0)) * resolve all ESLint warnings, add zero-warnings enforcement ([#899](#899)) ([079b46a](079b46a)) * subscription auth uses api_key, base URL optional for cloud providers ([#915](#915)) ([f0098dd](f0098dd)) ### Refactoring * semantic analyzer cleanup -- shared filtering, concurrency, extraction ([#908](#908)) ([81372bf](81372bf)) ### Documentation * brand identity and UX design system from [#765](#765) exploration ([#804](#804)) ([389a9f4](389a9f4)) * page structure and information architecture for v0.5.0 dashboard ([#809](#809)) ([f8d6d4a](f8d6d4a)) * write UX design guidelines with WCAG-verified color system ([#816](#816)) ([4a4594e](4a4594e)) ### Tests * add unit tests for agent hooks and page components ([#875](#875)) ([#901](#901)) ([1d81546](1d81546)) ### CI/CD * bump actions/deploy-pages from 4.0.5 to 5.0.0 in the major group ([#831](#831)) ([01c19de](01c19de)) * bump astral-sh/setup-uv from 7.6.0 to 8.0.0 in /.github/actions/setup-python-uv in the all group ([#920](#920)) ([5f6ba54](5f6ba54)) * bump codecov/codecov-action from 5.5.3 to 6.0.0 in the major group ([#868](#868)) ([f22a181](f22a181)) * bump github/codeql-action from 4.34.1 to 4.35.0 in the all group ([#883](#883)) ([87a4890](87a4890)) * bump sigstore/cosign-installer from 4.1.0 to 4.1.1 in the minor-and-patch group ([#830](#830)) ([7a69050](7a69050)) * bump the all group with 3 updates ([#923](#923)) ([ff27c8e](ff27c8e)) * bump wrangler from 4.76.0 to 4.77.0 in /.github in the minor-and-patch group ([#822](#822)) ([07d43eb](07d43eb)) * bump wrangler from 4.77.0 to 4.78.0 in /.github in the all group ([#882](#882)) ([f84118d](f84118d)) ### Maintenance * add design system enforcement hook and component inventory ([#846](#846)) ([15abc43](15abc43)) * add dev-only auth bypass for frontend testing ([#885](#885)) ([6cdcd8a](6cdcd8a)) * add pre-push rebase check hook ([#855](#855)) ([b637a04](b637a04)) * backend hardening -- eviction/size-caps and model validation ([#911](#911)) ([81253d9](81253d9)) * bump axios from 1.13.6 to 1.14.0 in /web in the all group across 1 directory ([#922](#922)) ([b1b0232](b1b0232)) * bump brace-expansion from 5.0.4 to 5.0.5 in /web ([#862](#862)) ([ba4a565](ba4a565)) * bump eslint-plugin-react-refresh from 0.4.26 to 0.5.2 in /web ([#801](#801)) ([7574bb5](7574bb5)) * bump faker from 40.11.0 to 40.11.1 in the minor-and-patch group ([#803](#803)) ([14d322e](14d322e)) * bump https://github.com/astral-sh/ruff-pre-commit from v0.15.7 to 0.15.8 ([#864](#864)) ([f52901e](f52901e)) * bump nginxinc/nginx-unprivileged from `6582a34` to `f99cc61` in /docker/web in the all group ([#919](#919)) ([df85e4f](df85e4f)) * bump nginxinc/nginx-unprivileged from `ccbac1a` to `6582a34` in /docker/web ([#800](#800)) ([f4e9450](f4e9450)) * bump node from `44bcbf4` to `71be405` in /docker/sandbox ([#827](#827)) ([91bec67](91bec67)) * bump node from `5209bca` to `cf38e1f` in /docker/web ([#863](#863)) ([66d6043](66d6043)) * bump picomatch in /site ([#842](#842)) ([5f20bcc](5f20bcc)) * bump recharts 2->3 and @types/node 22->25 in /web ([#802](#802)) ([a908800](a908800)) * Bump requests from 2.32.5 to 2.33.0 ([#843](#843)) ([41daf69](41daf69)) * bump smol-toml from 1.6.0 to 1.6.1 in /site ([#826](#826)) ([3e5dbe4](3e5dbe4)) * bump the all group with 3 updates ([#921](#921)) ([7bace0b](7bace0b)) * bump the minor-and-patch group across 1 directory with 2 updates ([#829](#829)) ([93e611f](93e611f)) * bump the minor-and-patch group across 1 directory with 3 updates ([#841](#841)) ([7010c8e](7010c8e)) * bump the minor-and-patch group across 1 directory with 3 updates ([#869](#869)) ([548cee5](548cee5)) * bump the minor-and-patch group in /site with 2 updates ([#865](#865)) ([9558101](9558101)) * bump the minor-and-patch group with 2 updates ([#867](#867)) ([4830706](4830706)) * consolidate Dependabot groups to 1 PR per ecosystem ([06d2556](06d2556)) * consolidate Dependabot groups to 1 PR per ecosystem ([#881](#881)) ([06d2556](06d2556)) * improve worktree skill with full dep sync and status enhancements ([#906](#906)) ([772c625](772c625)) * remove Vue remnants and document framework decision ([#851](#851)) ([bf2adf6](bf2adf6)) * update web dependencies and fix brace-expansion CVE ([#880](#880)) ([a7a0ed6](a7a0ed6)) * upgrade to Storybook 10 and TypeScript 6 ([#845](#845)) ([52d95f2](52d95f2)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary
Comprehensive UX overhaul of the first-run setup wizard, addressing visual issues, broken flows, and missing functionality:
cost-estimator.tsand its tests also deleted.PUT /setup/agents/{idx}/personality,GET /setup/personality-presets) + editable dropdown in agent cards with all 24 presets.POST /setup/completeno longer requires agents (Quick Setup mode).page-structure.mdanduser_guide.mdreflect the new wizard flow.Review coverage
Pre-reviewed by 7 specialized agents (docs-consistency, code-reviewer, frontend-reviewer, api-contract-drift, silent-failure-hunter, type-design-analyzer, security-reviewer). 16 findings identified and addressed in the second commit.
Test plan
🤖 Generated with Claude Code