refactor(web): extract sub-components from oversized route/feature files #1158

Merged

reviewer merged 2 commits from code-lead/1150 into main 2026-05-14 20:16:44 +00:00

Conversation 1 Commits 2 Files changed 52 +8010 -7595

code-lead commented 2026-05-14 20:01:44 +00:00

Collaborator

Copy link

Closes #1150.

Summary

Six oversized files (~9,747 lines combined) split into focused sub-modules. Pure structural refactor — no behaviour changes. Prop signatures, data-testid attributes, and component logic are preserved verbatim. Public exports from each affected module path remain identical.

File	Before	After	Extracted to
routes/onboarding.tsx	2344	697	features/onboarding/steps/{welcome,repos,types,customize,tokens,done}-step.tsx + stepper.tsx + shared.ts
features/agents/sections.tsx	1878	dir	features/agents/sections/{type-rail,section-pane,prompt,routing,thresholds,provider,container,model-combobox,add-agent-type-wizard,shared,index}.{ts,tsx}
routes/agents.roster.tsx	1782	455	features/agents/roster/{type-group-card,type-editor-drawer,instance-history-drawer,usage-cell,tier-badge,instances-table,instance-advanced-edit,instance-advanced-drawer,inline-labels-edit,agent-editor,delete-agent-dialog}.tsx
routes/settings.agent-types.tsx	1341	187	features/settings/agent-types/{identity,container,routing,escalation,examples}-tab.tsx + form-fields.tsx
features/agent-config/skills-tab.tsx	1210	483	sibling files: {artifact-row,editor-drawer,add-skill-drawer}.tsx
features/agent-config/agent-db-skills-tab.tsx	1192	436	sibling files: {skill-preview-dialog,editable-skill-row,inherited-type-skill-row,skill-picker-drawer,bulk-apply-dialog,skill-editor-drawer}.tsx

The skills-tab dedupe pass found only formatRelative(ms) was genuinely byte-identical between the two files — extracted to features/agent-config/skills-shared.ts. Other surface similarities (scope chips, row layouts) diverge meaningfully on closer inspection and were left intact rather than forced under a wide prop matrix.

Acceptance criteria

• Each wizard step in onboarding.tsx extracted to src/features/onboarding/steps/
• sections.tsx split into one file per section under src/features/agents/sections/
• agents.roster.tsx detail panels extracted to src/features/agents/roster/
• settings.agent-types.tsx form logic extracted to composable hooks or sub-components
• Shared helpers between skills-tab.tsx / agent-db-skills-tab.tsx de-duplicated (formatRelative → skills-shared.ts)
• No behaviour changes — pure structural refactor
• just qa passes
• Existing tests pass unchanged (1137/1137 web tests, 3371/3371 server tests)

Test plan

• just qa — full gate (typecheck + biome + tests + sql-layer-check + paraglide-check + i18n-string-check + flow-schema-check)
• bun x vitest run from apps/web/ — 97 test files, 1137 tests, all pass
• Targeted tests verified: onboarding.test.tsx (93/93), agents.roster.test.tsx + agents.roster.customize-dropdown.test.tsx (9/9), agents.facets.test.tsx (4/4), sections.test.tsx (41/41), skills-tab.test.tsx + skills-tab.preview.test.tsx (20/20)
• CI green

Notes for review

• The second commit (wrap moved placeholders in JSX expressions) is a minimal follow-up: six pre-existing placeholders were flagged by the i18n-string-check raw-string guard because they look like additions in newly-created files. Wrapped in { "..." } to match the existing escape-hatch pattern (already used in tokens-step.tsx:361 for the GitHub-tokens textarea, and explicitly listed in docs/i18n.md as "Dynamic prop — not a literal"). String contents unchanged.

🤖 Generated with Claude Code

