Skip to content

Phase 2 S2: AttentionRow surfaces pending-decision counts#6

Merged
screenleon merged 2 commits intomainfrom
feat/phase2-s2-attention-row
Apr 22, 2026
Merged

Phase 2 S2: AttentionRow surfaces pending-decision counts#6
screenleon merged 2 commits intomainfrom
feat/phase2-s2-attention-row

Conversation

@screenleon
Copy link
Copy Markdown
Owner

Summary

Phase 2 slice S2 per docs/phase2-planning-workspace-design.md §7. Adds a new AttentionRow at the top of the Planning workspace that answers the operator's real question on arrival: "what's blocked on my review right now?"

Four click-through tiles, each derived from state that's already loaded by ProjectDetail / PlanningTabno new API call:

Tile Source Jump target
Requirements awaiting planning requirements.filter(status==='draft') scroll to RequirementQueue
Candidates awaiting review selected run's planningCandidates.status in {draft, approved} scroll to CandidateReviewPanel
Applied tasks still open tasks.filter(source==='agent:planning-orchestrator' && status in {todo, in_progress}) switch to Tasks tab
Open drift signals openDriftCount (already computed in ProjectDetail) switch to Drift tab

Zero-count tiles disable their click handler and render at 60% opacity; positive-count attention tiles (requirements / candidates / drift) get a red left border for the "needs decision" signal.

What this PR does NOT do

  • No cross-run candidate aggregation (design doc §7-S2 flagged "per-run and total"; total requires a new endpoint which S2 explicitly excludes).
  • No behaviour change elsewhere in the workspace. PlanningStepper / foundation grid / launcher / run list / candidate review all render identically.
  • No backend schema change. No new API call.

Props change

PlanningTab now takes three additional props:

  • openDriftCount: number
  • onNavigateToTasks: () => void
  • onNavigateToDrift: () => void

ProjectDetail.tsx is the only caller and wires them to existing setTab() / openDriftCount state. The existing PlanningTab.test.tsx baseProps fixture is updated accordingly.

Test plan

  • cd frontend && npm ci && npm test — 61 cases green (57 → 61; +4 for AttentionRow)
  • cd frontend && npm run lint && npx tsc --noEmit && npm run build
  • make lint-governance
  • bash scripts/test-with-sqlite.sh — backend untouched
  • Manual: anpm serve, open Planning tab on a project with drift signals + draft requirements; verify tile counts match; click each tile to confirm navigation / scroll

Design doc status

