task(review): independent review done, findings filed
closes board-viewer-review Subagent code-review against design spec + acceptance criteria found: - 2 Major: README emoji legend stale (board-viewer-readme-emoji-fix), card meta missing owner pill + last-commit-marker (board-viewer-card-meta-fix) - 6 minor/nitpick bundled as board-viewer-polish All shipped code: 43/43 tests pass, TDD-immutability clean, secret hygiene OK. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
60
.tasks/board-viewer-polish.md
Normal file
60
.tasks/board-viewer-polish.md
Normal file
@@ -0,0 +1,60 @@
|
||||
# board-viewer-polish
|
||||
|
||||
## Goal
|
||||
|
||||
Bundle of minor / nitpick findings from `board-viewer-review` checkpoint. Each — small, individually below the task-creation threshold; collectively worth doing in one sweep.
|
||||
|
||||
## Items
|
||||
|
||||
### M1. Document parser's `**Status:**` field neglect
|
||||
|
||||
- **Location:** `src/parser.ts` (in `parseOneBlock`)
|
||||
- **Problem:** `**Status:**` text field is matched by `FIELD_RE` but never stored in `StatusBlock` (emoji is SoT). Future maintainer reads parser and wonders why.
|
||||
- **Fix:** comment `// **Status:** text field intentionally ignored — emoji is canonical` near where it's matched, OR `delete fields['Status']` after parse to avoid the orphan field in local var. Choose the comment.
|
||||
|
||||
### M2. `formatAge` future-timestamp behavior is undocumented
|
||||
|
||||
- **Location:** `src/render.ts:31` + `static/board.js:49`
|
||||
- **Problem:** `Math.max(0, …)` silently clamps future ISO to "0m". With server-clock drift на VDS vs Gitea это маскирует ошибку синхронизации.
|
||||
- **Fix:** add one test asserting `formatAge(futureIso, now) === '0m'` to lock behavior, and add inline comment "// clamps to '0m' on future iso — see test".
|
||||
|
||||
### M3. `gitea-reader` acceptance "per-task без записи в STATUS.md (legacy)" is impossible by design
|
||||
|
||||
- **Location:** `.tasks/board-viewer-gitea-reader.md` Decisions log
|
||||
- **Problem:** Acceptance line 42 promises tests for "per-task без записи в STATUS.md (legacy)" case, but the reader iterates STATUS.md blocks only — orphan per-task files are invisible by design (matches spec: STATUS.md is the index).
|
||||
- **Fix:** append a decisions-log entry to `board-viewer-gitea-reader.md` clarifying this is intentional; STATUS.md is the entry point, orphan per-task files are out of scope.
|
||||
|
||||
### M4. Client filter doesn't match status in by-status mode; "multi-select по статусам" not implemented
|
||||
|
||||
- **Location:** `static/board.js:139-147` (`applyFilter`)
|
||||
- **Problem:** Filter reads `.badge.status` / `.badge.project` text from DOM. In by-status mode there's no status badge → typing "blocked" matches nothing. Также acceptance в `board-viewer-html-render.md:27` обещает multi-select по статусам — не реализован.
|
||||
- **Fix:** rebuild `applyFilter` to query the underlying `records` array (status always available regardless of view mode). Decide on multi-select: either ship a status-pill filter row, or amend acceptance criteria. Recommendation: add multi-select pills below the filter input — simple `<button data-status="…" class="status-filter">` toggle.
|
||||
|
||||
### M5. Build container failures show only `echo` output, no Docker healthcheck
|
||||
|
||||
- **Location:** `deploy/Dockerfile.build:14`
|
||||
- **Problem:** `... || echo 'render failed; will retry next tick'` swallows non-zero exit. Operator must `docker compose logs build` to notice. No Docker healthcheck signals unhealthy.
|
||||
- **Fix:** emit failure to stderr (`>&2 echo`), and add a `HEALTHCHECK` to Dockerfile that checks `[ -f /output/index.html ]` AND `[ $(date +%s) - $(stat -c %Y /output/index.html) -lt 600 ]` (file exists AND fresher than 10 min). Compose `web` service can `depends_on` health.
|
||||
|
||||
### M6. `getLatestCommitIso` called serially, N=80+ per tick
|
||||
|
||||
- **Location:** `src/reader.ts:36-37`
|
||||
- **Problem:** `for...of await` serializes 80+ HTTP calls. Within budget for now but a Gitea hiccup serializes through all of them.
|
||||
- **Fix:** parallelize per-repo or globally with `Promise.all(blocks.map(...))`. Cap concurrency at ~10 if politeness is a concern.
|
||||
|
||||
## Decisions log
|
||||
|
||||
- 2026-05-22: bundle создан промоушеном из review-чекпойнта; severity варьируется minor → nitpick; bundled, чтобы не плодить 6 микро-тасок.
|
||||
|
||||
## Completed steps
|
||||
|
||||
- [ ] M1: parser comment
|
||||
- [ ] M2: formatAge future-ts test + comment
|
||||
- [ ] M3: decisions-log entry in gitea-reader.md
|
||||
- [ ] M4: filter rebuild + status multi-select OR acceptance amendment
|
||||
- [ ] M5: stderr + Docker HEALTHCHECK
|
||||
- [ ] M6: parallelize commit-iso fetches
|
||||
|
||||
## Notes
|
||||
|
||||
Каждый item — TDD по тому, что есть testable. M1/M3 — comment/docs only ([skip-tdd: visual]). M2 — pure test. M4/M5/M6 — TDD logic.
|
||||
Reference in New Issue
Block a user