Fix #564: stale index cards suppress milestone lists; deep link drops state #565

Merged
crueber merged 2 commits from fix/issue-564 into main 2026-09-15 12:43:50 +00:00
Owner

Fixes #564.

Backend (internal/issues)

Stale index cards suppressed the milestone-filtered list forever: ListIssues serves index-first when indexComplete is true, but it checked coverage only, never freshness. A card written before the Milestone field existed in the Card projection (or a lost update) won the fast path indefinitely while counters (separate store) and thread headers stayed right.

  • Persisted Index carries additive card_version (omitempty JSON; fixtures round-trip, law 5). indexComplete returns false when absent/older so the LIST fallback heals the window (header wins).
  • Only RepairIndex (new one-shot backfill: diffs every card vs its header, PR-kind cards preserved, compacted watermark respected, stamps current version) and fresh-index creation stamp the version; updateIndex/CompactIndex preserve it.
  • updateIndex/bumpMilestone failures are logged (optional Service.Log, nil-safe) + counted (IndexDrops/MilestoneDrops), never swallowed.
  • Concurrency: bounded CAS loops only, no locks (13 SS3/5).

Frontend (web/)

Deep-link choice (the issue's either/or): the LINK carries the promise. milestoneFilterHref emits ?milestone=<id>&state=all (label counts open+closed; all maps to the omitted wire param so the server returns both). Bare visits keep the #323 open-only default. Thread-sidebar chip routed through the shared helper; #416 dropdown binds verbatim; link survives refresh. No layout change, no new deps.

Docs

Amendments appended to docs/features/02_issues.md and docs/go/12_web_ui.md in the same commit (law 12).

Verification

  • go test ./internal/issues/ -race green (new indexfresh_test.go incl. -count=3 stress on new tests)
  • make cover: full gate green, issues 96.2% (≥95%)
  • web unit full-minus-smoke: 1335 pass / 0 fail (new milestone-deep-link-564.test.js; smoke excluded, needs live server)
  • vite build + esbuild SDK bundle green; web/dist/.keep restored
  • gofmt clean, go vet ./... + go build ./... clean
  • Note: make vet/make fmt wrappers unusable in this env (pnpm CLI bootstrap fails even on trivial commands; make fmt errors on clean trees via bare gofmt -w) — direct equivalents above all pass.
Fixes #564. ## Backend (internal/issues) Stale index cards suppressed the milestone-filtered list forever: ListIssues serves index-first when indexComplete is true, but it checked coverage only, never freshness. A card written before the Milestone field existed in the Card projection (or a lost update) won the fast path indefinitely while counters (separate store) and thread headers stayed right. - Persisted Index carries additive `card_version` (omitempty JSON; fixtures round-trip, law 5). indexComplete returns false when absent/older so the LIST fallback heals the window (header wins). - Only RepairIndex (new one-shot backfill: diffs every card vs its header, PR-kind cards preserved, compacted watermark respected, stamps current version) and fresh-index creation stamp the version; updateIndex/CompactIndex preserve it. - updateIndex/bumpMilestone failures are logged (optional Service.Log, nil-safe) + counted (IndexDrops/MilestoneDrops), never swallowed. - Concurrency: bounded CAS loops only, no locks (13 SS3/5). ## Frontend (web/) Deep-link choice (the issue's either/or): the LINK carries the promise. milestoneFilterHref emits `?milestone=<id>&state=all` (label counts open+closed; `all` maps to the omitted wire param so the server returns both). Bare visits keep the #323 open-only default. Thread-sidebar chip routed through the shared helper; #416 dropdown binds verbatim; link survives refresh. No layout change, no new deps. ## Docs Amendments appended to docs/features/02_issues.md and docs/go/12_web_ui.md in the same commit (law 12). ## Verification - go test ./internal/issues/ -race green (new indexfresh_test.go incl. -count=3 stress on new tests) - make cover: full gate green, issues 96.2% (≥95%) - web unit full-minus-smoke: 1335 pass / 0 fail (new milestone-deep-link-564.test.js; smoke excluded, needs live server) - vite build + esbuild SDK bundle green; web/dist/.keep restored - gofmt clean, go vet ./... + go build ./... clean - Note: make vet/make fmt wrappers unusable in this env (pnpm CLI bootstrap fails even on trivial commands; make fmt errors on clean trees via bare gofmt -w) — direct equivalents above all pass.
Backend (docs/features/02 SS2/P4): persisted Index carries card_version
(CardProjectionVersion); indexComplete refuses the fast path when absent
or older so the LIST fallback heals the window (header wins). Only
RepairIndex (new one-shot backfill diffing every card against its header;
PR cards preserved, compacted watermark respected) and fresh-index
creation stamp the version. updateIndex/bumpMilestone failures are logged
(optional Service.Log) + counted (IndexDrops/MilestoneDrops) instead of
swallowed. Additive omitempty JSON field: fixtures round-trip (law 5).
Concurrency: bounded CAS loops only, no locks (13 SS3/5).

Frontend (12_web_ui): milestoneFilterHref emits ?milestone=<id>&state=all
(the View-N label counts open+closed; bare visits keep the #323 open
default); thread-sidebar chip routed through the shared helper. #416
dropdown binds verbatim; link survives refresh.

Tests: internal/issues/indexfresh_test.go; web/test/unit/
milestone-deep-link-564.test.js. make cover gate holds (issues 96.2%).
RepairIndex had no production caller (dead backfill) and Service.Log was
never wired outside tests, so stale repos paid the LIST fallback on every
read forever with no log line. ListIssues now heals a version-stale index
best-effort while the header truth is in hand (small repos only,
empty scans never stamp, failures reuse the drop channel); newIssuesService
sets svc.Log = slog.Default () per the notify.go precedent. Docs amended
(02_issues.md, law 12); tests: read-path self-heal + no-stamp-on-empty.
Author
Owner

Independent review of 1d7eac2 vs #564 acceptance criteria — verdict: APPROVE (2 small review fixes pushed as dc77ee6, same branch).

BACKEND (all hold): card_version is additive omitempty JSON on issues/index.json — encoding/json ignores unknown fields both directions, absent reads as 0 = stale = safe-direction fallback; no protobuf touched (only testdata in repo is internal/store/proto, untouched) — law 5 OK. indexComplete gate adds zero store calls (pure in-memory check; fast path still index+counter = 2 GETs); the LIST fallback is failure-path-only — law 6 budgets untouched (no sim/budget file modified, no assertion weakened). RepairIndex: header-diff, PR-kind ride-through, compacted_through respected, bounded CAS (10) via casUpdate, no locks anywhere, nothing held across store calls. Logging/counters nil-safe (tests pin both). Coverage verified: 96.0% statements on internal/issues (≥95% holds).

FRONTEND (coherent end-to-end, traced): milestoneFilterHref ?milestone=&state=all → resolveIssueState(all)=all → issueListState=all→'' (omitted) → SDK qs skips '' → server absent-state = both — landing shows the promised open+closed set. Bare visits unchanged (#323 open default intact, query builder untouched). #416 dropdown binds search.milestone verbatim → selected on landing; all state in URL → refresh-safe. Thread-sidebar chip (Issue.jsx:614) + issues-row chips + both Milestones.jsx links all route through the one helper; no hand-rolled ?milestone= hrefs remain. No package.json change, no new deps — law 1 OK. No layout/CSS change (href-only), so themes + 390px hold by construction; no rendered browser proof (stated, same as PR).

TESTS (genuinely pin the fix): new JS test run against origin/main worktree fails 5/7 (the 2 passing are pre-existing-behavior regression guards: bare-visit default, dropdown binding). Go tests don't compile pre-fix (new symbols) and the gate cases contradict old logic by inspection (old indexComplete returned true for coverage-without-version). No budget assertions weakened.

DOCS: amendments in 02_issues.md + 12_web_ui.md in-commit (law 12); frozen primitives untouched (no route/auth/policy/event/task/CLI registry changes).

REVIEW FIXES pushed (dc77ee6): (1) RepairIndex had zero production callers — a backfill nobody can invoke, so stale repos (incl. the live one) would pay LIST on every read forever. ListIssues now self-heals a version-stale index best-effort while header truth is in hand (healStaleIndex: small repos only, next-1<=headerScanCap proves full scan coverage; empty scans never stamp an index on issue-less repos; failures reuse the drop channel, never surface). Precedent: updateIndex already does sync CompactIndex maintenance writes. (2) Service.Log was never wired outside tests — newIssuesService now sets svc.Log=slog.Default() (notify.go precedent), so drops are logs in prod, not just counters. Tests added: read-path self-heal + no-stamp-on-empty; -count=3 stress green. Full issues suite -race green, cmd/walhub tests green, node unit 1337/1338 (sole failure is the pre-existing live-server smoke test, fails on pristine 1d7eac2 too), gofmt/vet/build clean.

LIVE-INSTANCE angle: not exercised — repair path verified in unit tests only; no live instance reachable from here. Note the self-heal means the live instance recovers on its next milestone-filtered list read after deploy (no operator step). Themes/390px rendered check still open if you want belt-and-braces, but nothing in this diff can move layout.

Independent review of 1d7eac2 vs #564 acceptance criteria — verdict: APPROVE (2 small review fixes pushed as dc77ee6, same branch). BACKEND (all hold): card_version is additive omitempty JSON on issues/index.json — encoding/json ignores unknown fields both directions, absent reads as 0 = stale = safe-direction fallback; no protobuf touched (only testdata in repo is internal/store/proto, untouched) — law 5 OK. indexComplete gate adds zero store calls (pure in-memory check; fast path still index+counter = 2 GETs); the LIST fallback is failure-path-only — law 6 budgets untouched (no sim/budget file modified, no assertion weakened). RepairIndex: header-diff, PR-kind ride-through, compacted_through respected, bounded CAS (10) via casUpdate, no locks anywhere, nothing held across store calls. Logging/counters nil-safe (tests pin both). Coverage verified: 96.0% statements on internal/issues (≥95% holds). FRONTEND (coherent end-to-end, traced): milestoneFilterHref ?milestone=<id>&state=all → resolveIssueState(all)=all → issueListState=all→'' (omitted) → SDK qs skips '' → server absent-state = both — landing shows the promised open+closed set. Bare visits unchanged (#323 open default intact, query builder untouched). #416 dropdown binds search.milestone verbatim → selected on landing; all state in URL → refresh-safe. Thread-sidebar chip (Issue.jsx:614) + issues-row chips + both Milestones.jsx links all route through the one helper; no hand-rolled ?milestone= hrefs remain. No package.json change, no new deps — law 1 OK. No layout/CSS change (href-only), so themes + 390px hold by construction; no rendered browser proof (stated, same as PR). TESTS (genuinely pin the fix): new JS test run against origin/main worktree fails 5/7 (the 2 passing are pre-existing-behavior regression guards: bare-visit default, dropdown binding). Go tests don't compile pre-fix (new symbols) and the gate cases contradict old logic by inspection (old indexComplete returned true for coverage-without-version). No budget assertions weakened. DOCS: amendments in 02_issues.md + 12_web_ui.md in-commit (law 12); frozen primitives untouched (no route/auth/policy/event/task/CLI registry changes). REVIEW FIXES pushed (dc77ee6): (1) RepairIndex had zero production callers — a backfill nobody can invoke, so stale repos (incl. the live one) would pay LIST on every read forever. ListIssues now self-heals a version-stale index best-effort while header truth is in hand (healStaleIndex: small repos only, next-1<=headerScanCap proves full scan coverage; empty scans never stamp an index on issue-less repos; failures reuse the drop channel, never surface). Precedent: updateIndex already does sync CompactIndex maintenance writes. (2) Service.Log was never wired outside tests — newIssuesService now sets svc.Log=slog.Default() (notify.go precedent), so drops are logs in prod, not just counters. Tests added: read-path self-heal + no-stamp-on-empty; -count=3 stress green. Full issues suite -race green, cmd/walhub tests green, node unit 1337/1338 (sole failure is the pre-existing live-server smoke test, fails on pristine 1d7eac2 too), gofmt/vet/build clean. LIVE-INSTANCE angle: not exercised — repair path verified in unit tests only; no live instance reachable from here. Note the self-heal means the live instance recovers on its next milestone-filtered list read after deploy (no operator step). Themes/390px rendered check still open if you want belt-and-braces, but nothing in this diff can move layout.
Sign in to join this conversation.
No description provided.