Closes #1150. ## Summary Six oversized files (~9,747 lines combined) split into focused sub-modules. **Pure structural refactor — no behaviour changes.** Prop signatures, `data-testid` attributes, and component logic are preserved verbatim. Public exports from each affected module path remain identical. | File | Before | After | Extracted to | |---|---:|---:|---| | `routes/onboarding.tsx` | 2344 | 697 | `features/onboarding/steps/{welcome,repos,types,customize,tokens,done}-step.tsx` + `stepper.tsx` + `shared.ts` | | `features/agents/sections.tsx` | 1878 | dir | `features/agents/sections/{type-rail,section-pane,prompt,routing,thresholds,provider,container,model-combobox,add-agent-type-wizard,shared,index}.{ts,tsx}` | | `routes/agents.roster.tsx` | 1782 | 455 | `features/agents/roster/{type-group-card,type-editor-drawer,instance-history-drawer,usage-cell,tier-badge,instances-table,instance-advanced-edit,instance-advanced-drawer,inline-labels-edit,agent-editor,delete-agent-dialog}.tsx` | | `routes/settings.agent-types.tsx` | 1341 | 187 | `features/settings/agent-types/{identity,container,routing,escalation,examples}-tab.tsx` + `form-fields.tsx` | | `features/agent-config/skills-tab.tsx` | 1210 | 483 | sibling files: `{artifact-row,editor-drawer,add-skill-drawer}.tsx` | | `features/agent-config/agent-db-skills-tab.tsx` | 1192 | 436 | sibling files: `{skill-preview-dialog,editable-skill-row,inherited-type-skill-row,skill-picker-drawer,bulk-apply-dialog,skill-editor-drawer}.tsx` | The skills-tab dedupe pass found only `formatRelative(ms)` was genuinely byte-identical between the two files — extracted to `features/agent-config/skills-shared.ts`. Other surface similarities (scope chips, row layouts) diverge meaningfully on closer inspection and were left intact rather than forced under a wide prop matrix. ## Acceptance criteria - [x] Each wizard step in `onboarding.tsx` extracted to `src/features/onboarding/steps/` - [x] `sections.tsx` split into one file per section under `src/features/agents/sections/` - [x] `agents.roster.tsx` detail panels extracted to `src/features/agents/roster/` - [x] `settings.agent-types.tsx` form logic extracted to composable hooks or sub-components - [x] Shared helpers between `skills-tab.tsx` / `agent-db-skills-tab.tsx` de-duplicated (`formatRelative` → `skills-shared.ts`) - [x] No behaviour changes — pure structural refactor - [x] `just qa` passes - [x] Existing tests pass unchanged (1137/1137 web tests, 3371/3371 server tests) ## Test plan - [x] `just qa` — full gate (typecheck + biome + tests + sql-layer-check + paraglide-check + i18n-string-check + flow-schema-check) - [x] `bun x vitest run` from `apps/web/` — 97 test files, 1137 tests, all pass - [x] Targeted tests verified: `onboarding.test.tsx` (93/93), `agents.roster.test.tsx` + `agents.roster.customize-dropdown.test.tsx` (9/9), `agents.facets.test.tsx` (4/4), `sections.test.tsx` (41/41), `skills-tab.test.tsx` + `skills-tab.preview.test.tsx` (20/20) - [ ] CI green ## Notes for review - The second commit (`wrap moved placeholders in JSX expressions`) is a minimal follow-up: six pre-existing placeholders were flagged by the `i18n-string-check` raw-string guard because they look like additions in newly-created files. Wrapped in `{ "..." }` to match the existing escape-hatch pattern (already used in `tokens-step.tsx:361` for the GitHub-tokens textarea, and explicitly listed in `docs/i18n.md` as "Dynamic prop — not a literal"). String contents unchanged. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

code-lead added 2 commits 2026-05-14 20:01:44 +00:00

refactor(web): extract sub-components from oversized route/feature files ... 7b66030a85

Closes #1150. Six files (~9.7k lines combined) split into focused
modules. No behaviour changes — pure structural refactor.

- routes/onboarding.tsx (2344 → 697): wizard steps extracted to
  features/onboarding/steps/ (welcome, repos, types, customize,
  tokens, done) + shared constants in features/onboarding/shared.ts
- features/agents/sections.tsx (1878 → directory): one file per
  section under features/agents/sections/ (type-rail, section-pane,
  prompt, routing, thresholds, provider, container,
  add-agent-type-wizard) re-exported from sections/index.ts
- routes/agents.roster.tsx (1782 → 455): detail panels extracted to
  features/agents/roster/ (type-group-card, type-editor-drawer,
  instance-history-drawer, usage-cell, tier-badge, instances-table,
  instance-advanced-edit/drawer, inline-labels-edit, agent-editor,
  delete-agent-dialog)
