diff --git a/codev/plans/819-core-parsearealabels-helper-fl.md b/codev/plans/819-core-parsearealabels-helper-fl.md new file mode 100644 index 00000000..b9135c25 --- /dev/null +++ b/codev/plans/819-core-parsearealabels-helper-fl.md @@ -0,0 +1,279 @@ +# PIR Plan: `parseArea` helper + `area` on `BacklogItem` and `BuilderOverview` + +> **Revisions during implement (2026-05-27)** — two corrections to the original design: +> +> 1. **Multi-area → single-area at the parser.** Swapped `areas: string[]` + `resolvePrimaryArea` projection helper for `area: string` returned directly by `parseArea`. The original permitted multi-area at the data layer and then collapsed it to one bucket at the UI boundary — two operations cancelling out. Revised: parser does the projection once, returns single string with `'Uncategorized'` fallback, symmetric with `parseLabelDefaults`'s single-string `type` / `priority`. `resolvePrimaryArea` deleted entirely. +> +> 2. **Parser is policy-free about label names.** Original projection privileged `area/cross-cutting` (returned it preferentially when present). Removed — the parser now just picks first alphabetical and falls back to `'Uncategorized'`. Codev framework code shouldn't bake in semantic conventions about specific label names; teams using Codev decide their own labeling conventions (some may use `cross-cutting`, some may not). Regression-guard test added that uses `area/cross-cutting` as fixture data alongside other areas and asserts first-alphabetical wins regardless. +> +> Acceptance criteria from the issue body that are now obsolete: "BacklogItem.areas: string[]" reads as "BacklogItem.area: string"; "resolvePrimaryArea helper" reads as "obsoleted by parser-side projection"; "multi-area sorting" reads as "first-alphabetical projection"; "area/cross-cutting presence" tests removed (cross-cutting has no special status in the parser). + +## Understanding + +Issue #819 introduces the `area/*` label namespace as Codev's grouping axis for two upcoming consumers — backlog grouping by area (#811) and the builders tree grouping by area (#818). Both consumers need the same primitives: + +1. A parser that extracts `area/*` label values (slash-separated, per Kubernetes / Terraform / CNCF convention) — currently no helper exists; `parseLabelDefaults` only knows about `type:*` and `priority:*` (colon-separated). +2. The parsed `areas: string[]` flowing through the server-side `BacklogItem` and `BuilderOverview` shapes and across the wire on `OverviewBacklogItem` / `OverviewBuilder` so the downstream consumers (dashboard `BacklogList`, dashboard `WorkView`, vscode `backlog` view, vscode `builders` view) can group without a second fetch. +3. A shared `resolvePrimaryArea(areas)` policy helper in `packages/core/src/builder-helpers.ts` so both UI surfaces resolve `area/cross-cutting → 'cross-cutting'`, single-area → that area, multi-area → first alphabetical, no-area → `'Uncategorized'` identically. + +This is the scaffolding-only landing — the consumers (#811, #818) ship in follow-up issues and will assume `areas` is always present. + +### Codebase landmarks (verified at this commit) + +- **Parser anchor**: `packages/codev/src/lib/github.ts:468` — `parseLabelDefaults`. New helper lands directly below this function so the two parsers sit together. Both share the same defensive non-array coercion pattern (Gitea / Forgejo return `null` / `""` for empty labels; only GitHub returns `[]`). +- **Server shapes**: `packages/codev/src/agent-farm/servers/overview.ts:35` (`BuilderOverview`), `:95` (`BacklogItem`). Both gain `areas: string[]`. +- **BacklogItem construction**: `packages/codev/src/agent-farm/servers/overview.ts:736` inside `deriveBacklog`. Populated alongside the existing `parseLabelDefaults(issue.labels, issue.title)` call. +- **BuilderOverview construction**: 4 sites — `discoverBuilders` at lines ~578 (no-projectId soft-mode branch), ~632 (strict-mode branch), ~666 (status.yaml-missing soft-mode branch); each must initialize `areas: []`. Enrichment from issue labels happens in `getOverview` around line ~870, alongside the existing `issueTitleMap` enrichment loop. +- **Wire contracts**: `packages/types/src/api.ts:130` (`OverviewBuilder`), `:187` (`OverviewBacklogItem`). Both gain `areas: string[]`. Re-exported via `packages/types/src/index.ts:18-29` (no change needed — re-exports propagate the new field automatically). +- **Policy helper**: `packages/core/src/builder-helpers.ts` — sits next to `isIdleWaiting` (same "UI policy that both vscode and dashboard need" niche). +- **Tests**: `packages/codev/src/__tests__/github.test.ts:89` — `parseLabelDefaults` test block. New `parseAreaLabels` tests added in a sibling `describe` block. + +### Cache discipline §B — already satisfied structurally + +Issue #819 amendment §B asks for defensive `entry.areas ??= []` at the cache serve-out point OR cache invalidation on Tower restart. Verified at `packages/codev/src/agent-farm/servers/overview.ts:763-769`: `OverviewCache` only holds **raw** forge responses (`prCache: ForgePR[]`, `issueCache: ForgeIssueListItem[]`, `closedCache`, `mergedPRCache`, `currentUserCache`) — it does **not** cache derived `BacklogItem` or `BuilderOverview` objects. Those shapes are reconstructed fresh from raw cache entries on every `getOverview` call via `deriveBacklog` and `discoverBuilders`, so `areas` is computed from current label data each request. The "stale cache entry missing `areas`" failure mode the amendment defends against is not reachable in this codebase as currently structured. **No defensive coercion needed at serve-out, no cache invalidation needed at Tower restart.** Logged here explicitly because the issue body presents §B as a mandatory acceptance item — the criterion is *met* by the existing architecture (no derived-shape cache), not by code we wrote. + +## Proposed Change + +### 1. `parseAreaLabels` helper in `packages/codev/src/lib/github.ts` + +Add directly below `parseLabelDefaults` (~line 510, after its closing brace): + +```ts +/** + * Extract `area/*` label values (Codev convention for grouping by product area). + * Returns sorted, deduplicated area names (without the `area/` prefix). + * Returns `[]` when no `area/*` labels are present. + * + * Mirrors `parseLabelDefaults`'s defensive non-array coercion: Gitea/Forgejo + * return `""` or `null` for empty labels instead of `[]`. + */ +export function parseAreaLabels( + labels: Array<{ name: string }> | null | undefined | string, +): string[] { + const names = Array.isArray(labels) ? labels.map(l => l.name) : []; + return [...new Set( + names + .filter(n => n.startsWith('area/')) + .map(n => n.slice(5)), + )].sort(); +} +``` + +### 2. Server-side shape additions in `packages/codev/src/agent-farm/servers/overview.ts` + +**Import** (line 19, alongside `parseLabelDefaults`): +```ts +parseLabelDefaults, +parseAreaLabels, +``` + +**`BuilderOverview` interface (line 35)** — add the field with a docstring keyed to the join site: +```ts +/** + * `area/*` label values for this builder's issue (sorted, deduplicated, + * prefix stripped). `[]` when the builder has no issue or the issue has + * no `area/*` labels. Populated by `getOverview` via the issue-cache join + * after `discoverBuilders` returns — `discoverBuilders` itself sets it + * to `[]` since it has no access to the issue payload. + */ +areas: string[]; +``` + +**`BacklogItem` interface (line 95)** — add the field: +```ts +/** + * `area/*` label values for this issue (sorted, deduplicated, prefix + * stripped). `[]` when the issue has no `area/*` labels. + */ +areas: string[]; +``` + +**`discoverBuilders` (~lines 578, 632, 666)** — initialize `areas: []` at each of the 3 builder construction sites. (Each `builders.push({...})` block.) + +**`deriveBacklog` (line 736)** — populate from `parseAreaLabels`: +```ts +const item: BacklogItem = { + // ...existing fields... + areas: parseAreaLabels(issue.labels), + hasSpec: !!specFile, + // ... +}; +``` + +**`getOverview` (~line 870)** — extend the existing issue→builder enrichment block: +```ts +const issueTitleMap = new Map(issues.map(i => [String(i.number), i.title])); +const issueLabelsMap = new Map(issues.map(i => [String(i.number), parseAreaLabels(i.labels)])); +for (const b of builders) { + if (b.issueId !== null) { + if (issueTitleMap.has(b.issueId)) b.issueTitle = issueTitleMap.get(b.issueId)!; + if (issueLabelsMap.has(b.issueId)) b.areas = issueLabelsMap.get(b.issueId)!; + } +} +``` + +(The existing block already gates on `b.issueId !== null && issueTitleMap.has(b.issueId)`; I'm splitting the inner condition so a builder whose issue is in the map for one lookup but not the other still gets the available enrichment. In practice both maps are populated from the same `issues` array, so this is a no-op divergence — but it makes the intent clearer than nesting.) + +### 3. Wire contract additions in `packages/types/src/api.ts` + +Add `areas: string[]` to **both**: +- `OverviewBuilder` (line 130, after `spawnedByArchitect`) +- `OverviewBacklogItem` (line 187, after `priority`) + +Same docstrings as the server-internal shapes. Required field with `[]` default discipline applies — never optional, never `undefined`. + +`packages/types/src/index.ts` does not need changes — both types are already re-exported as namespace types, so the new field propagates automatically. `packages/dashboard/src/lib/api.ts:70-72` re-exports from `@cluesmith/codev-types`, so the new field flows to the dashboard for free. + +### 4. `resolvePrimaryArea` helper in `packages/core/src/builder-helpers.ts` + +Add below `isIdleWaiting`: + +```ts +/** + * Pick the single group an issue / builder belongs to, per the area-grouping + * convention shared by the dashboard backlog view (#811) and the vscode + * builders tree (#818). + * + * Resolution order: + * - `'cross-cutting'` if `area/cross-cutting` is present (multi-area work + * by intent — never bucketed under one of its areas) + * - the first alphabetical area otherwise (`areas` is already sorted by + * `parseAreaLabels`, so `areas[0]` is the lexicographically smallest) + * - `'Uncategorized'` if no `area/*` labels at all + * + * Lives here (not in `@cluesmith/codev-types`) because it's *application + * policy* — the rule the UI applies when projecting a `string[]` of areas + * to a single grouping bucket. Co-locating the policy here prevents silent + * drift where the dashboard says "Auth" and vscode says "cross-cutting" + * for the same multi-area builder. + */ +export function resolvePrimaryArea(areas: string[]): string { + if (areas.includes('cross-cutting')) return 'cross-cutting'; + return areas[0] ?? 'Uncategorized'; +} +``` + +No new file — extends the existing `builder-helpers.ts`. + +### 5. Tests + +**`packages/codev/src/__tests__/github.test.ts`** — add `parseAreaLabels` to the import block, then a sibling `describe('parseAreaLabels', ...)` after the `parseLabelDefaults` block. Cases (per acceptance criteria): + +- empty array → `[]` +- `null` → `[]` (Gitea/Forgejo defensive coercion) +- `undefined` → `[]` (Gitea/Forgejo defensive coercion) +- empty string → `[]` (Gitea/Forgejo defensive coercion) +- single `area/auth` → `['auth']` +- mixed `area/*` + `type:*` + `priority:*` + bare → only `area/*` extracted, prefix stripped +- `area/cross-cutting` alongside other areas → present in result, alphabetically sorted with the rest +- multi-area, unsorted input → deduplicated and alphabetically sorted in output +- duplicate `area/auth` entries → single output +- bare `area` (no slash, just the word) → excluded (must have `/` separator) + +**`packages/core/__tests__/builder-helpers.test.ts`** (or equivalent — confirm test layout, may need creating) — `resolvePrimaryArea`: +- `[]` → `'Uncategorized'` +- `['auth']` → `'auth'` +- `['auth', 'core']` → `'auth'` (first alphabetical) +- `['cross-cutting']` → `'cross-cutting'` +- `['auth', 'cross-cutting', 'tower']` → `'cross-cutting'` (cross-cutting wins over alphabetical first) + +## Files to Change + +- `packages/codev/src/lib/github.ts` — add `parseAreaLabels` (~510, after `parseLabelDefaults`) +- `packages/codev/src/agent-farm/servers/overview.ts:19` — import `parseAreaLabels` +- `packages/codev/src/agent-farm/servers/overview.ts:35` — add `areas: string[]` to `BuilderOverview` +- `packages/codev/src/agent-farm/servers/overview.ts:95` — add `areas: string[]` to `BacklogItem` +- `packages/codev/src/agent-farm/servers/overview.ts:~578,~632,~666` — initialize `areas: []` in 3 `discoverBuilders` push sites +- `packages/codev/src/agent-farm/servers/overview.ts:736` — populate `areas: parseAreaLabels(issue.labels)` in `deriveBacklog` +- `packages/codev/src/agent-farm/servers/overview.ts:~870` — extend issue→builder enrichment to populate `b.areas` +- `packages/types/src/api.ts:130` — add `areas: string[]` to `OverviewBuilder` +- `packages/types/src/api.ts:187` — add `areas: string[]` to `OverviewBacklogItem` +- `packages/core/src/builder-helpers.ts` — add `resolvePrimaryArea` below `isIdleWaiting` +- `packages/codev/src/__tests__/github.test.ts` — add `parseAreaLabels` import + `describe` block with 10 cases listed above +- `packages/core/__tests__/builder-helpers.test.ts` (or wherever existing helper tests live; confirmed during implement) — add `resolvePrimaryArea` test block with 5 cases listed above + +**Estimated diff size**: ~80 lines of production code, ~80 lines of tests. ~12 files touched, but most edits are one or two lines. + +## Risks & Alternatives Considered + +### Risk: Cache shape discipline §B + +The issue body presents discipline §B (defensive `??= []` at cache serve-out, or restart-time invalidation) as a mandatory acceptance criterion. The current architecture makes this structurally satisfied — `OverviewCache` only holds raw forge responses, never derived shapes — so no defensive coercion is added. **Verified at `overview.ts:763-769`.** Recording this here so a reviewer who reads the issue and expects to see `??= []` somewhere in the diff knows where to look (and confirms it's not needed). + +If a future change *did* add a derived-shape cache (e.g. caching the full `OverviewData` between requests), the discipline would need to be re-applied at that point. Out of scope for this PIR. + +### Risk: Downstream consumer breakage + +Adding a required field to a wire contract is a forward-compatible change for new clients (always present) but a *breaking* change for any existing serialized payloads that lack the field. The only such surface in this codebase is the in-memory cache discussed above — no on-disk serialization of `OverviewBacklogItem` / `OverviewBuilder` exists (verified via `grep -rn "OverviewBacklogItem\|OverviewBuilder" packages/`, all hits are type-level only). Safe to land as required. + +### Alternative: `areas?: string[]` (optional) + +Rejected per discipline §A in the issue body. The required-with-default form forces every construction site to populate the field explicitly, which prevents the "I forgot one branch of `discoverBuilders` and it's `undefined` in prod" failure mode. The four consumer call sites (`BacklogList.tsx`, dashboard `api.ts`, vscode `backlog.ts`, vscode `builders.ts` — to be touched in #811 / #818) can rely on `areas.length` without null guards. + +### Alternative: parse `area/*` inside `parseLabelDefaults` + +Rejected. `parseLabelDefaults` returns `{type, priority}` — adding `areas` would couple two unrelated namespaces and force every caller to destructure an unused field. Separate helper keeps each parser single-purpose. + +### Open question (tracked separately): mixed-separator convention + +This PIR ships `area/*` on slash while keeping `type:*` and `priority:*` on colon, per the issue body's stated Kubernetes-alignment rationale. The resulting mixed-separator state across Codev's label namespaces is a legitimate engineering concern (cognitive load, two near-identical parsers, no principled rule for future namespaces) — but not one to resolve inside this PIR. + +Tracked as **#869** ("Label namespace separator: resolve mixed colon-vs-slash convention") with three options laid out (A: all-slash, B: all-colon, C: stay mixed) and the "web dashboard pathway compatibility" constraint flagged for verification. Whichever way #869 resolves, the changes from #819 are forward-compatible — the `parseAreaLabels` helper would either stay (option C), get its `area/` literal swapped for `area:` (option B), or get merged into a unified slash-based parser alongside renamed `type/` and `priority/` (option A). + +### Alternative: `resolvePrimaryArea` in `@cluesmith/codev-types` + +Rejected — types package is wire contracts only (per existing convention; see `packages/core/src/builder-helpers.ts:13-19` docstring on `IDLE_WAITING_THRESHOLD_MS` which spells this out). Policy / implementation goes in `@cluesmith/codev-core`. + +### Alternative: read GitHub Issue Types instead of labels + +Explicitly out of scope per the issue body — labels are the universal cross-forge primitive; Issue Types are GitHub-only. + +## Test Plan + +### Unit tests + +- `pnpm --filter @cluesmith/codev test src/__tests__/github.test.ts` — verifies the new `parseAreaLabels` block (10 cases). +- `pnpm --filter @cluesmith/codev-core test` — verifies `resolvePrimaryArea` (5 cases). + +### Integration / type-check + +- `pnpm -w build` — full workspace build. Must succeed. Required-field discipline means TypeScript will flag any unpopulated `areas` site at compile time, so a green build is strong evidence that all construction sites were touched. +- `grep -rn "areas:" packages/codev/src/agent-farm/servers/overview.ts` — manual check that all 3 `discoverBuilders` push sites + `deriveBacklog` + `getOverview` enrichment all populate the field. + +### Manual / dev-approval gate + +The reviewer at `dev-approval` should verify: + +1. `pnpm -w build` is green — confirms wire-contract field propagated to all four downstream consumers without compile errors. (Critically, this PIR adds the field but doesn't *yet* touch the consumers, so the build passing is the primary evidence that nothing existing broke.) +2. `pnpm --filter @cluesmith/codev test` — github.test.ts new block passes. +3. `pnpm --filter @cluesmith/codev-core test` — builder-helpers test passes. +4. Inspect `/api/overview` payload on a running Tower (`afx dev main`, then hit `http://localhost:/api/overview` or use the dashboard's network tab): + - Every `backlog[]` entry has `"areas": [...]` (may be `[]`). + - Every `builders[]` entry has `"areas": [...]` (may be `[]`). + - This issue itself (#819) carries `area/core` — when filtered against the live backlog, its entry should show `"areas": ["core"]`. +5. Optionally, manually exercise `resolvePrimaryArea`: + ```ts + import { resolvePrimaryArea } from '@cluesmith/codev-core'; + resolvePrimaryArea(['core']); // 'core' + resolvePrimaryArea(['cross-cutting', 'tower']); // 'cross-cutting' + resolvePrimaryArea([]); // 'Uncategorized' + ``` + +### Cross-platform / dashboard + +No UI changes in this PIR — the consumers (#811 backlog grouping, #818 builders tree grouping) ship in follow-up issues. The wire field landing here is invisible to end users until those consumers wire it through. + +### How to test locally at the `dev-approval` gate + +```bash +# Build everything +pnpm -w build + +# Unit tests +pnpm --filter @cluesmith/codev test src/__tests__/github.test.ts +pnpm --filter @cluesmith/codev-core test + +# Spin up Tower against this branch (optional — only needed to inspect live payload) +afx dev main +# Then in the dashboard or via curl, inspect /api/overview and look for "areas" on every backlog + builder entry. +``` diff --git a/codev/projects/819-core-parsearealabels-helper-fl/819-review-iter1-rebuttals.md b/codev/projects/819-core-parsearealabels-helper-fl/819-review-iter1-rebuttals.md new file mode 100644 index 00000000..8fbe85e8 --- /dev/null +++ b/codev/projects/819-core-parsearealabels-helper-fl/819-review-iter1-rebuttals.md @@ -0,0 +1,35 @@ +# PIR #819 — CMAP iteration 1 rebuttals + +## Gemini + +**Verdict in file**: SKIPPED (parsed by porch as REQUEST_CHANGES because the verdict isn't APPROVE/COMMENT). + +**Disposition**: Not a model verdict — infrastructure failure documented as a skip per explicit architect directive. No code or review changes are warranted. + +**Details**: Three consecutive `consult -m gemini --protocol pir --type impl --project-id 819` attempts exited code 1 in ~1.8–4 seconds with an opaque `[object Object]` error and produced no output file. The architect reproduced the failure independently and confirmed root cause: the `consult` CLI is hardcoded to call `gemini-3-pro-preview`, which Google has retired (`ModelNotFoundError`). The model-identifier bump is being tracked as a separate `area/consult` bug. + +**Architect directive (verbatim)**: + +> Architect direction: skip Gemini for this CMAP pass and advance to the pr gate. Root cause confirmed architect-side: Google has retired the gemini-3-pro-preview model that Codev's consult CLI is hardcoded to call (verified by reproducing the same 1.8s fast-fail and reading the dumped error report — ModelNotFoundError). Standing verdicts: Codex=COMMENT (addressed in commit 234e88bc), Claude=APPROVE. 2/2 favorable; Gemini's absence is infrastructure failure, not signal. Per PIR protocol, CMAP-2 is advisory and a missing verdict doesn't block escalation to the human at the pr gate. Override porch's CMAP-complete check and advance. Tracking the model identifier bump as a separate area/consult bug. + +The full directive is also preserved in `819-review-iter1-gemini.txt` for the audit trail. + +## Codex + +**Verdict in file**: COMMENT (not REQUEST_CHANGES). + +**Disposition**: Both COMMENT findings were valid and addressed in commit `234e88bc`: +1. Files Changed count in the review file was 9, actually 10 — review file itself was missing from the list. **Fixed**: count corrected to 10, review file added with `(+90 / -0) — this file`. +2. The `import { parseArea } from '@cluesmith/codev'` example in "How to Test Locally" was wrong — `parseArea` isn't exported from the package root. **Fixed**: rewrote the bullet to point at the unit-test file as the cleanest path for exercising edge cases. + +PR body was re-uploaded via `gh pr edit 876 --body-file ...` after the review file revision. + +## Claude + +**Verdict in file**: APPROVE. + +No rebuttal needed. + +## Effective outcome + +2/2 substantive verdicts favorable (Codex COMMENT addressed, Claude APPROVE). Gemini missing due to infrastructure failure, not signal. Per architect directive, advancing to `pr` gate. diff --git a/codev/projects/819-core-parsearealabels-helper-fl/status.yaml b/codev/projects/819-core-parsearealabels-helper-fl/status.yaml new file mode 100644 index 00000000..8bcc9c00 --- /dev/null +++ b/codev/projects/819-core-parsearealabels-helper-fl/status.yaml @@ -0,0 +1,29 @@ +id: '819' +title: core-parsearealabels-helper-fl +protocol: pir +phase: verified +plan_phases: [] +current_plan_phase: null +gates: + plan-approval: + status: approved + requested_at: '2026-05-26T05:47:28.053Z' + approved_at: '2026-05-27T04:00:38.067Z' + dev-approval: + status: approved + requested_at: '2026-05-27T04:09:25.639Z' + approved_at: '2026-05-27T04:33:16.753Z' + pr: + status: approved + requested_at: '2026-05-27T04:52:40.437Z' + approved_at: '2026-05-27T09:44:50.868Z' +iteration: 1 +build_complete: true +history: [] +started_at: '2026-05-26T05:43:05.270Z' +updated_at: '2026-05-27T09:45:24.546Z' +pr_history: + - phase: review + pr_number: 876 + branch: builder/pir-819 + created_at: '2026-05-27T04:35:50.096Z' diff --git a/codev/reviews/819-core-parsearealabels-helper-fl.md b/codev/reviews/819-core-parsearealabels-helper-fl.md new file mode 100644 index 00000000..1b1d1303 --- /dev/null +++ b/codev/reviews/819-core-parsearealabels-helper-fl.md @@ -0,0 +1,103 @@ +# PIR Review: `parseArea` helper + `area` field on `BacklogItem` and `BuilderOverview` + +Fixes #819 + +## Summary + +Adds the `parseArea` helper (in `packages/codev/src/lib/github.ts`) that extracts a single `area/*` value from an issue's labels, and threads that value as a required `area: string` field through `BacklogItem` / `BuilderOverview` (server-internal) and `OverviewBacklogItem` / `OverviewBuilder` (wire contracts). This is pure scaffolding for two follow-up consumers — #811 (backlog grouping by area) and #818 (builders-tree grouping by area). No user-visible behavior change; the new field appears on every `/api/overview` payload but no UI surface reads it yet. + +## Files Changed + +Computed via `git diff --stat origin/main...HEAD` (branch-only — excludes commits picked up from main during a mid-PIR merge): + +- `codev/plans/819-core-parsearealabels-helper-fl.md` (+279 / -0) +- `codev/projects/819-core-parsearealabels-helper-fl/status.yaml` (+27 / -0) — porch-managed, not hand-edited +- `codev/reviews/819-core-parsearealabels-helper-fl.md` (+90 / -0) — this file +- `codev/state/pir-819_thread.md` (+79 / -0) +- `packages/codev/src/__tests__/github.test.ts` (+66 / -0) +- `packages/codev/src/agent-farm/servers/overview.ts` (+38 / -5) +- `packages/codev/src/lib/github.ts` (+29 / -0) +- `packages/core/src/constants.ts` (+9 / -0) +- `packages/types/src/api.ts` (+15 / -0) +- `packages/vscode/src/test/builders.test.ts` (+1 / -0) + +Total: 10 files, +628 / -5. + +## Commits + +Implementation: + +- `da040105` [PIR #819] Add parseAreaLabels helper + unit tests +- `fc8b3001` [PIR #819] Add resolvePrimaryArea helper + tests +- `763d8170` [PIR #819] Wire areas[] through BacklogItem and BuilderOverview +- `6e90f5c6` [PIR #819] Add areas[] to OverviewBuilder and OverviewBacklogItem wire types +- `65739680` [PIR #819] Thread: log implement-phase progress + +Design revisions at dev-approval (two rounds — see *Things to Look At*): + +- `114aee99` [PIR #819] Revise: parseArea returns single string (drop array shape) +- `df442ca8` [PIR #819] Revise: drop resolvePrimaryArea (parser projects to single area now) +- `5c8800f8` [PIR #819] Revise: BacklogItem.area + BuilderOverview.area (single string) +- `7cf2d8cb` [PIR #819] Revise: OverviewBuilder.area + OverviewBacklogItem.area wire fields +- `2aa42101` [PIR #819] Document design revision in plan + thread +- `f638e84c` [PIR #819] Revise: drop cross-cutting privilege (parser is policy-free about label names) +- `62df91f7` [PIR #819] Extract 'Uncategorized' to UNCATEGORIZED_AREA constant in @cluesmith/codev-core + +## Test Results + +- `pnpm -w build`: ✓ pass (full workspace incl. types, core, codev, dashboard) +- `pnpm --filter @cluesmith/codev test src/__tests__/github.test.ts`: ✓ pass (66 tests, 10 new for `parseArea`) +- `pnpm --filter @cluesmith/codev test` (full suite): ✓ pass (3149 tests, 13 pre-existing skips, no regressions introduced) +- `pnpm --filter codev-vscode run check-types`: ✓ pass (verifies the wire-type addition propagates through vscode without compile errors) +- Manual verification (at `dev-approval` gate): the human inspected the running implementation and approved. + +## Architecture Updates + +No changes to `codev/resources/arch.md`. The PR adds a parser + a required field on existing wire-contract shapes; it doesn't introduce new module boundaries, new caching layers, new endpoints, or new architectural patterns. The cache discipline question raised in the issue body (defensive `??= []` at serve-out) was investigated and found to be structurally satisfied — `OverviewCache` already holds only raw forge responses, never derived `BacklogItem` / `BuilderOverview` shapes, so the "stale cache entry missing `area`" failure mode isn't reachable. This is a *finding about existing architecture*, not a *change to it*, so it lives in this review and the implement-phase thread rather than in `arch.md`. + +## Lessons Learned Updates + +No additions to `codev/resources/lessons-learned.md`, but two durable principles emerged that landed in the project's memory system instead (which is the appropriate home for AI-collaboration-shape lessons): + +1. **Framework code must be policy-free about specific label values.** The initial parser implementation privileged `area/cross-cutting` (returning it preferentially when present). The user pushed back — Codev framework code shouldn't bake in semantic conventions about specific label names; teams using Codev decide their own labeling conventions. Stripped the privilege; added a no-privilege regression-guard test. Captured as [`feedback_framework_neutral_on_label_semantics.md`](/.claude/projects/-Users-amrmohamed-repos-cluesmith-codev/memory/feedback_framework_neutral_on_label_semantics.md). + +2. **Wire-shape "permissiveness then projection" is a smell.** The initial design returned `string[]` from `parseAreaLabels` and then collapsed it to a single bucket via a separate `resolvePrimaryArea` helper at the UI boundary — two operations cancelling each other out. The cleaner shape is for the parser to do the projection once at the boundary and return `string` directly, symmetric with `parseLabelDefaults`'s single-string `type` / `priority` returns. This isn't a generalizable arch principle (it's specific to this case), so it's documented in the plan revision notes and this review rather than `lessons-learned.md`. + +`codev/resources/arch.md` and `codev/resources/lessons-learned.md` would be updated as a follow-up by the MAINTAIN protocol's quarterly sweep if either principle generalizes further; nothing about this PR warrants forcing them into the docs now. + +## Things to Look At During PR Review + +1. **Two rounds of design revision during the implement phase** — visible in the commit history. The original implementation followed the issue body verbatim (`areas: string[]` + `resolvePrimaryArea` helper, with `cross-cutting` privilege). The human at dev-approval flagged two issues and the design collapsed to single-string at the parser with no special-cased label names. The final shape is meaningfully smaller and cleaner than what the issue body proposed — see the plan file's revision note for the full reasoning. + +2. **The `discoverBuilders` defaulting pattern.** Three `builders.push({...})` sites in `discoverBuilders` each set `area: UNCATEGORIZED_AREA`. The `getOverview` enrichment loop then overrides this with `parseArea(issue.labels)` for builders whose issue is in the cached issue list. Builders with `issueId: null` (soft-mode / task-mode) keep the default. Worth a glance to confirm the defaulting + enrichment flow is what you'd expect; one alternative would be to make the field `area: string | null` and let the UI render its own fallback, but `'Uncategorized'` as a server-side default keeps consumers free of null-handling. + +3. **`OverviewCache` does not cache derived shapes** — verified at `overview.ts:763-769`. Only raw `ForgePR[]` / `ForgeIssueListItem[]` are cached. `BacklogItem` and `BuilderOverview` are rebuilt fresh on every `getOverview` call, so `parseArea` runs against current labels every time. The "stale cache entry missing `area`" concern from the issue body §B isn't reachable in the current architecture. If a future change ever adds a derived-shape cache, that discipline would need to be re-applied at that point. + +4. **Two follow-up issues filed during this PIR:** + - **#869** — "Label namespace separator: resolve mixed colon-vs-slash convention" (the mixed-separator state across `type:` / `priority:` / `area/` is a real engineering concern worth resolving globally; this PIR ships the slash convention as-spec'd; #869 lays out options A/B/C for the wider question). + - **#875** — "Collapse duplicate `Overview*` / `*Overview` types" (five paired interfaces across `packages/codev/src/agent-farm/servers/overview.ts` and `packages/types/src/api.ts` are structurally identical; this PIR's `area` addition had to land in both halves of the pair, demonstrating the drift cost; proposed fix is to make `@cluesmith/codev-types` the single source of truth). + +5. **`parseArea` projection rule**: first-alphabetical wins, no label name is privileged, `'Uncategorized'` fallback. The no-privilege test explicitly uses `area/cross-cutting` as fixture data to prove the parser doesn't treat it specially — that's intentional and is the regression guard against re-introducing the privilege. + +## 3-Way Consultation Dispositions + +Single advisory pass (PIR's `max_iterations: 1`). Verdicts: + +- **Claude**: APPROVE. +- **Codex**: COMMENT. Two accuracy findings on this review file: (1) the Files Changed list underreported (was 9, actually 10 — review file itself was missing); (2) the local-test instruction recommended `import { parseArea } from '@cluesmith/codev'` but `parseArea` isn't exported from the package root. **Both addressed** in this same review file revision — see the corrected Files Changed list and the updated "How to Test Locally" section. +- **Gemini**: failed to produce a verdict (`consult` exited code 1 with opaque `[object Object]` error on three attempts; no output file written). Surfaced to the architect; no model verdict obtained for this PR. + +No `REQUEST_CHANGES` findings. Codex's COMMENT findings were minor accuracy issues on the review file (not on the code) and were corrected in place. + +## How to Test Locally + +For reviewers pulling the branch: + +- **View diff**: VSCode sidebar → right-click builder `pir-819` → **Review Diff** (auto-detects the repo's default branch). Or `git diff main...HEAD`. +- **Run dev server**: VSCode sidebar → **Run Dev Server**, or `afx dev pir-819` from a shell. +- **What to verify**: + - `pnpm -w build` is green. + - `pnpm --filter @cluesmith/codev test src/__tests__/github.test.ts` — 66 tests pass, including the new `parseArea` block. + - Hit `/api/overview` on the running dev server: `curl http://localhost:/api/overview | jq '.backlog[0] | {id, area}'`. Every backlog entry should have a populated `area` string. This issue (#819) is labeled `area/core`, so its entry should show `"area": "core"`. + - Same for builders: `curl http://localhost:/api/overview | jq '.builders[] | {id, issueId, area}'`. Builders with an `issueId` matching a labeled issue inherit that issue's area; builders without an issue (soft-mode / task-mode) show `"area": "Uncategorized"`. + - `parseArea` is currently internal to the codev server (no public re-export from the package root). To exercise edge cases directly, the cleanest path is the unit test file at `packages/codev/src/__tests__/github.test.ts` — run `pnpm --filter @cluesmith/codev test src/__tests__/github.test.ts`, or add a temporary case there. The 10 included cases already cover `null`, `''`, `[{name: 'area/auth'}, {name: 'area/cross-cutting'}]` (returns `'auth'` — first alphabetical, cross-cutting not privileged), and the other defensive paths. diff --git a/codev/state/pir-819_thread.md b/codev/state/pir-819_thread.md new file mode 100644 index 00000000..2af67850 --- /dev/null +++ b/codev/state/pir-819_thread.md @@ -0,0 +1,110 @@ +# pir-819 thread + +## Plan phase — 2026-05-26 + +Drafted `codev/plans/819-core-parsearealabels-helper-fl.md` and pushed. + +### Scope this PIR + +Pure scaffolding for the `area/*` namespace — no UI consumers in this issue (those land in #811 backlog grouping and #818 builders tree grouping). Lands: + +- `parseAreaLabels` helper in `packages/codev/src/lib/github.ts` +- `areas: string[]` field on `BacklogItem`, `BuilderOverview` (server-internal) and `OverviewBacklogItem`, `OverviewBuilder` (wire contracts) — required, never undefined +- `resolvePrimaryArea` policy helper in `packages/core/src/builder-helpers.ts` +- Unit tests in `packages/codev/src/__tests__/github.test.ts` + new builder-helpers test + +### Non-obvious finding for the reviewer + +Issue body §B (defensive `??= []` at cache serve-out OR cache invalidation on restart) is **already structurally satisfied** — `OverviewCache` only holds raw forge responses (`ForgePR[]`, `ForgeIssueListItem[]`), never derived `BacklogItem` / `BuilderOverview` objects. Those shapes are reconstructed fresh from raw cache entries on every `getOverview` call. No code added for §B; called out in the plan's Risks section so the reviewer knows where to look (and isn't surprised by its absence in the diff). + +Awaiting `plan-approval`. + +## Plan phase — follow-up + +User flagged the mixed-separator state (`type:` / `priority:` colon vs `area/` slash) as a real concern. Chose option D: ship #819 as the spec calls for, file a follow-up to relitigate separators globally. Filed **#869** ("Label namespace separator: resolve mixed colon-vs-slash convention") — unassigned, `area/core` label, lays out options A (all-slash), B (all-colon), C (stay mixed) plus a "verify the web-dashboard pathway compatibility constraint" callout. Plan revised to reference #869 under Risks & Alternatives → Open question. + +## Implement phase — 2026-05-27 + +Plan approved. Implemented in four commits matching the plan's logical units: + +- `da040105` — `parseAreaLabels` helper + 10 unit tests (no labels, null/undefined/empty-string defensive paths, single area, mixed-namespace, alphabetical sort, dedup, cross-cutting alongside others, bare `area` rejected, `area:` separator-typo rejected). +- `fc8b3001` — `resolvePrimaryArea` helper in `packages/core/src/builder-helpers.ts` + 5 tests in `packages/vscode/src/test/builders.test.ts` (existing home of `isIdleWaiting` tests). +- `763d8170` — wire `areas[]` through `BacklogItem` and `BuilderOverview`: type declarations, `areas: []` init at all 3 `discoverBuilders` push sites, `parseAreaLabels(issue.labels)` populate in `deriveBacklog`, parallel `issueAreasMap` join in `getOverview` enrichment loop (refactored alongside existing `issueTitleMap` join for clarity). +- `6e90f5c6` — wire-contract `areas: string[]` field on `OverviewBuilder` and `OverviewBacklogItem` in `packages/types/src/api.ts`. + +Verification: +- `pnpm -w build` — green (full workspace incl. dashboard) +- `pnpm --filter codev-vscode run check-types` — green (covers `builders.test.ts` with `resolvePrimaryArea` tests) +- `pnpm --filter @cluesmith/codev test src/__tests__/github.test.ts` — 66 tests pass (10 new) +- `pnpm --filter @cluesmith/codev test` (full) — 3149 tests pass, 13 pre-existing skips, no regressions + +Branch pushed. Awaiting `dev-approval`. + +## Implement phase — design revision (2026-05-27) + +User caught the design smell during dev-approval review: `BacklogItem.areas: string[]` permitted multi-area at the data layer while `resolvePrimaryArea` immediately collapsed it to a single bucket at the UI boundary. Two operations cancelling each other out — pointing to a misalignment between the data shape and the project convention ("one `area/` per issue; `area/cross-cutting` is the explicit multi-area marker", per `feedback_single_area_per_issue.md` memory). The array shape was inherited from the issue body without questioning whether it matched the project's existing convention. + +Revised to single-area at the parser (option B from the conversation): + +- `1142aee99` — `parseAreaLabels: (...) => string[]` → `parseArea: (...) => string`. Parser now projects once at the boundary: `'cross-cutting'` if present, else first alphabetical, else `'Uncategorized'`. Symmetric with `parseLabelDefaults`'s single-string `type` / `priority` returns. Tests rewritten for single-string outputs (12 cases, +1 from the previous 10). +- `df442ca8` — deleted `resolvePrimaryArea` from `packages/core/src/builder-helpers.ts` and its 5-case suite from `packages/vscode/src/test/builders.test.ts`. The function had no callers — its job is now done at the parser. +- `5c8800f8` — `BacklogItem.areas: string[]` → `BacklogItem.area: string`; same for `BuilderOverview`. 3 `discoverBuilders` push sites now init `area: 'Uncategorized'`. `getOverview` enrichment loop renamed `issueAreasMap` → `issueAreaMap`. +- `7cf2d8cb` — same shape change on the wire-contract types in `packages/types/src/api.ts`. + +Plan header updated with a revision note explaining the change. + +Re-verification: build ✓, github tests (67 pass, +1) ✓, vscode check-types ✓. Net diff vs original revised design: ~30 LOC smaller (deleted helper + simpler test cases offset the unchanged parser body). + +Still at `dev-approval`. + +## Implement phase — cross-cutting privilege removed (2026-05-27) + +User flagged that the parser still baked in a semantic convention about a specific label name (`if (areas.includes('cross-cutting')) return 'cross-cutting'`). Stripped: parser is now policy-free about which `area/*` value any team uses. First alphabetical wins; `'Uncategorized'` fallback. Codev framework code shouldn't impose its own conventions on teams using Codev — they pick their own labeling semantics. + +Changes: +- Removed the `cross-cutting` privilege line from `parseArea`. +- Dropped two `cross-cutting`-specific test cases; added one explicit no-privilege regression-guard that uses `area/cross-cutting` as fixture data alongside other areas and asserts first-alphabetical wins regardless. +- Stripped docstring references to `cross-cutting` from `parseArea`, `BacklogItem.area`, `BuilderOverview.area` (server-internal), and `OverviewBacklogItem.area` / `OverviewBuilder.area` (wire-contract). Docstrings now only describe the mechanical behavior ("first-alphabetical wins; `'Uncategorized'` when no `area/*` labels"). + +Re-verification: build ✓, github tests (66 pass, net −1 from previous since two cross-cutting tests collapsed into one no-privilege guard) ✓. + +Still at `dev-approval`. + +## Implement phase — `'Uncategorized'` extracted to shared constant (2026-05-27) + +User flagged: the `'Uncategorized'` literal was hardcoded in two places (parser fallback in `parseArea`, and the three `discoverBuilders` builder-init sites). Extracted to `UNCATEGORIZED_AREA` in `packages/core/src/constants.ts` (alongside `DEFAULT_TOWER_PORT`, `AGENT_FARM_DIR`). Both `github.ts` and `overview.ts` now import and reference the constant. Downstream UI consumers (dashboard, vscode) that ever want to filter/match against the default can import the same constant — single source of truth. + +Build ✓, tests still 66/66 ✓. + +Still at `dev-approval`. + +## Review phase — 2026-05-27 + +`dev-approval` approved. Wrote `codev/reviews/819-core-parsearealabels-helper-fl.md` (commit `12f98fca`) with Summary, Files Changed, Commits, Test Results, Architecture Updates (none — no new boundaries), Lessons Learned Updates (no `lessons-learned.md` edits; the two principles surfaced went to the project's memory system instead via `feedback_framework_neutral_on_label_semantics.md`), Things to Look At, and How to Test Locally sections. + +Opened PR #876 against main using the review file as the body. Recorded with porch (`porch done 819 --pr 876 --branch builder/pir-819`). + +**Mid-PIR merge from origin/main**: user flagged a merge conflict. Fetched and merged `origin/main` (de4b060d) into the branch. Conflicts in two files, both stemming from the same upstream change — bugfix #872 added a `prReady: boolean` field to `OverviewBuilder` at the same position my PIR added `area: string`. Both fields are independent; resolved by keeping both. Three more conflicts in `discoverBuilders` push sites (same shape — `area: UNCATEGORIZED_AREA` and `prReady: false`/`derivePrReady(parsed)` both added at end of each push site). Resolved keeping both. Merge commit `6254a9c3`; pushed. + +Re-verification post-merge: `pnpm -w build` ✓, `pnpm --filter @cluesmith/codev test src/__tests__/github.test.ts` ✓ (66 tests), `pnpm --filter @cluesmith/codev test` (full) ✓ (3172 tests pass, +23 from bugfix-872's new tests, 13 pre-existing skips, no regressions). + +**3-way consultation results** (PIR single-pass, `max_iterations: 1`): +- **Claude**: APPROVE. +- **Codex**: COMMENT (two accuracy findings on the review file — files-changed count was 9 not 10 because I forgot to include the review file itself, and the `import { parseArea } from '@cluesmith/codev'` example was wrong since `parseArea` isn't exported from the package root). Both addressed in commit `234e88bc`. PR body re-uploaded. +- **Gemini**: persistently failing — three consecutive `consult -m gemini` attempts exited code 1 with opaque `[object Object]` error in ~2-4s, no output file written. The `[warn] Failed to extract usage for gemini: Unexpected end of JSON input` suggests Gemini API returned an empty/malformed body. Not a code issue — looks like an API/auth/quota problem with the `consult` CLI's Gemini path. Architect notified via `afx send` for direction (skip / retry later / investigate). + +Porch currently blocking on Gemini producing an output. PR gate not yet pending — `porch next 819` reports `status: tasks` requesting another Gemini attempt. + +### Architect directive — Gemini skip + +Architect responded with explicit directive: skip Gemini, advance to pr gate. Root cause confirmed architect-side — `consult` CLI is hardcoded to `gemini-3-pro-preview` which Google has retired (`ModelNotFoundError`). Tracking the model-identifier bump as a separate `area/consult` bug. Standing: Codex=COMMENT addressed, Claude=APPROVE, 2/2 favorable; Gemini's absence is infra failure not signal. + +Executed: +- Wrote `819-review-iter1-gemini.txt` with `VERDICT: SKIPPED` + the architect's directive verbatim for audit trail. +- Porch parsed SKIPPED as REQUEST_CHANGES (anything non-APPROVE/COMMENT does); wrote `819-review-iter1-rebuttals.md` documenting the architect-directed skip + Codex's addressed COMMENT + Claude's APPROVE. +- `porch done 819` → checks green → `porch gate 819` → **pr gate now pending**. +- Notified architect via `afx send` that pr gate is pending and ready for human approval. + +Consult artifacts (`819-review-iter1-*.txt`, `-rebuttals.md`) live in the worktree only — they're gitignored per codev convention; the audit trail stays in the worktree until cleanup. + +Awaiting `pr` gate approval; on approval, will merge via `gh pr merge --merge` then `porch done 819 --merged 876`. diff --git a/packages/codev/src/__tests__/github.test.ts b/packages/codev/src/__tests__/github.test.ts index d4268498..23bb1458 100644 --- a/packages/codev/src/__tests__/github.test.ts +++ b/packages/codev/src/__tests__/github.test.ts @@ -8,6 +8,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { parseLinkedIssue, parseLabelDefaults, + parseArea, } from '../lib/github.js'; // Mock forge.js for concept command routing tests @@ -291,6 +292,71 @@ describe('parseLabelDefaults', () => { }); }); +describe('parseArea', () => { + it('returns "Uncategorized" for an empty label array', () => { + expect(parseArea([])).toBe('Uncategorized'); + }); + + it('returns "Uncategorized" for null labels (Gitea/Forgejo defensive path)', () => { + expect(parseArea(null)).toBe('Uncategorized'); + }); + + it('returns "Uncategorized" for undefined labels (Gitea/Forgejo defensive path)', () => { + expect(parseArea(undefined)).toBe('Uncategorized'); + }); + + it('returns "Uncategorized" for empty-string labels (Gitea/Forgejo defensive path)', () => { + expect(parseArea('')).toBe('Uncategorized'); + }); + + it('extracts a single area/* label with the prefix stripped', () => { + expect(parseArea([{ name: 'area/auth' }])).toBe('auth'); + }); + + it('ignores non-area labels in a mixed set', () => { + expect(parseArea([ + { name: 'area/core' }, + { name: 'type:bug' }, + { name: 'priority:high' }, + { name: 'bug' }, + ])).toBe('core'); + }); + + it('picks first alphabetical for multi-area input', () => { + expect(parseArea([ + { name: 'area/tower' }, + { name: 'area/core' }, + { name: 'area/porch' }, + ])).toBe('core'); + }); + + it('does not privilege any specific area name (alphabetical wins; no special-cased label)', () => { + // Regression guard: the parser must remain policy-free about what any + // given area name means. Teams using Codev decide their own conventions. + expect(parseArea([ + { name: 'area/auth' }, + { name: 'area/cross-cutting' }, + { name: 'area/tower' }, + ])).toBe('auth'); + }); + + it('deduplicates repeated area/* labels before picking', () => { + expect(parseArea([ + { name: 'area/tower' }, + { name: 'area/tower' }, + { name: 'area/core' }, + ])).toBe('core'); + }); + + it('does not match bare "area" without the slash', () => { + expect(parseArea([{ name: 'area' }])).toBe('Uncategorized'); + }); + + it('does not match area: with a colon (separator-typo)', () => { + expect(parseArea([{ name: 'area:core' }])).toBe('Uncategorized'); + }); +}); + // ============================================================================= // Forge concept command routing tests // ============================================================================= diff --git a/packages/codev/src/agent-farm/servers/overview.ts b/packages/codev/src/agent-farm/servers/overview.ts index a4c1e0da..249e0e8d 100644 --- a/packages/codev/src/agent-farm/servers/overview.ts +++ b/packages/codev/src/agent-farm/servers/overview.ts @@ -9,6 +9,7 @@ import fs from 'node:fs'; import path from 'node:path'; +import { UNCATEGORIZED_AREA } from '@cluesmith/codev-core/constants'; import { fetchPRList, fetchIssueList, @@ -17,6 +18,7 @@ import { fetchCurrentUser, parseLinkedIssue, parseLabelDefaults, + parseArea, } from '../../lib/github.js'; import type { ForgePR, ForgeIssueListItem } from '../../lib/github.js'; import { loadProtocol } from '../../commands/porch/protocol.js'; @@ -80,6 +82,16 @@ export interface BuilderOverview { * an inline attribution tag when the workspace hosts more than one architect. */ spawnedByArchitect: string | null; + /** + * Single `area/*` value for this builder's issue, projected via + * `parseArea` (first-alphabetical wins; `'Uncategorized'` when the + * builder has no issue or the issue has no `area/*` labels). Populated + * by `getOverview` via the issue-cache join after `discoverBuilders` + * returns — `discoverBuilders` itself sets it to `'Uncategorized'` since + * it has no access to the issue payload. Consumed by the builders-tree + * grouping in #818 and the equivalent dashboard view. + */ + area: string; /** * Canonical "PR is waiting on a human reviewer" signal (Issue #872). True the * moment porch transitions out of the CMAP-emitting state for the PR-creating @@ -110,6 +122,13 @@ export interface BacklogItem { url: string; type: string; priority: string; + /** + * Single `area/*` value for this issue, projected via `parseArea` + * (first-alphabetical wins; `'Uncategorized'` when the issue has no + * `area/*` labels). Consumed by the backlog grouping in #811 and the + * equivalent vscode view. + */ + area: string; hasSpec: boolean; hasPlan: boolean; hasReview: boolean; @@ -649,6 +668,7 @@ export function discoverBuilders(workspaceRoot: string): BuilderOverview[] { idleMs: 0, lastDataAt: null, spawnedByArchitect: null, + area: UNCATEGORIZED_AREA, prReady: false, }); continue; @@ -705,6 +725,7 @@ export function discoverBuilders(workspaceRoot: string): BuilderOverview[] { idleMs: computeIdleMs(parsed), lastDataAt: null, spawnedByArchitect: null, + area: UNCATEGORIZED_AREA, prReady: derivePrReady(parsed), }); found = true; @@ -738,6 +759,7 @@ export function discoverBuilders(workspaceRoot: string): BuilderOverview[] { idleMs: 0, lastDataAt: null, spawnedByArchitect: null, + area: UNCATEGORIZED_AREA, prReady: false, }); } @@ -796,6 +818,7 @@ export function deriveBacklog( url: issue.url, type, priority, + area: parseArea(issue.labels), hasSpec: !!specFile, hasPlan: !!planFile, hasReview: !!reviewFile, @@ -918,13 +941,18 @@ export class OverviewCache { } else { backlog = deriveBacklog(issues, workspaceRoot, activeBuilderIssues, prLinkedIssues); - // Enrich builder titles from GitHub issue titles - // (status.yaml stores a slug, not the human-readable title) + // Enrich builder titles + area from the cached issue list. + // (status.yaml stores a slug, not the human-readable title; and + // discoverBuilders has no access to the issue payload, so area + // starts as 'Uncategorized' and gets filled here.) const issueTitleMap = new Map(issues.map(i => [String(i.number), i.title])); + const issueAreaMap = new Map(issues.map(i => [String(i.number), parseArea(i.labels)])); for (const b of builders) { - if (b.issueId !== null && issueTitleMap.has(b.issueId)) { - b.issueTitle = issueTitleMap.get(b.issueId)!; - } + if (b.issueId === null) continue; + const title = issueTitleMap.get(b.issueId); + if (title) b.issueTitle = title; + const area = issueAreaMap.get(b.issueId); + if (area) b.area = area; } } diff --git a/packages/codev/src/lib/github.ts b/packages/codev/src/lib/github.ts index 5c8bc927..46e464f8 100644 --- a/packages/codev/src/lib/github.ts +++ b/packages/codev/src/lib/github.ts @@ -9,6 +9,7 @@ * @see codev/specs/589-non-github-repository-support.md */ +import { UNCATEGORIZED_AREA } from '@cluesmith/codev-core/constants'; import { executeForgeCommand, type ForgeConfig } from './forge.js'; import { getRepoInfo } from './team-github.js'; import type { IssueViewResult, PrListItem, IssueListItem } from './forge-contracts.js'; @@ -505,3 +506,31 @@ export function parseLabelDefaults( priority: priorityLabels[0] || 'medium', }; } + +/** + * Extract the single `area/*` value for an issue. Symmetric with + * `parseLabelDefaults`'s single-string `type` / `priority` returns. + * + * Resolution order: + * - the first alphabetical `area/*` value (no label name is privileged — + * the parser is policy-free about what any particular area means; teams + * using Codev decide their own labeling conventions) + * - `'Uncategorized'` when no `area/*` labels are present + * + * Mirrors `parseLabelDefaults`'s defensive non-array coercion: Gitea/Forgejo + * return `""` or `null` for empty labels instead of `[]`. + * + * The slash separator (vs `type:` / `priority:`'s colon) is intentional; + * see #869 for the broader namespace-separator discussion. + */ +export function parseArea( + labels: Array<{ name: string }> | null | undefined | string, +): string { + const names = Array.isArray(labels) ? labels.map(l => l.name) : []; + const areas = [...new Set( + names + .filter(n => n.startsWith('area/')) + .map(n => n.slice(5)), + )].sort(); + return areas[0] ?? UNCATEGORIZED_AREA; +} diff --git a/packages/core/src/constants.ts b/packages/core/src/constants.ts index 0caaf88e..2a9659f8 100644 --- a/packages/core/src/constants.ts +++ b/packages/core/src/constants.ts @@ -3,3 +3,12 @@ import { homedir } from 'node:os'; export const DEFAULT_TOWER_PORT = 4100; export const AGENT_FARM_DIR = resolve(homedir(), '.agent-farm'); + +/** + * Fallback `area` value emitted by the server when an issue or builder has + * no `area/*` label (or — for builders — no associated issue). The single + * source of truth so the parser default, the server-side initializer for + * builders pending issue-cache enrichment, and any downstream UI filter or + * matcher all agree on the literal. + */ +export const UNCATEGORIZED_AREA = 'Uncategorized'; diff --git a/packages/types/src/api.ts b/packages/types/src/api.ts index 8065fea6..6efd9540 100644 --- a/packages/types/src/api.ts +++ b/packages/types/src/api.ts @@ -172,6 +172,14 @@ export interface OverviewBuilder { * hosts more than one architect. */ spawnedByArchitect: string | null; + /** + * Single `area/*` value for this builder's issue, projected via + * `parseArea` (first-alphabetical wins; `'Uncategorized'` when the + * builder has no issue or the issue has no `area/*` labels). + * Required-with-default — never `undefined`. Consumed by the + * builders-tree grouping in #818 and the equivalent dashboard view. + */ + area: string; /** * Canonical "PR is waiting on a human reviewer" signal (Issue #872). True the * moment porch transitions out of the CMAP-emitting state for the PR-creating @@ -203,6 +211,13 @@ export interface OverviewBacklogItem { url: string; type: string; priority: string; + /** + * Single `area/*` value for this issue, projected via `parseArea` + * (first-alphabetical wins; `'Uncategorized'` when the issue has no + * `area/*` labels). Required-with-default — never `undefined`. Consumed + * by the backlog grouping in #811 and the equivalent vscode view. + */ + area: string; hasSpec: boolean; hasPlan: boolean; hasReview: boolean; diff --git a/packages/vscode/src/test/builders.test.ts b/packages/vscode/src/test/builders.test.ts index e6805d72..89019e31 100644 --- a/packages/vscode/src/test/builders.test.ts +++ b/packages/vscode/src/test/builders.test.ts @@ -107,3 +107,4 @@ suite('orderForDisplay', () => { assert.strictEqual(orderForDisplay(bs, NOW).length, bs.length); }); }); +