PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: This is the correct complementary half of the #12309 fix. #12310 (merged) corrected the generator; this PR fixes the consumed reality the operator actually sees — the committed index.json was stale on dev, and the service worker served /resources/ cache-first so even a corrected file reloaded stale. Both layers are now pinned with recurrence guards. No blocking defects; the one cross-consumer concern is non-blocking (safe via cache-fallback). Approve over Approve+Follow-Up because no concrete follow-up is required now (the cross-consumer note is a watch-item, not a tracked gap).

Peer-Review Opening: Strong, self-aware fix — the "Evolution" section naming this a verification-boundary error (generator proven in isolation, consumed JSON + SW cache layer left unverified) is exactly the right framing, and the new committed-data guards make sure the boundary stays pinned. Approving; one non-blocking cross-consumer note below to keep on the radar.

🕸️ Context & Graph Linking

• Target Issue ID: Resolves #12309 (leaf bug, not epic — valid close-target)
• Related Graph Nodes: #12310 (merged generator half), Epic #12204 (Portal news), #12313 (my sibling pulls-theme fix — no overlap)

🔬 Depth Floor

Challenge: The /resources/data/ addition to ServiceBase.networkFirstPaths is a base-level default, so it flips cache behavior for every Neo app's /resources/data/, not just the portal — a cross-consumer default change (shared-build default = cross-consumer contract). It is safe because it is network-first with cache fallback (offline still resolves, and it re-caches), and it is correct for content-synced app data. But any consumer serving large, rarely-changing data under /resources/data/ now pays an online network round-trip per load (cache only on failure). Non-blocking — the data-freshness semantics justify the default — but worth an explicit awareness note; if a future consumer needs cache-first data under that path, a portal-scoped override or a cacheFirstPaths exception list would be the surgical follow-up (hypothesis — needs V-B-A before implementation).

Secondary (minor): the interceptor matches via request.url.includes('/resources/data/') (substring) — broad, though the path is specific enough that accidental matches are unlikely.

Rhetorical-Drift Audit (§7.4):

• PR description: the L3 framing ("Neural Link runtime inspection ... found stale Portal.store.Pulls chunk order after reload") matches a genuine runtime-inspection claim; not overshooting the diff.
• Anchor & Echo: the networkFirstPaths JSDoc accurately states the mechanical reason (framework-version-independent data must reflect fresh syncs).
• [RETROSPECTIVE]: n/a (no tag); the "Evolution" prose accurately characterizes the verification-boundary miss.
• Linked anchors: #12310 genuinely is the generator half it cites.

Findings: Pass.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: The decisive lesson is the verification boundary: a generator/unit fix proven in isolation is not proven in the consumed substrate. The visible Portal runs through (a) committed index.json and (b) the service-worker cache layer — both must be verified, not just the generator output. This PR's committed-data order guards + SW-policy test institutionalize that boundary so the next chunk-order change can't regress silently.
• [KB_GAP]: "Generator output ≠ consumed data" — committed apps/portal/resources/data/** is the runtime source of truth; a generator change is inert until the committed artifact is regenerated AND the SW cache policy lets the fresh copy through.

N/A Audits — 📑 📡 🔗 🛂 🔌

N/A across listed dimensions: bug fix with no new public-API contract (Contract Ledger), no openapi.yaml touch (MCP budget), no new skill/convention (Cross-Skill), no novel architectural abstraction (Provenance), and the SW change is a cache-policy default, not a wire-format/payload-schema change.

🎯 Close-Target Audit

• Close-targets identified: Resolves #12309 (own line, valid syntax)
• #12309 confirmed not epic-labeled (leaf bug) → valid close-target

Findings: Pass. Observation (non-blocking): both #12310 (merged) and this PR carry Resolves #12309. This is not a premature-close hazard — PRs target dev, not the default branch, so GitHub does not auto-close on dev-merge; #12309 closes once at the dev→main release, to which both PRs legitimately contribute (complementary halves). Verified: #12310 MERGED 2026-06-01T16:15Z, #12309 still OPEN.

🪜 Evidence Audit

• PR body contains an Evidence: declaration (L3 Neural Link runtime inspection → L3 required; residual = post-merge SW update confirmation).
• Residual explicitly listed in ## Post-Merge Validation (reload Portal under the SW, confirm descending chunk ranges for pulls + tickets).
• Two-ceiling distinction honored: shipped at L3 with the residual being the browser-controlled SW update (genuine post-merge boundary, not under-probing).

Findings: Pass — Evidence declaration present and honest; the post-merge residual is a real browser-SW-lifecycle boundary, appropriately deferred.

🧪 Test-Execution & Location Audit

• Branch checked out locally (gh pr checkout 12311, head bbcef046c).
• Canonical location: generator guards in test/playwright/unit/ai/buildScripts/docs/index/, SW test in test/playwright/unit/worker/ — both correct per unit-test.md.
• Ran the 3 related specs: 8 passed (940ms) — TicketIndex + PortalContentIndexes (committed-index chunk-order guards + the #12310 generator tests) + ServiceBase (network-first-before-generic-cache).

Findings: Tests pass; locations correct.

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 92 — extends the existing networkFirstPaths config + reuses the committed-data-guard test pattern; idiomatic. 8 deducted: /resources/data/ lands on the BASE default, changing cache behavior for every Neo app (cross-consumer flip) where a portal-scoped override would be more surgical — mitigated by network-first-with-cache-fallback.
• [CONTENT_COMPLETENESS]: 95 — Fat body (root cause, deltas, evidence ladder, evolution, post-merge validation, commit); networkFirstPaths JSDoc updated with the data-freshness rationale. 5 deducted: the new expectChunkFoldersDescending guard's >1 assertion silently couples to the committed data always having a multi-chunk group; no JSDoc note on that assumption.
• [EXECUTION_QUALITY]: 92 — checked out; 8 related tests green locally; committed-data guards + SW-policy test + L3 Neural Link verification of the actual stale-cache failure mode. 8 deducted: substring includes('/resources/data/') match is broad; the guard's data-coupling noted above.
• [PRODUCTIVITY]: 100 — closes the exact verification-boundary gap #12310 left (stale committed data + SW cache-first). I considered whether it overshoots scope (no), under-delivers (no — data + cache + guards all addressed), or misses an AC (no — both post-merge items map to the operator's reported symptom); none apply.
• [IMPACT]: 60 — fixes a visible Portal regression + a service-worker caching-correctness gap + adds recurrence guards; substantive but localized to portal data + one SW config entry.
• [COMPLEXITY]: 45 — Moderate: three layers (committed-data regen, SW cache policy, test guards), each delta small and well-isolated; the SW cache-layer reasoning is the main cognitive load for a reader.
• [EFFORT_PROFILE]: Quick Win — small surface, high ROI: fixes the visible regression and pins both verification boundaries (consumed JSON + SW cache) against recurrence.

Approved — clean, well-tested, and the verification-boundary guards are the durable win here. Nice catch on the SW cache layer; that one would have been easy to miss. Over to @tobiu for the merge gate.