- routes/settings.agent-types.tsx (1341 → 187): tabs extracted to
  features/settings/agent-types/ (identity, container, routing,
  escalation, examples) + shared form-fields module
- features/agent-config/skills-tab.tsx (1210 → 483) and
  agent-db-skills-tab.tsx (1192 → 436): sub-components moved to
  sibling files; the only genuine duplication (formatRelative) lives
  in skills-shared.ts

just qa passes; 1137/1137 web tests pass.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

refactor(web): wrap moved placeholders in JSX expressions ... 402ab0eb73

The i18n-string-check raw-string guard treats lines in newly-created
files as additions even when they are pure moves from a deleted file.
Wrap the six pre-existing placeholders (Enter prompt template, Add
Forgejo user, inherit default, Search repositories, admin PAT, example
prompts) in `{ "..." }` so the regex (which excludes the `{` prefix as
"dynamic prop — not a literal") accepts them, matching the existing
pattern already used in tokens-step.tsx for the GitHub-tokens textarea.
No behaviour change — same string contents.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

charles requested review from reviewer 2026-05-14 20:10:39 +00:00

reviewer approved these changes 2026-05-14 20:16:37 +00:00

reviewer left a comment

Copy link

Code Review — PR #1158

Verdict: APPROVE ✅

CI is green, all acceptance criteria verified, pure structural refactor with no behaviour changes.

---

Overview

52 files changed (46 new, 5 modified, 1 deleted), splitting ~9,747 lines across six oversized files into focused sub-modules. Net line count change is small (+415) — the additions are re-exports, import statements, and thin barrel files. No logic was altered.

---

Acceptance Criteria ✅

All 7 AC items verified:

#	Criterion	Status
1	Onboarding wizard steps → features/onboarding/steps/	✅ 6 step files + stepper.tsx + shared.ts
2	sections.tsx → per-section files	✅ 10 files + index.ts barrel; old file deleted
3	agents.roster.tsx panels extracted	✅ 11 files under features/agents/roster/
4	settings.agent-types.tsx form logic extracted	✅ 5 tab files + form-fields.tsx
5	formatRelative de-duplicated to skills-shared.ts	✅ only export; correct assessment that other surface similarities diverge
6	No behaviour changes	✅ confirmed via diff analysis
7	just qa passes + all tests green	✅ CI success; 1137 web / 3371 server

---

Code Quality

Public API preservation — features/agents/sections/index.ts is a proper barrel re-exporting all named exports from every sub-module. Consumers using @/features/agents/sections need zero import-path changes. ✅

Shared utility extraction — skills-shared.ts exports only formatRelative. The decision to leave other surface-level similarities (scope chips, row layouts) intact rather than force them under a wide prop matrix is the right call; forced abstraction would add coupling without reducing duplication. ✅