Updated §Status tracking table: S1 → merged (PR #5), S2 → in review (this PR). S3 / S4 / S5 remain not-started.

🤖 Generated with Claude Code

…kspace top

Phase 2 slice S2 per docs/phase2-planning-workspace-design.md §7.

New component `pages/ProjectDetail/planning/AttentionRow.tsx` at the top
of the Planning workspace presents four click-through tiles:

* Requirements awaiting planning — requirements.status === 'draft'.
* Candidates awaiting review — planningCandidates.status in
  {draft, approved} for the currently-selected run. Per-run scope is
  intentional (the design doc §7-S2 notes "per-run and total"; total
  requires a cross-run query which S2 does not add).
* Applied tasks still open — tasks with source === 'agent:planning-
  orchestrator' and status in {todo, in_progress}. The source tag is
  assigned by the apply-candidate flow (appliedCandidateTaskSource
  constant in backlog_candidate_store.go).
* Open drift signals — comes from openDriftCount already computed in
  ProjectDetail.

All four counts are derived from state already loaded by ProjectDetail /
PlanningTab. **S2 intentionally adds no new API call** (design doc §7).

PlanningTab's public props extend by three: openDriftCount, onNavigate-
ToTasks, onNavigateToDrift. ProjectDetail wires them to the existing
setTab() + openDriftCount plumbing. Other callers are unaffected —
there is only one.

Tile behaviour:

* Zero-count tiles render with reduced opacity and are disabled (no
  click handler fires). This prevents accidental navigation to an empty
  Drift tab or a Tasks tab that has no applied tasks to review.
* Positive-count attention-tone tiles (requirements, candidates, drift)
  get a red left border to signal "needs decision".
* Click-through scrolls within the workspace for requirements /
  candidates, or invokes onNavigateToTasks / onNavigateToDrift to switch
  tabs for the cross-tab jumps.

Tests: 4 new smoke cases covering counts, disabled-on-zero, and click
propagation. Total vitest suite goes from 57 → 61.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 22, 2026 05:20
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an “AttentionRow” summary at the top of the Planning workspace to surface counts of items awaiting operator attention (requirements, candidates, applied open tasks, drift), with click-through navigation using already-loaded state (no new API calls).

Changes:

  • Introduces AttentionRow component + tests to render four count tiles with disabled/active behavior.
  • Wires PlanningTab/ProjectDetail props to supply openDriftCount and navigation callbacks, and computes the four counts from existing state.
  • Updates the Phase 2 design doc status-tracking table for S1/S2.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
frontend/src/pages/ProjectDetail/planning/AttentionRow.tsx New attention summary UI component with four tiles and click-through handlers
frontend/src/pages/ProjectDetail/planning/AttentionRow.test.tsx Adds unit tests for rendering, disabled state, and click behavior
frontend/src/pages/ProjectDetail.tsx Passes openDriftCount and navigation callbacks into PlanningTab
frontend/src/components/PlanningTab.tsx Computes pending-decision counts; adds scroll-jump handlers and renders AttentionRow
frontend/src/components/PlanningTab.test.tsx Updates test props fixture for the new PlanningTab props
docs/phase2-planning-workspace-design.md Updates slice status table to reflect S1 merged and S2 in review

Comment thread frontend/src/pages/ProjectDetail/planning/AttentionRow.tsx
Comment thread frontend/src/components/PlanningTab.tsx Outdated
…l helpers

Two Copilot findings on PR #6, both real:

1. AttentionRow tile styling: the commit message promised a "red left
   border for positive-count attention tiles" but the implementation set
   `border: 1px solid var(--danger)` which coloured all four edges. A
   full danger-coloured border reads like an error state rather than an
   "attention needed" accent.

   Change to the originally advertised behaviour: the neutral border is
   preserved on three edges, and only `borderLeft` uses the danger
   colour (3px accent stripe). The tile still reads as part of the row
   and the attention signal stays subtle.

2. PlanningTab scroll helpers: `jumpToCandidates()` duplicated the
   querySelector + scrollIntoView logic already inlined inside
   `PlanningStepper`'s `onJumpToCandidates` callback. `onJumpToWorkspace`
   and `onJumpToIntake` also inlined their own variants of the same
   pattern.

   Extract two local helpers — `scrollToSelector(selector)` and
   `focusSelector(selector)` — and have every jump closure call them.
   `jumpToCandidates`, `jumpToRequirements`, `jumpToWorkspace`, and
   `jumpToIntake` are now one-line arrow functions. Selector changes now
   happen in exactly one place per target surface.

No behaviour change. Tests stay 61/61 green; lint + tsc clean.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@screenleon screenleon merged commit 2d8458b into main Apr 22, 2026
4 checks passed
@screenleon screenleon deleted the feat/phase2-s2-attention-row branch April 22, 2026 05:43
screenleon added a commit that referenced this pull request Apr 27, 2026
…talog enforcement (#27)

* feat(phase6c-pr2): authoring lifecycle + actor_audit + multi-point catalog enforcement

Phase 6c PR-2 (per docs/phase6c-plan.md v5.1 §3.2). Closes the
catch-22 where role_dispatch was UI-disabled because no surface
existed to set candidate.execution_role. The role choice now flows
through the apply payload (and an inline candidate editor), is
validated against the catalog at four entry points, and every
mutation writes a structured audit row.

## Backend

- Migration 030: new actor_audit table (generic discriminator on
  subject_kind so other fields can adopt the same model later).
  Append-only, no FK on subject_id (history-preserving by design).
- New backend/internal/audit/ package: Record(tx, ...) inside
  caller's transaction; QueryLatest(ctx, db, ...) for reads.
  ActorKind enum: user / api_key / router / system / connector.
  Confidence is rejected for non-router actors so downstream
  router-only queries cannot be polluted (critic round 1 #1).
- Role.Category field on backend/internal/roles/catalog.go (all 6
  roles tagged "role"; the dispatcher meta-role lands in PR-3).
  TestCatalogMatchesPromptDir now cross-checks the frontmatter
  category column.
- BacklogCandidateStore.Update(id, req, actor) and ApplyToTaskWithMode
  (id, mode, executionRole, actor) signatures changed to carry
  ActorInfo. Both validate execution_role against the catalog,
  write the column, and emit the audit row inside the same
  transaction. New typed errors:
  ErrBacklogCandidateMissingExecutionRole,
  ErrBacklogCandidateUnknownExecutionRole.
- BacklogCandidateStore.EnrichWithAuthoring populates
  BacklogCandidate.ExecutionRoleAuthoring from the latest audit
  row so the GET / PATCH / apply responses surface the
  who/when/rationale trail without a second API round-trip
  (risk-reviewer H1 fix). Pre-Phase-6c rows have no audit history
  and surface null — backfill is intentionally not done.
- TaskStore.ClaimNextDispatchTask drains stale-role tasks
  (role no longer in catalog) by transitioning queued -> failed
  with error_kind=role_not_found and an actor_kind=system audit
  row, then continues to find the next valid task. Drain is
  bounded by staleDrainCap (16 per call) so a poisoned queue
  cannot wedge the connector loop (critic round 1 #6).
- New GET /api/roles handler (public, no auth — catalog is
  public source data). Filters category="role"; meta-roles
  introduced in PR-3 will be excluded from this surface.
- New buildAuthoringActor(r, rationale) helper: routes session
  callers to ActorUser, API-key callers to ActorAPIKey
  (id="api-key:<key id>"), else logs an "unauthenticated"
  defense-in-depth row. Resolves critic round 1 #3 +
  risk-reviewer M1 (the apply/PATCH paths previously collapsed
  both auth channels into ActorUser with empty actor_id when
  api-key was active).

## Frontend

- New types/roles.ts: KNOWN_ROLE_IDS hand-mirror of the Go
  catalog + isKnownRoleId narrow function. Drift-detected by
  roles.test.ts which fetches /api/roles via mock and asserts
  set equality.
- New ExecutionRoleAuthoring type on BacklogCandidate; the
  candidate response now carries who/when/why for execution_role.
- usePlanningWorkspaceData fetches /api/roles on mount;
  availableRoles is RoleInfo[] | null with the null sentinel
  meaning "still loading" so the panel can suppress the
  stale-role warning during the fetch (critic round 1 #5 +
  risk-reviewer L1).
- CandidateReviewPanel: role_dispatch radio is always enabled
  (the Phase 5 disabled-on-no-role gating is removed). When
  selected, an inline role <select> shows each catalog role
  with version + estimated minutes. Apply button is disabled
  until a role is chosen. Stale-role warning fires only after
  /api/roles resolves with the candidate's role missing from it.
- New CandidateRoleEditor: chip + inline edit popover so
  operators can pre-tag candidates with a role outside the
  apply flow. PATCH carries the actor through the same audit
  path as the apply endpoint.

## Tests

- audit/audit_test.go: 9 tests covering record + query + rollback
  + concurrent insert + ordering + router confidence required +
  non-router confidence rejected.
- store: replaced two Phase 5 contract tests
  ("unknown role accepted", "rune-truncation defense") with
  Phase 6c PR-2 catalog-enforcement counterparts. Existing Update
  callers updated to pass audit.ActorInfo{}.
- handlers/connector_dispatch_test.go: three new tests covering
  TestClaimNextTask_StaleRoleTransitionsToFailed,
  TestClaimNextTask_DrainsStaleRoleThenClaimsNext,
  TestRolesEndpoint_ReturnsCatalog.
- frontend: 7 CandidateRoleEditor tests, 5 new
  CandidateReviewPanel tests covering role select rendering
  + Apply gating + stale warning, 6 drift tests against
  /api/roles.

## Reviews completed

- make pre-pr: green twice (SQLite + PostgreSQL + frontend
  build + lint + typecheck).
- critic subagent (round 1): 10 findings, 4 Mandatory all
  addressed (#1 confidence rule, #3 actor disambiguation,
  #5 stale-warning race, #6 drain loop cap). 3 Should-fix
  addressed (#2 documented, #7 documented, #9 verified).
- /security-review: 0 findings at confidence >= 8.
- risk-reviewer: 3 HIGH, 4 MED, 6 LOW. HIGH/MED all addressed
  inline (H1 audit read path via EnrichWithAuthoring; H2
  DECISIONS entry added; H3 documented as PR-4 surfacing path;
  M1 buildAuthoringActor; M2/M3 docs/api-surface.md +
  docs/data-model.md updated).

## Docs

- docs/api-surface.md: PATCH + apply contracts updated, new
  GET /api/roles documented, breaking-change call-out for
  the apply payload.
- docs/data-model.md: actor_audit table documented;
  execution_role description updated.
- DECISIONS.md: new 2026-04-26 entry recording the
  implementation refinements that diverged from the
  2026-04-25 entry's pre-implementation API names + cascade-
  delete description.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(phase6c-pr2): address Copilot PR #27 review feedback

Six line-level findings from copilot-pull-request-reviewer on PR #27,
all addressed without simple/temporary workarounds:

1. CandidateReviewPanel pass-through: stop coercing the null loading
   sentinel to []; pass `availableRoles ?? null` so CandidateRoleEditor
   keeps its loading-state suppression.

2. CandidateRoleEditor staleness SoT: once /api/roles loads, base
   isStale on the runtime catalog (`availableRoles.find`), and only
   fall back to the static KNOWN_ROLE_IDS mirror while still loading.
   Survives staggered deploys where backend catalog leads frontend.

3. usePlanningWorkspaceData fetch failure: keep availableRoles=null on
   catch (never []), and add a sibling availableRolesError string state
   plumbed through PlanningTab → both panels. Editor + dropdown render
   an explicit "Failed to load roles: …" message so operators can
   distinguish transient failure from "catalog is empty".

4. task_store.parseRoleIDFromSource: split malformed-source from
   role-not-found. New error_kind=role_dispatch_malformed (with its
   own remediation hint) is emitted when the task source has no
   role suffix; role_not_found stays for catalog absence with a
   well-formed id. Helper now returns (roleID, hasSuffix).

5. Migration 030 comment: enumerate all five actor_kind values
   (added api_key) and remove an embedded `;` that re-tripped the
   SQLite migration-parser comment-split bug.

6. audit package: split ErrConfidenceNotAllowed (non-router supplied
   confidence) from ErrInvalidConfidence (router missing/out-of-range).
   Logs/debug now describe the actual failure mode.

pre-pr (lint + SQLite + PostgreSQL + frontend typecheck + vitest +
production build): all green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(phase6c-pr2): critic round-4 follow-up — connector parity, tests, docs

Critic round 4 raised two Mandatory and three Suggested findings on
the prior commit (72e066d). All addressed:

M1 — Connector parser parity. backend/internal/connector/service.go
RunOnceTask was emitting ErrorKind: "unknown" for both the missing-
suffix and unknown-role branches. Now mirrors the server: emits
models.ErrorKindRoleDispatchMalformed for missing/empty role suffix
and models.ErrorKindRoleNotFound for well-formed-but-absent ids,
matching the diagnostic the server uses on the claim path. Updated
godoc on parseRoleIDFromSource to reflect this lockstep.

M2 — Backfill obligation documented. New 2026-04-27 DECISIONS entry
constraint (d) explicitly waives backfill: Phase 6c is greenfield,
no live deployment carries tasks rows with source="role_dispatch"
(no colon), so the new error_kind has no historical backfill
obligation. Future operators who observe this kind should
investigate the candidate-applier code path.

S1 — Malformed-source store-layer test. New
TestClaimNextTask_MalformedSourceTransitionsToFailed in
connector_dispatch_test.go covers all three sub-cases (no_colon,
empty_suffix, whitespace_suffix) and asserts error_kind +
error_message + audit row. Added seedQueuedTaskWithSource helper
to seed arbitrary source strings.

S2 — Frontend error-UI test coverage. Two new
CandidateRoleEditor.test.tsx tests:
  - shows "Failed to load roles: …" alert when availableRolesError
    is set and the editor is opened
  - does NOT flag stale while availableRoles is loading (null)
    without an error (regression guard for the suppression logic).

S4 — Server-push catalog assumption documented inline. New code
comment in CandidateRoleEditor.tsx flags the assumption that
availableRoles is fetched once on mount and never mutated mid-
session, so a future Phase 6c PR-4/PR-5 SSE change to push
catalog updates needs to revisit the staleness check.

pre-pr (lint + SQLite + PostgreSQL + frontend typecheck + vitest +
production build): all green.
risk-reviewer: zero HIGH/MEDIUM, six LOW (all dispositioned).
/security-review: zero findings at confidence >= 8.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants