Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 96 additions & 0 deletions codev/plans/885-vscode-capitalize-area-group-h.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
# PIR Plan: Capitalize area group header labels

## Understanding

After #811 (backlog grouping) and #818 (builders-tree grouping) shipped, the area group headers in both trees render area names verbatim from the wire (`OverviewBacklogItem.area` / `OverviewBuilder.area`). Since GitHub `area/*` labels are lowercase by convention, headers come out as `vscode (12)`, `tower (4)`, `porch (3)` — visually inconsistent next to the `Uncategorized (8)` fallback header, which is PascalCase by virtue of the `UNCATEGORIZED_AREA` constant value.

The wire value and the matcher sentinel (`UNCATEGORIZED_AREA = 'Uncategorized'`) are the canonical strings and must not change — only the *displayed* label changes.

Where the label is built today: `packages/vscode/src/views/area-group-tree-item.ts:23`, which forwards the raw `areaName` into `super(\`${areaName} (${count})\`, ...)`. That single line is the entire bug surface — both `BacklogGroupTreeItem` and `BuilderGroupTreeItem` route through it.

## Proposed Change

Apply **title-case with separator-to-space normalization** to the displayed label inside `AreaGroupTreeItem`'s constructor. Split on `-`, `_`, and whitespace; capitalize the first character of each word; rejoin with a single space.

```ts
export function formatAreaForDisplay(area: string): string {
return area
.split(/[-_\s]+/)
.filter(w => w.length > 0)
.map(w => w.charAt(0).toUpperCase() + w.slice(1))
.join(' ');
}
```

Expected outputs:

- `vscode` → `Vscode`
- `tower` → `Tower`
- `cross-cutting` → `Cross Cutting`
- `front_end` → `Front End`
- `Uncategorized` → `Uncategorized` (no-op on the fallback sentinel)

The raw `areaName` field, the `id`, and the `contextValue` keep using the wire value verbatim — only the human-visible TreeItem label is transformed.

**Why this rule**:

- Codev's actual `area/*` set is `vscode, tower, porch, consult, panel, terminal, config, core, docs, cross-cutting`. None are real acronyms. The multi-word case (`cross-cutting`) is the most visible difference between rules, and `Cross Cutting` reads as a proper category name instead of a tag ID — meaningfully better than `Cross-cutting` or `Cross-Cutting`.
- The same rule applied to `Uncategorized` is a no-op (single-word, first char already upper), so a single uniform path covers every area including the sentinel — no special-case branch.
- Single-word acronyms (`api`, `ui`, `cli`) still mangle to `Api`, `Ui`, `Cli`. Codev's set has none today; if a downstream repo surfaces real acronym pain, the helper signature stays single-string-in, single-string-out, so adding a `codev.areaDisplayNames: Record<string, string>` config override on top is a clean non-breaking extension.
- Framework-neutrality: this is a purely structural rule (separator handling + capitalize first letter). No hardcoded list of "known acronyms" — that would privilege specific label values inside framework code and conflict with the principle that teams using codev decide their own label semantics.

**Helper location**: exported from `packages/core/src/area-grouping.ts`. Sits next to `groupByArea` — same concern, no vscode dep, reusable by future consumers (e.g. dashboard equivalent of these views) without a second migration. The subpath `@cluesmith/codev-core/area-grouping` is already exported, so no `packages/core/package.json` change.

## Files to Change

- `packages/core/src/area-grouping.ts` — add `export function formatAreaForDisplay(area: string): string`. Two lines plus a brief JSDoc explaining the "wire stays raw; this is display-only" contract.
- `packages/vscode/src/views/area-group-tree-item.ts:23` — wrap `areaName` in the call site: `super(\`${formatAreaForDisplay(areaName)} (${count})\`, collapsibleState)`. Add the import at top.
- `packages/vscode/src/test/area-grouping.test.ts` — append a `suite('formatAreaForDisplay', ...)` block covering: lowercase single-word area → capitalized first char; hyphenated `'cross-cutting'` → `'Cross Cutting'`; underscored `'front_end'` → `'Front End'`; mixed/multi-word; `'Uncategorized'` → unchanged; empty string → empty string (defensive — never expected, but cheap to lock).