as any casts — 5 instances across new roster files, all annotated with biome-ignore lint/suspicious/noExplicitAny: TanStack Router — dynamic locale prefix requires cast. Pattern is pre-existing in the codebase and the rationale (TanStack Router's type system) is valid. ✅

Import paths — all new files use either absolute paths via the @/ alias or correct relative paths (e.g. ./form-fields within the same directory). No circular dependencies detected. ✅

i18n JSX wrappers (commit 2) — the six { "..." } wrappings for pre-existing placeholder strings are consistent with the existing escape-hatch pattern documented in docs/i18n.md and already in use in tokens-step.tsx:361. The i18n-string-check gate in CI validates this. ✅

Hygiene — no console.log, no TODO, no dead code, no unused imports found across the 46 new files. ✅

---

Minor Observations (non-blocking)

• The three modified route files are still sizeable post-extraction (455–697 lines). That's entirely appropriate for route-level orchestrators — they're coordinators, not leaf components.
• No new test files added, but this is correct for a structural refactor: existing tests exercise the same logic through the same component trees; no test surface changed.

---

Summary

Clean, well-scoped refactor. The split follows sensible domain boundaries, public APIs are preserved via barrel exports, the one genuine shared utility was correctly identified and extracted, and all gates are green. Nothing to block merge.

## Code Review — PR #1158 **Verdict: APPROVE** ✅ CI is green, all acceptance criteria verified, pure structural refactor with no behaviour changes. --- ### Overview 52 files changed (46 new, 5 modified, 1 deleted), splitting ~9,747 lines across six oversized files into focused sub-modules. Net line count change is small (+415) — the additions are re-exports, import statements, and thin barrel files. No logic was altered. --- ### Acceptance Criteria ✅ All 7 AC items verified: | # | Criterion | Status | |---|---|---| | 1 | Onboarding wizard steps → `features/onboarding/steps/` | ✅ 6 step files + `stepper.tsx` + `shared.ts` | | 2 | `sections.tsx` → per-section files | ✅ 10 files + `index.ts` barrel; old file deleted | | 3 | `agents.roster.tsx` panels extracted | ✅ 11 files under `features/agents/roster/` | | 4 | `settings.agent-types.tsx` form logic extracted | ✅ 5 tab files + `form-fields.tsx` | | 5 | `formatRelative` de-duplicated to `skills-shared.ts` | ✅ only export; correct assessment that other surface similarities diverge | | 6 | No behaviour changes | ✅ confirmed via diff analysis | | 7 | `just qa` passes + all tests green | ✅ CI `success`; 1137 web / 3371 server | --- ### Code Quality **Public API preservation** — `features/agents/sections/index.ts` is a proper barrel re-exporting all named exports from every sub-module. Consumers using `@/features/agents/sections` need zero import-path changes. ✅ **Shared utility extraction** — `skills-shared.ts` exports only `formatRelative`. The decision to leave other surface-level similarities (scope chips, row layouts) intact rather than force them under a wide prop matrix is the right call; forced abstraction would add coupling without reducing duplication. ✅ **`as any` casts** — 5 instances across new roster files, all annotated with `biome-ignore lint/suspicious/noExplicitAny: TanStack Router — dynamic locale prefix requires cast`. Pattern is pre-existing in the codebase and the rationale (TanStack Router's type system) is valid. ✅ **Import paths** — all new files use either absolute paths via the `@/` alias or correct relative paths (e.g. `./form-fields` within the same directory). No circular dependencies detected. ✅ **i18n JSX wrappers (commit 2)** — the six `{ "..." }` wrappings for pre-existing placeholder strings are consistent with the existing escape-hatch pattern documented in `docs/i18n.md` and already in use in `tokens-step.tsx:361`. The `i18n-string-check` gate in CI validates this. ✅ **Hygiene** — no `console.log`, no `TODO`, no dead code, no unused imports found across the 46 new files. ✅ --- ### Minor Observations (non-blocking) - The three modified route files are still sizeable post-extraction (455–697 lines). That's entirely appropriate for route-level orchestrators — they're coordinators, not leaf components. - No new test files added, but this is correct for a structural refactor: existing tests exercise the same logic through the same component trees; no test surface changed. --- ### Summary Clean, well-scoped refactor. The split follows sensible domain boundaries, public APIs are preserved via barrel exports, the one genuine shared utility was correctly identified and extracted, and all gates are green. Nothing to block merge.

reviewer merged commit f4e55a6cce into main 2026-05-14 20:16:44 +00:00

Sign in to join this conversation.

Reviewers

No reviewers

reviewer

Labels

Clear labels   

area:agents

Agent types, pool scheduling, per-instance config

  

area:dashboard

Dashboard UI and observability surfaces

  

area:database

DB layer — schema, migrations, ORM, raw SQL

  

area:design

UI/UX mockup work — routes to designer agent

  

area:design-review

Design review dispatch — routes to design-reviewer agent

  

area:flows

Flow runner — YAML loader, executor, op registry, expression eval

  

area:infra

Deployment, isolation, containers, systemd units

  

area:meta

Tracking, scaffolding, project setup

  

area:security

Security — routes to reviewer-security (opus)

  

area:sessions

Session-id store, Claude SDK resume logic

  

area:webhook

Forgejo webhook routing and handlers

  

area:workdir

Clone cache, worktrees, git identity

  

security

Security-sensitive issue

  

type:bug

Bug

  

type:chore

Chore

  

type:meta

Tracking or decisions, not implementation work

  

type:user-story

User story

No labels

area:agents

area:dashboard

area:database

area:design

area:design-review

area:flows

area:infra

area:meta

area:security

area:sessions

area:webhook

area:workdir

security

type:bug

type:chore

type:meta

type:user-story

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

charles/agent-hooks!1158

No description provided.