diff --git a/docs/coding-agent/lessons.md b/docs/coding-agent/lessons.md index 8d41d09..34159b6 100644 --- a/docs/coding-agent/lessons.md +++ b/docs/coding-agent/lessons.md @@ -112,3 +112,60 @@ Prevention: Evidence: - User explicitly flagged missed plan completion/archival after task execution. + +## 2026-02-22 — Reviewer gate must include required UI evidence artifacts [tags: validation, review] + +Context: +- Plan: docs/coding-agent/plans/active/sleep-edit-theme-shell-unification-plan.md +- Task/Wave: Task_2 / Wave 2 +- Roles involved: Orchestrator, Reviewer + +Deviation: +- Reviewer returned NEEDS_REVISION because required dark-mode screenshots under `.playwright-cli/` were missing. +- Initial review evidence did not conclusively satisfy the planned create/edit/delete smoke-flow proof. + +Root cause: +- Reviewer dispatch prompt did not enforce artifact path verification as a hard completion condition before returning status. +- Validation gate was interpreted as best-effort test execution instead of strict evidence checklist completion. + +Fix applied: +- Paused execution and re-routed through improvement loop before continuing. +- Added a plan decision-log delta to re-run reviewer gate with explicit artifact capture and evidence checklist checks. + +Prevention: +- Primary promotion target: references/ui-e2e.md +- Candidate prevention rule (optional): + - audience: orchestrator + - proposed rule: Reviewer prompts for UI-impact tasks must include explicit required artifact filenames/locations and fail-fast if missing. +- Optional guardrail: + - Before accepting Reviewer APPROVED, verify each required artifact path exists. + +Evidence: +- Reviewer report status NEEDS_REVISION with missing `.playwright-cli` screenshot artifacts. + +## 2026-02-22 — Normalize cwd in persistent terminal validation runs [tags: environment, validation] + +Context: +- Plan: docs/coding-agent/plans/active/sleep-edit-theme-shell-unification-plan.md +- Task/Wave: Task_1 / Wave 1 +- Roles involved: Worker + +Deviation: +- Required command `cd sleep-ui && npm run test:unit` failed on first attempt because the persistent shell was already in `sleep-ui`. + +Root cause: +- Validation commands assumed repository-root cwd for each invocation despite persistent terminal state. + +Fix applied: +- Worker recovered by checking cwd and running `npm run test:unit` from the correct directory. + +Prevention: +- Primary promotion target: troubleshooting/index.md +- Candidate prevention rule (optional): + - audience: worker + - proposed rule: In persistent terminal sessions, verify cwd (`pwd`) before repeated `cd && ...` sequences. +- Optional guardrail: + - Prefer absolute-path command forms for required validation scripts when run serially. + +Evidence: +- Worker report captured initial cwd-related failure followed by successful recovered run. diff --git a/docs/coding-agent/plans/completed/sleep-edit-theme-shell-unification-plan.md b/docs/coding-agent/plans/completed/sleep-edit-theme-shell-unification-plan.md new file mode 100644 index 0000000..7149e77 --- /dev/null +++ b/docs/coding-agent/plans/completed/sleep-edit-theme-shell-unification-plan.md @@ -0,0 +1,174 @@ +# Plan: Align Sleep Edit UI Theme and DRY Wrapper Structure + +- status: done +- generated: 2026-02-22 +- last_updated: 2026-02-22 +- work_type: code + +## Goal +- Make the sleep-log edit page match the dark-mode-ready visuals used by the create page, and remove avoidable wrapper duplication while preserving existing behavior and selectors. + +## Definition of Done +- Edit page wrapper uses semantic theme classes/tokens (no hard-coded light-only wrapper chrome). +- Create/edit wrapper chrome is unified where safe (presentational reuse only). +- Existing create/edit behavior remains intact (save/cancel/delete/prefill flow). +- Existing E2E selectors remain stable unless explicitly justified. +- Required validation and reviewer UI evidence are recorded. + +## Scope / Non-goals +- Scope: + - `sleep-ui/src/routes/sleep/new/+page.svelte` + - `sleep-ui/src/routes/sleep/[id]/edit/+page.svelte` + - `sleep-ui/src/lib/components/**` (only if needed for shared presentational shell) + - `sleep-ui/tests/**` only if selector or UI validation updates are required +- Non-goals: + - API/backend/model changes + - Broad redesign of sleep flows + - Rewriting `SleepForm` business logic + +## Context (workspace) +- Related files/areas: + - `sleep-ui/src/routes/sleep/new/+page.svelte` + - `sleep-ui/src/routes/sleep/[id]/edit/+page.svelte` + - `sleep-ui/src/lib/components/SleepForm.svelte` + - `sleep-ui/src/app.css` +- Existing patterns or references: + - Semantic theme classes (`page-title`, `text-muted`, `surface-card`, `btn-danger`) should be preferred over hard-coded color utilities. + - `SleepForm.svelte` already centralizes form logic for create/edit modes. +- Repo reference docs consulted: + - `docs/coding-agent/rules/common.md` + - `docs/coding-agent/rules/orchestrator.md` + - `docs/coding-agent/references/validation.md` + - `docs/coding-agent/references/ui-e2e.md` + +## Open Questions (max 3) +- Q1: None; proceed with minimal presentational unification and preserve existing test ids. + +## Assumptions +- A1: Dark-mode mismatch is limited to route-level wrapper chrome on edit page. +- A2: A shared presentational shell can be introduced without altering route load/action contracts. + +## Tasks + +### Task_1: Implement theme parity and DRY wrapper reuse for create/edit pages +- type: impl +- owns: + - `sleep-ui/src/routes/sleep/new/+page.svelte` + - `sleep-ui/src/routes/sleep/[id]/edit/+page.svelte` + - `sleep-ui/src/lib/components/**` +- depends_on: [] +- description: | + Replace edit wrapper light-only classes with semantic theme classes matching create behavior. + If duplication remains, extract shared presentational shell component/structure while keeping route-specific logic in route files. + Preserve existing ids used by tests. +- acceptance: + - Edit wrapper uses semantic theme classes equivalent to create shell visuals. + - Shared wrapper structure is centralized when safe (presentational only). + - No behavior regression in create/edit submission, cancel, and delete flows. + - Existing test ids are preserved or intentionally updated with corresponding test adjustments. +- validation: + - kind: command + required: true + owner: worker + detail: "cd sleep-ui && npm run check" + - kind: command + required: true + owner: worker + detail: "cd sleep-ui && npm run test:unit" + - kind: command + required: true + owner: worker + detail: "cd sleep-ui && npm run build" + +### Task_2: Reviewer UI/E2E evidence gate for create/edit dark-mode parity +- type: review +- owns: [] +- depends_on: [Task_1] +- description: | + Run browser-based checks for create/edit wrappers in dark mode and key create->edit flow stability. + Collect evidence artifacts under `.playwright-cli/`. +- acceptance: + - Reviewer confirms create and edit wrappers render with consistent dark-mode visuals. + - Reviewer confirms critical flow is intact (create/edit/delete path smoke coverage). + - Reviewer status is APPROVED or explicit issues are returned. +- validation: + - kind: e2e + required: true + owner: reviewer + detail: "Run the E2E spec in this plan using playwright-cli and capture evidence under .playwright-cli/" + +## Task Waves (explicit parallel dispatch sets) + +- Wave 1 (parallel): [Task_1] +- Wave 2 (parallel): [Task_2] + +## E2E / Visual Validation Spec (required: UI impacted) + +- artifact_root: ".playwright-cli" +- base_url: "http://127.0.0.1:5173" +- app_start_command: "cd sleep-ui && npm run test:e2e" +- readiness_check: "Playwright global setup completes and app is reachable on strict local host" +- flows: + - "Open create sleep page and verify heading/card/subtext theme visuals in dark mode" + - "Create/edit entry path smoke check and verify edit heading/card/delete chrome in dark mode" +- viewports: + - "desktop default from playwright config" +- evidence_requirements: + - "At least one screenshot for create page dark mode wrapper" + - "At least one screenshot for edit page dark mode wrapper" + - "Reviewer note for flow pass/fail and console/network anomalies" +- known_flakiness: + - "Use existing E2E harness isolation and localhost guardrails" + +## Rollback / Safety +- Revert changes in route wrappers and shared shell component; keep `SleepForm` API unchanged. +- If shared shell introduces risk, fall back to direct class parity update in edit page only. + +## Progress Log (append-only) + +- 2026-02-22 00:00 Draft created. + - Summary: Planned dark-mode parity fix + DRY wrapper unification + reviewer UI evidence gate. + - Validation evidence: N/A (planning stage) + - Notes: Awaiting user approval before execution. + +- 2026-02-22 00:20 Wave 1 completed: [Task_1] + - Summary: Added shared `SleepEntryShell` presentational component and switched create/edit wrappers to semantic theme-tokenized shell. + - Validation evidence: + - `cd sleep-ui && npm run check` (pass) + - `cd sleep-ui && npm run test:unit` (pass after cwd normalization retry) + - `cd sleep-ui && npm run build` (pass) + - Notes: Existing key test ids were preserved. + +- 2026-02-22 00:30 Wave 2 attempted: [Task_2] + - Summary: Reviewer returned NEEDS_REVISION; required dark-mode screenshots under `.playwright-cli/` were missing and flow evidence was inconclusive due E2E failures. + - Validation evidence: Reviewer report status `NEEDS_REVISION`. + - Notes: Execution paused for mandatory deviation improvement loop before re-running review gate. + +- 2026-02-22 00:42 Wave 2 completed: [Task_2] + - Summary: Reviewer re-gate APPROVED with required dark-mode create/edit wrapper screenshots and create→edit→delete smoke-flow confirmation. + - Validation evidence: + - `sleep-ui/.playwright-cli/task2-create-dark-wrapper.png` + - `sleep-ui/.playwright-cli/task2-edit-dark-wrapper.png` + - Reviewer flow snapshots and summary evidence under `sleep-ui/.playwright-cli/`. + - Notes: No additional implementation changes required after review re-gate. + +## Decision Log (append-only; re-plans and major discoveries) + +- 2026-02-22 00:00 Decision: + - Trigger / new insight: Research showed form logic is already shared; wrapper chrome drift is in edit route. + - Plan delta (what changed): Prioritized presentational wrapper unification over form-level refactor. + - Tradeoffs considered: Monolithic create/edit page merge rejected to avoid coupling route-specific side effects. + - User approval: yes + +- 2026-02-22 00:32 Decision: + - Trigger / new insight: Reviewer gate lacked required screenshot artifacts and did not conclusively establish smoke-flow stability. + - Plan delta (what changed): Re-run Task_2 with explicit artifact capture requirements and focused create/edit/delete smoke verification. + - Tradeoffs considered: Avoid code changes until reviewer evidence confirms whether observed failures are regressions versus harness flakiness. + - User approval: not required (no scope expansion) + +## Notes +- Risks: + - Selector drift could break E2E if ids change. + - Dark-mode contrast on destructive action needs verification. +- Edge cases: + - Edit route async prefill behavior must remain unchanged. diff --git a/docs/coding-agent/references/ui-e2e.md b/docs/coding-agent/references/ui-e2e.md index cc2b3a8..af66495 100644 --- a/docs/coding-agent/references/ui-e2e.md +++ b/docs/coding-agent/references/ui-e2e.md @@ -39,6 +39,9 @@ For each required flow: - record unexpected redirects (if applicable) 4) viewports: - at least 2 representative breakpoints (e.g., mobile + desktop) +5) artifact presence check: + - verify each required screenshot path exists under `.playwright-cli/` + - if any required artifact is missing, mark review as `NEEDS_REVISION` --- @@ -64,6 +67,7 @@ When reporting results (review output or task report), include: - flows executed and whether each passed - viewports tested - screenshots captured (paths under `.playwright-cli/`) +- explicit confirmation that all required artifact paths exist - console errors/warnings (if any) - failed network requests/unexpected redirects (if any) diff --git a/docs/coding-agent/troubleshooting/index.md b/docs/coding-agent/troubleshooting/index.md index d004350..919b520 100644 --- a/docs/coding-agent/troubleshooting/index.md +++ b/docs/coding-agent/troubleshooting/index.md @@ -34,4 +34,4 @@ If a troubleshooting pattern is cross-repo and durable, stage it for global migr Add links here as you create new docs under this directory. -- (none yet) +- `persistent-terminal-cwd.md` — validation command failures caused by persistent shell working-directory drift diff --git a/docs/coding-agent/troubleshooting/persistent-terminal-cwd.md b/docs/coding-agent/troubleshooting/persistent-terminal-cwd.md new file mode 100644 index 0000000..5f48c42 --- /dev/null +++ b/docs/coding-agent/troubleshooting/persistent-terminal-cwd.md @@ -0,0 +1,19 @@ +# Persistent terminal cwd mismatches during validation + +- Symptom: + - A required command like `cd sleep-ui && npm run test:unit` fails with `No such file or directory` in an agent run. + +- Likely cause: + - The terminal session is persistent and the current directory is already `sleep-ui`, so repeating `cd sleep-ui` points to a non-existent nested path. + +- Confirmations (commands / checks): + - Run `pwd` to confirm current working directory. + - Run `ls` to confirm expected project files exist in the active directory. + +- Fix: + - If already in target directory, run command without `cd` (for example: `npm run test:unit`). + - Or normalize explicitly with an absolute path in one step. + +- Prevention (docs/rules/checks): + - Before running chained `cd && ...` commands in persistent terminals, verify cwd first. + - Prefer absolute-path command forms for required validations when commands are run sequentially. diff --git a/sleep-ui/src/lib/components/SleepEntryShell.svelte b/sleep-ui/src/lib/components/SleepEntryShell.svelte new file mode 100644 index 0000000..712f1bc --- /dev/null +++ b/sleep-ui/src/lib/components/SleepEntryShell.svelte @@ -0,0 +1,27 @@ + + +
+ {#if $$slots.actions} +
+
+

{title}

+

{description}

+
+ +
+ {:else} +
+

{title}

+

{description}

+
+ {/if} + +
+ +
+
\ No newline at end of file diff --git a/sleep-ui/src/routes/sleep/[id]/edit/+page.svelte b/sleep-ui/src/routes/sleep/[id]/edit/+page.svelte index 4baf1b6..1729bb6 100644 --- a/sleep-ui/src/routes/sleep/[id]/edit/+page.svelte +++ b/sleep-ui/src/routes/sleep/[id]/edit/+page.svelte @@ -3,6 +3,7 @@ import { get } from 'svelte/store'; import { browser } from '$app/environment'; import SleepForm from '$lib/components/SleepForm.svelte'; + import SleepEntryShell from '$lib/components/SleepEntryShell.svelte'; import { goto } from '$app/navigation'; import { deleteSleep, getExerciseIntensity } from '$lib/api'; import type { SleepSession } from '$lib/api'; @@ -78,22 +79,22 @@ } -
-
-
-

Edit sleep entry

-

Update details or remove this session.

-
+ + -
+ -
-
-
+ diff --git a/sleep-ui/src/routes/sleep/new/+page.svelte b/sleep-ui/src/routes/sleep/new/+page.svelte index b7d5791..f1de69c 100644 --- a/sleep-ui/src/routes/sleep/new/+page.svelte +++ b/sleep-ui/src/routes/sleep/new/+page.svelte @@ -1,5 +1,6 @@ -
-
-

New sleep entry

-

Log bedtime, wake time, and how you feel.

-
-
+ -
-
+