**Not changed** (per issue scope):
- `packages/core/src/constants.ts` — `UNCATEGORIZED_AREA` literal stays.
- `packages/core/src/area-grouping.ts` `groupByArea` — bucketing logic unchanged; it keys on raw `area`.
- `packages/codev/src/lib/github.ts` `parseArea` — wire-side parser unchanged.
- `packages/vscode/src/views/backlog.ts`, `packages/vscode/src/views/builders.ts` — `element.areaName === ...` lookups continue to use the raw value (the field is untouched).
- `packages/vscode/src/views/area-group-expansion.ts` — expansion store keys off raw `areaName`, so expand/collapse state is preserved across the rendering change with no migration.

## Risks & Alternatives Considered

- **Risk**: a future area label uses an acronym (`api`, `ui`, `cli`, `sdk`) and the mangle becomes painful. **Mitigation**: the helper is one line; layering a config-driven override map on top is a non-breaking extension when needed. Codev's current set has zero acronyms, so deferring is cheap.
- **Risk**: stable-id stability — the `id` field on the TreeItem is what VSCode uses to persist expansion state. We keep `this.id = \`${kind}-group:${areaName}\`` (raw), so expansion state survives. The `areaName` public field is also unchanged, so the views' `element.areaName === ...` lookups in `backlog.ts:54` and `builders.ts:158` still match against the wire value.
- **Risk**: someone reads the label string back (e.g. accessibility tooling) and tries to match it against the raw area. **Mitigation**: label is documented as display-only in the helper's JSDoc; matchers in the codebase all key off `.areaName` (the typed field), not the label string.

**Alternatives rejected**:

- **Plain sentence-case** (`name.charAt(0).toUpperCase() + name.slice(1)`): renders `cross-cutting` as `Cross-cutting`, which reads as a tag ID rather than a proper category label. Title-case + separator-to-space gives `Cross Cutting`, which sits visually next to `Uncategorized` as a peer.
- **Per-repo `codev.areaDisplayNames` setting (issue's option 2)**: would solve single-word-acronym mangling cleanly, but costs a settings entry + config-read plumbing for a problem codev's own area set doesn't have. The chosen helper's signature (single string in, single string out) keeps this a clean non-breaking follow-up: a config-lookup wrapper layered on top, falling back to the structural rule for unconfigured areas.
- **Hardcoded acronym dictionary inside codev** (e.g. `const ACRONYMS = { vscode: 'VSCode', api: 'API' }`): privileges specific label values inside framework code, conflicting with the principle that teams using codev decide their own labeling semantics. A user-supplied config override is the framework-neutral way to opt in.
- **Length-based or vowel-pattern acronym heuristic** (e.g. uppercase if ≤3 chars, or no vowels): every variant has false positives in codev's own set — `web → WEB`, `app → APP`, `core → CORE`, `docs → DOCS`. No clean general rule exists.
- **Inline the helper at the call site (skip core export)**: only one caller today (`AreaGroupTreeItem`), so inlining is arguably YAGNI in the other direction. Exporting from core costs one extra file modification but pays for itself the first time another consumer (dashboard, CLI status output) needs the same display rule. A future cross-package consumer is plausible enough that I'd rather front-load the small abstraction than copy-paste later.

## Test Plan

**Unit (core helper, tested in vscode test suite where core helpers are tested)**:

Append a `suite('formatAreaForDisplay', ...)` to `packages/vscode/src/test/area-grouping.test.ts` with these cases:

- `formatAreaForDisplay('vscode')` → `'Vscode'`
- `formatAreaForDisplay('tower')` → `'Tower'`
- `formatAreaForDisplay('cross-cutting')` → `'Cross Cutting'`
- `formatAreaForDisplay('front_end')` → `'Front End'`
- `formatAreaForDisplay('Uncategorized')` → `'Uncategorized'` (no-op on the fallback sentinel — the single uniform rule covers it)
- `formatAreaForDisplay('')` → `''` (defensive — never expected, but cheap to lock the contract)

**Manual / dev-approval gate**:

- Run `afx dev pir-885` to launch the worktree's VSCode extension dev host.
- Open the Codev sidebar. Both **Backlog** and **Builders** trees should now show:
- `Vscode (N)`, `Tower (N)`, `Porch (N)` — capitalized first letter, lowercase rest.
- `Cross Cutting (N)` if there are cross-cutting items (hyphen replaced with space, each word capitalized).
- `Uncategorized (N)` last — unchanged in appearance (was already PascalCase, rule is a no-op).
- Expand/collapse a group; reload the extension. The expansion state should persist (verifies `id` stability).
- Right-click a group header → context menu should still work (verifies `contextValue` stability).
- Right-click a builder / backlog row inside a group → existing per-row commands still work (verifies the tree-item shape downstream of the group is unchanged).
- Degenerate case: a workspace whose every issue is uncategorized still flat-lists the items with no group header (the `groups.length === 1 && groups[0].area === UNCATEGORIZED_AREA` early-return in both `backlog.ts:73` and `builders.ts:132` is untouched).

**Cross-platform**: VSCode-only change (no mobile / web surfaces).
30 changes: 30 additions & 0 deletions codev/projects/885-vscode-capitalize-area-group-h/status.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
id: '885'
title: vscode-capitalize-area-group-h
protocol: pir
phase: verified
plan_phases: []
current_plan_phase: null
gates:
plan-approval:
status: approved
requested_at: '2026-05-27T23:09:03.242Z'
approved_at: '2026-05-27T23:19:27.477Z'
dev-approval:
status: approved
requested_at: '2026-05-27T23:23:24.105Z'
approved_at: '2026-05-27T23:27:21.932Z'
pr:
status: approved
requested_at: '2026-05-27T23:31:41.642Z'
approved_at: '2026-05-27T23:38:35.135Z'
iteration: 1
build_complete: false
history: []
started_at: '2026-05-27T23:05:37.187Z'
updated_at: '2026-05-27T23:38:51.000Z'
pr_history:
- phase: review
pr_number: 893
branch: builder/pir-885
created_at: '2026-05-27T23:28:48.054Z'
pr_ready_for_human: false
56 changes: 56 additions & 0 deletions codev/reviews/885-vscode-capitalize-area-group-h.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# PIR Review: Capitalize area group header labels

Fixes #885

## Summary

The Backlog and Builders trees grouped by `area/*` rendered headers verbatim (`vscode (12)`, `tower (4)`), visually inconsistent next to the PascalCase `Uncategorized (8)` fallback. This PR adds a `formatAreaForDisplay` helper in `@cluesmith/codev-core/area-grouping` (title-case with `-`/`_` → space) and applies it in `AreaGroupTreeItem`'s label, so both trees now render `Vscode (12)`, `Tower (4)`, `Cross Cutting (N)`, `Uncategorized (8)`. The raw `areaName` field, the stable `id`, and the `contextValue` continue to use the wire value, so expansion-state persistence and `===` matchers in the view providers are untouched.

## Files Changed

- `packages/core/src/area-grouping.ts` (+27 / −0) — new `formatAreaForDisplay` next to `groupByArea`
- `packages/vscode/src/views/area-group-tree-item.ts` (+8 / −1) — wrap only the displayed label
- `packages/vscode/src/test/area-grouping.test.ts` (+35 / −1) — 7 new cases
- `codev/plans/885-vscode-capitalize-area-group-h.md` (+96 / −0) — approved PIR plan

## Commits

- `830aafe4` [PIR #885] Plan draft
- `a031721e` [PIR #885] Plan revised: title-case + separator-to-space
- `534d94e7` [PIR #885] feat: formatAreaForDisplay helper + apply in AreaGroupTreeItem
- `325ead6d` [PIR #885] test: formatAreaForDisplay cases

## Test Results

- `pnpm build`: ✓ pass
- `pnpm test` (vscode-test / mocha + esbuild type-check + lint): ✓ pass (97 tests, 7 new)
- `pnpm test:unit` (vitest, separate harness): ✓ pass (49 tests)
- Manual verification at the `dev-approval` gate: human reviewed the running worktree via `afx dev pir-885`; group headers in both Backlog and Builders trees render with the new capitalization; expansion state survives reload.

## Architecture Updates

No arch.md changes — this PR adds one helper next to an existing function in the same module, and the call site is a single line. No new module boundary, no new pattern, no consumer-facing API shift. The display-only nature of the helper is documented in its JSDoc (the "wire stays raw" contract).

## Lessons Learned Updates

No lessons-learned.md changes. The design call worth recording — *display-formatting helpers live in core next to the data they format, not in the view layer* — is one instance of a broader pattern already implicit in how `groupByArea` is structured. Two data points isn't a pattern. If a third consumer (e.g. dashboard parity for the same view) reuses `formatAreaForDisplay` directly without copy-paste, that becomes a lessons-worthy generalization.

## Things to Look At During PR Review

- **The acronym question** is intentionally deferred. The plan and the helper's JSDoc both explain this: codev's current `area/*` set has no real acronyms, so `vscode → Vscode` reads cleanly. If a downstream repo surfaces real pain (`api → Api` rather than `API`), a `codev.areaDisplayNames: Record<string, string>` config override is a clean non-breaking extension — the helper's signature (single string in, single string out) supports a config-lookup wrapper layered on top with sentence-case fallback. Not paying that cost preemptively.
- **No hardcoded acronym dictionary.** A built-in `{ vscode: 'VSCode', api: 'API' }` map inside framework code would conflict with `feedback_framework_neutral_on_label_semantics` — teams using codev decide their own labeling semantics; a user-supplied config is the framework-neutral way to opt in.
- **`UNCATEGORIZED_AREA` sentinel is intentionally a no-op** under the rule (single word, first char already upper). The single uniform path through the helper covers every area including the fallback — no special-case branch.
- **Stable `id`** is preserved (`${kind}-group:${areaName}` still uses the raw value), so per-group expansion state survives across the rendering change with zero migration.

## How to Test Locally

For reviewers pulling the branch:

- **View diff**: VSCode sidebar → right-click builder `pir-885` → **View Diff**
- **Run dev server**: VSCode sidebar → **Run Dev Server**, or `afx dev pir-885`
- **What to verify**:
- Open the Codev sidebar; both **Backlog** and **Builders** trees should show `Vscode (N)`, `Tower (N)`, `Porch (N)`, `Cross Cutting (N)` if present, and `Uncategorized (N)` last.
- Expand/collapse a group, reload the extension — expansion state persists (verifies `id` stability).
- Right-click a group header → context menu works (verifies `contextValue` stability).
- Right-click rows inside a group → per-row commands still work (verifies tree shape downstream is unchanged).
- Degenerate case: a workspace whose every issue is uncategorized still flat-lists items with no group header (the early-return at `backlog.ts:73` / `builders.ts:132` is untouched).
42 changes: 42 additions & 0 deletions codev/state/pir-885_thread.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# pir-885 — vscode: capitalize area group header labels

## Plan phase (2026-05-28)

Drafted `codev/plans/885-vscode-capitalize-area-group-h.md`.

Picked **sentence-case** over the per-repo override map. Reasons in the plan: codev's actual `area/*` set has zero acronyms, `'Uncategorized'.charAt(0).toUpperCase() + slice(1)` is a no-op so one rule covers everything including the fallback, and a config override is a clean follow-up if real acronym pain surfaces later.

Helper lives in `packages/core/src/area-grouping.ts` next to `groupByArea` (pure, no vscode dep, reusable by future dashboard equivalent).

Application point is the single line `area-group-tree-item.ts:23` — both `BacklogGroupTreeItem` and `BuilderGroupTreeItem` route through it, so one wrap covers both views. `areaName` field, `id`, and `contextValue` stay raw; only the displayed label changes. Expansion state survives unchanged.

Tests append to `packages/vscode/src/test/area-grouping.test.ts` (core has no test infra of its own; that's where `groupByArea` tests live).

Plan committed (830aafe4) and pushed. Sitting at `plan-approval` gate.

### Revision 2026-05-28 (a031721e)

User reopened the acronym question. Walked through why no general heuristic works (length-based, vowel-pattern all have false positives in codev's own set). User picked **title-case + replace `-`/`_` with space**: `cross-cutting → Cross Cutting`, `front_end → Front End`. Single-word acronyms still mangle (`api → Api`) — accepted as a known cost; config override remains the clean follow-up.

Updated plan's Proposed Change, Files to Change (test cases), Alternatives Considered, and Test Plan sections. `Uncategorized` is still a no-op under the new rule, so the single uniform path through `AreaGroupTreeItem` constructor is preserved.

## Implement phase (2026-05-28)

Plan approved. Commits:
- `534d94e7` — `formatAreaForDisplay` helper exported from `packages/core/src/area-grouping.ts`; `AreaGroupTreeItem` constructor wraps the displayed label via the helper. `areaName` field, `id`, `contextValue` all keep using the raw wire value.
- `325ead6d` — 7 new tests covering single-word, hyphenated, underscored, mixed, the Uncategorized sentinel no-op, empty string, and consecutive-separator collapse.

Build ✓, all 97 vscode tests pass (49 vitest + 48 mocha — area-grouping tests in the mocha suite). Sitting at `dev-approval` gate.

## Review phase (2026-05-28)

`dev-approval` approved. Retrospective written to `codev/reviews/885-vscode-capitalize-area-group-h.md` (commit `114b940d`). Both arch and lessons-learned sections justify "no changes needed" — single helper next to an existing function, no new pattern, no consumer-facing API shift.

PR #893 opened against `main`. Recorded with porch. Ran the 3-way consultation per porch's verify task list.

3-way verdicts (all APPROVE):
- gemini: APPROVE — implementation matches plan, thorough test coverage
- codex: APPROVE — raw area values stay stable, solid review artifact
- claude: APPROVE — flagged one cosmetic JSDoc nit (line 20 of `area-group-tree-item.ts` said "sentence-case" but the rule is title-case). Fixed in `5f6a30a9` — one-word doc change.

Architect notified. Sitting at `pr` gate.
28 changes: 28 additions & 0 deletions packages/core/src/area-grouping.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,33 @@
import { UNCATEGORIZED_AREA } from './constants.js';

/**
* Render an area name as a human-readable group-header label.
*
* Display-only. The raw `area` value (the wire string from
* `OverviewBacklogItem.area` / `OverviewBuilder.area`, or the
* `UNCATEGORIZED_AREA` sentinel) MUST continue to be used for
* bucketing (`groupByArea`), id stability, and `===` matchers.
*
* Rule: split on `-`, `_`, and whitespace; capitalize the first
* character of each word; rejoin with a single space. Examples:
* 'vscode' -> 'Vscode'
* 'cross-cutting' -> 'Cross Cutting'
* 'front_end' -> 'Front End'
* 'Uncategorized' -> 'Uncategorized' (no-op; sentinel is already
* single-word, first-char-upper)
*
* Purely structural: no hardcoded list of "known acronyms". Teams
* using Codev decide their own labeling semantics; a per-repo
* override map can be layered on top as a non-breaking extension.
*/
export function formatAreaForDisplay(area: string): string {
return area
.split(/[-_\s]+/)
.filter(w => w.length > 0)
.map(w => w.charAt(0).toUpperCase() + w.slice(1))
.join(' ');
}

/**
* Bucket items by their resolved area, returning groups in canonical
* Codev order: alphabetical specific areas first, then `Uncategorized`
Expand Down
Loading
Loading