charles/agent-hooks

Fork 0

feat(web): BreakdownRulesSection for architect Routing fields (AR-2) #1208

Merged

reviewer merged 2 commits from code-lead/1202 into main

2026-05-15 15:55:39 +00:00

code-lead commented

2026-05-15 15:49:08 +00:00

Collaborator

Closes #1202.

Summary

When the operator opens the type-editor drawer for architect, the
Routing accordion now renders a /breakdown rules editor instead of
the worker-oriented chip lists. Worker types (dev, reviewer, …)
still mount the original RoutingSection. Only the architect branch
is replaced — every other section continues to work uniformly since AR-1.

The new BreakdownRulesSection edits the four architect-specific
fields that drive the /breakdown skill:

example_prompts — <ChipInput> over free-text prompts. Enter / comma commit.
label_rules — table of { pattern, exclude_pattern?, label }. Pattern + Exclude are regex with inline try { new RegExp() } validation; invalid rows flag aria-invalid + an error caption but still mirror the draft (server-side agents-config-schema.ts is the hard gate at the wire boundary).
assignee_rules — table of { when_label, assign_to }, both autocompleting from repoLabels / agentUsers via <datalist>.
default_assignee — single-line autocomplete from agentUsers, with the factory fallback hinted in the placeholder.

All four flow through onChange({ ...config, <field>: next }) to land
in the page's typesDraft → debounced autosave → putAgentConfig →
sub-resource splitter. example_prompts + default_assignee ride the
identity endpoint (they live on the agent_type row server-side);
label_rules + assignee_rules ride the routing endpoint.

What changed

apps/web/src/features/agents/sections/breakdown-rules-section.tsx — new component.
apps/web/src/features/agents/sections/section-pane.tsx — Routing branch picks BreakdownRulesSection when typeName === "architect".
apps/web/src/features/agents/sections/index.ts — barrel export.
apps/web/src/lib/api/agent-config-fleet.ts — extend AgentTypeConfig with the four fields + the LabelRule / AssigneeRule types; teach composeAgentTypeConfig to read them out of the resolver response; wire example_prompts + default_assignee into IDENTITY_FIELDS and label_rules + assignee_rules into ROUTING_FIELD_PAIRS.

Test plan

breakdown-rules-section.test.tsx — 17 tests covering Enter-to-commit chips, label_rules add/edit/exclude/remove, invalid-regex caption, assignee_rules autocomplete, default_assignee placeholder + datalist, payload integrity.
agents.roster.test.tsx — AR-2 integration suite: architect drawer mounts BreakdownRulesSection (RoutingSection is absent); editing a label rule debounces a PUT carrying label_rules; worker (dev) drawer still mounts the original RoutingSection.
just qa — 3362 server tests, 41 sections tests, format / lint / SQL-layer / paraglide / i18n-string / flow-schema all green.

🤖 Generated with Claude Code

Closes #1202. ## Summary When the operator opens the type-editor drawer for `architect`, the Routing accordion now renders a `/breakdown` rules editor instead of the worker-oriented chip lists. Worker types (`dev`, `reviewer`, …) still mount the original `RoutingSection`. Only the architect branch is replaced — every other section continues to work uniformly since AR-1. The new `BreakdownRulesSection` edits the four architect-specific fields that drive the `/breakdown` skill: - `example_prompts` — `<ChipInput>` over free-text prompts. Enter / comma commit. - `label_rules` — table of `{ pattern, exclude_pattern?, label }`. Pattern + Exclude are regex with inline `try { new RegExp() }` validation; invalid rows flag `aria-invalid` + an error caption but still mirror the draft (server-side `agents-config-schema.ts` is the hard gate at the wire boundary). - `assignee_rules` — table of `{ when_label, assign_to }`, both autocompleting from `repoLabels` / `agentUsers` via `<datalist>`. - `default_assignee` — single-line autocomplete from `agentUsers`, with the factory fallback hinted in the placeholder. All four flow through `onChange({ ...config, <field>: next })` to land in the page's `typesDraft` → debounced autosave → `putAgentConfig` → sub-resource splitter. `example_prompts` + `default_assignee` ride the identity endpoint (they live on the `agent_type` row server-side); `label_rules` + `assignee_rules` ride the routing endpoint. ## What changed - `apps/web/src/features/agents/sections/breakdown-rules-section.tsx` — new component. - `apps/web/src/features/agents/sections/section-pane.tsx` — Routing branch picks `BreakdownRulesSection` when `typeName === "architect"`. - `apps/web/src/features/agents/sections/index.ts` — barrel export. - `apps/web/src/lib/api/agent-config-fleet.ts` — extend `AgentTypeConfig` with the four fields + the `LabelRule` / `AssigneeRule` types; teach `composeAgentTypeConfig` to read them out of the resolver response; wire `example_prompts` + `default_assignee` into `IDENTITY_FIELDS` and `label_rules` + `assignee_rules` into `ROUTING_FIELD_PAIRS`. ## Test plan - [x] `breakdown-rules-section.test.tsx` — 17 tests covering Enter-to-commit chips, label_rules add/edit/exclude/remove, invalid-regex caption, assignee_rules autocomplete, default_assignee placeholder + datalist, payload integrity. - [x] `agents.roster.test.tsx` — AR-2 integration suite: architect drawer mounts `BreakdownRulesSection` (RoutingSection is absent); editing a label rule debounces a `PUT` carrying `label_rules`; worker (`dev`) drawer still mounts the original `RoutingSection`. - [x] `just qa` — 3362 server tests, 41 sections tests, format / lint / SQL-layer / paraglide / i18n-string / flow-schema all green. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

code-lead added 2 commits

2026-05-15 15:49:09 +00:00

feat(web): BreakdownRulesSection swaps Routing body for architect (AR-2, #1202 ) 93577cc0da

When the operator opens the type-editor drawer for `architect`, the
Routing accordion now renders a `/breakdown` rules editor instead of
the worker-oriented chip lists. Worker types (`dev`, `reviewer`, …)
still mount the original `RoutingSection` — only the architect branch
is replaced.

`BreakdownRulesSection` edits the four architect-specific fields that
drive the `/breakdown` skill: `example_prompts`, `label_rules`,
`assignee_rules`, and `default_assignee`. All mutations flow through
`onChange` to the page's draft → autosave splitter, which routes the
identity-resident fields (`example_prompts`, `default_assignee`) to
`PUT /api/agent-types/:name/identity` and the rule arrays to
`PUT /api/agent-types/:name/routing`. Regex patterns get inline
validation; invalid patterns flag the row with `aria-invalid` + an
error caption but still update the draft (server-side schema is the
hard gate at the wire boundary).

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

fix(web): i18n-string-check placeholder rejects 'agent user' (AR-2)

qa / sql-layer-check (pull_request) Successful in 6s

Details

qa / i18n-string-check (pull_request) Successful in 12s

Details

qa / dockerfile (pull_request) Successful in 12s

Details

qa / db-schema (pull_request) Successful in 14s

Details

qa / qa-1 (pull_request) Successful in 2m23s

Details

qa / qa (pull_request) Successful in 0s

Details

7fdb8c5ce8

Pre-push raw-string guard flagged `placeholder="agent user"` on the
assignee_rules assign_to input — a multi-word user-visible string that
isn't routed through Paraglide. Switch to `e.g. dev` so it falls under
the `e.g.` exception and the hint still tells operators what to type.

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

code-lead requested review from reviewer

2026-05-15 15:52:09 +00:00

reviewer approved these changes

2026-05-15 15:55:21 +00:00

reviewer left a comment

Review — AR-2: BreakdownRulesSection (feat/web, #1202)

CI ✅ green. APPROVED — solid implementation with good test coverage. A few non-blocking observations below.

What the PR does

Replaces the RoutingSection body with a new BreakdownRulesSection when the drawer is opened for the architect type. Four architect-specific fields are now editable: example_prompts, label_rules, assignee_rules, and default_assignee. The splitter correctly routes example_prompts/default_assignee through the identity endpoint and label_rules/assignee_rules through the routing endpoint.

All other types continue to use the original RoutingSection — the change is surgically isolated to the architect branch of sectionBody().

Correctness

composeAgentTypeConfig — type-safe reads. default_assignee and example_prompts already existed in AgentTypeSettingsResponse.resolved on main; the PR correctly reads them from r rather than r.routing. label_rules/assignee_rules are added to the routing sub-object and read from r.routing. Both paths land on the right wire endpoints in putAgentConfig. ✓

Splitter diff guard. The routing body only includes fields that changed (routingBody[dst] = n[src] ?? [] is gated inside if (!eq(...))) — so a worker type saving routes_labels won't spuriously send label_rules: [] to its routing endpoint. ✓

exclude_pattern clean-up. Stripping the key when the trimmed value is "" keeps the wire payload clean and matches the schema's optional semantics. ✓

default_assignee clean-up. Destructuring and omitting the key on clear is consistent with how the server falls back to the factory default. ✓

isValidRegex. Treating "" as valid is intentional and correct — the rule is simply inert. Inline aria-invalid + aria-describedby wiring is accessible. ✓

Non-blocking observations

1. Empty when_label/assign_to rows go straight to the autosave debounce.
Adding an assignee rule row initialises it as { when_label: "", assign_to: "" }. After 800 ms the debouncer fires and the splitter detects a diff vs. the server state, sending that empty row. The server schema is the hard gate (as the PR description states), so this will surface as a toast error if the operator doesn't fill the row quickly enough. The same applies to label_rules. Neither is new behaviour introduced in isolation by this PR — it's the same design trade-off as the rest of the table-based sections — but both tables appear together here for the first time, so it's worth keeping in mind when the UX is reviewed end-to-end (e.g. debounce could be skipped when any row is empty).

2. when_pattern in the type but absent from the UI.
AssigneeRule declares when_pattern?: string, and the JSDoc says "Either when_label or when_pattern is set". The component only renders a when_label input — a rule that was created with when_pattern (not currently possible since this is new) would be round-tripped opaquely on save but can't be edited or cleared from the UI. Fine for AR-2 scope; just worth tracking if when_pattern routing is implemented later.

3. No test for invalid exclude_pattern regex.
The pattern field has an explicit "[unclosed" test (with aria-invalid + error caption assertions). The exclude_pattern field uses identical validation logic but has no symmetric test. The code is obviously correct by symmetry, but an explicit test would close the gap.

4. Hardcoded "(default: dev)" placeholder.
The factory fallback is inlined in the component. If the server default is ever changed the UI hint will silently go stale. Fetching it from the derived config or a constant would be more resilient — low priority given how stable the factory default is.

Tests

17 unit tests covering every field + the invalid-regex and clear-strips-key edge cases, plus 3 integration tests verifying the drawer swap, debounced PUT payload, and the worker fallback. Coverage is thorough. The vi.useFakeTimers() / vi.useRealTimers() fencing around the debounce test is well-structured.

LGTM. The observations above are all non-blocking. Merging when ready.

## Review — AR-2: BreakdownRulesSection (feat/web, #1202) CI ✅ green. APPROVED — solid implementation with good test coverage. A few non-blocking observations below. --- ### What the PR does Replaces the `RoutingSection` body with a new `BreakdownRulesSection` when the drawer is opened for the `architect` type. Four architect-specific fields are now editable: `example_prompts`, `label_rules`, `assignee_rules`, and `default_assignee`. The splitter correctly routes `example_prompts`/`default_assignee` through the identity endpoint and `label_rules`/`assignee_rules` through the routing endpoint. All other types continue to use the original `RoutingSection` — the change is surgically isolated to the `architect` branch of `sectionBody()`. --- ### Correctness **`composeAgentTypeConfig` — type-safe reads.** `default_assignee` and `example_prompts` already existed in `AgentTypeSettingsResponse.resolved` on `main`; the PR correctly reads them from `r` rather than `r.routing`. `label_rules`/`assignee_rules` are added to the routing sub-object and read from `r.routing`. Both paths land on the right wire endpoints in `putAgentConfig`. ✓ **Splitter diff guard.** The routing body only includes fields that _changed_ (`routingBody[dst] = n[src] ?? []` is gated inside `if (!eq(...))`) — so a worker type saving `routes_labels` won't spuriously send `label_rules: []` to its routing endpoint. ✓ **`exclude_pattern` clean-up.** Stripping the key when the trimmed value is `""` keeps the wire payload clean and matches the schema's optional semantics. ✓ **`default_assignee` clean-up.** Destructuring and omitting the key on clear is consistent with how the server falls back to the factory default. ✓ **`isValidRegex`.** Treating `""` as valid is intentional and correct — the rule is simply inert. Inline `aria-invalid` + `aria-describedby` wiring is accessible. ✓ --- ### Non-blocking observations **1. Empty `when_label`/`assign_to` rows go straight to the autosave debounce.** Adding an assignee rule row initialises it as `{ when_label: "", assign_to: "" }`. After 800 ms the debouncer fires and the splitter detects a diff vs. the server state, sending that empty row. The server schema is the hard gate (as the PR description states), so this will surface as a toast error if the operator doesn't fill the row quickly enough. The same applies to `label_rules`. Neither is new behaviour introduced in isolation by this PR — it's the same design trade-off as the rest of the table-based sections — but both tables appear together here for the first time, so it's worth keeping in mind when the UX is reviewed end-to-end (e.g. debounce could be skipped when any row is empty). **2. `when_pattern` in the type but absent from the UI.** `AssigneeRule` declares `when_pattern?: string`, and the JSDoc says "Either `when_label` or `when_pattern` is set". The component only renders a `when_label` input — a rule that was created with `when_pattern` (not currently possible since this is new) would be round-tripped opaquely on save but can't be edited or cleared from the UI. Fine for AR-2 scope; just worth tracking if `when_pattern` routing is implemented later. **3. No test for invalid `exclude_pattern` regex.** The `pattern` field has an explicit `"[unclosed"` test (with `aria-invalid` + error caption assertions). The `exclude_pattern` field uses identical validation logic but has no symmetric test. The code is obviously correct by symmetry, but an explicit test would close the gap. **4. Hardcoded `"(default: dev)"` placeholder.** The factory fallback is inlined in the component. If the server default is ever changed the UI hint will silently go stale. Fetching it from the derived config or a constant would be more resilient — low priority given how stable the factory default is. --- ### Tests 17 unit tests covering every field + the invalid-regex and clear-strips-key edge cases, plus 3 integration tests verifying the drawer swap, debounced PUT payload, and the worker fallback. Coverage is thorough. The `vi.useFakeTimers()` / `vi.useRealTimers()` fencing around the debounce test is well-structured. --- **LGTM.** The observations above are all non-blocking. Merging when ready.

reviewer merged commit c7ed2aad1d into main

2026-05-15 15:55:39 +00:00