Remove the misleading Review apply-rate metric - #2413
Merged
Conversation
The seam map is read instead of the codebase, so a wrong row misroutes an agent and a wrong Verify column lets an unproven change look proven. The false-green is the important one. The Frontend row's Verify column said `npx vitest --run`, but vitest.config.ts:15 excludes tests/pwa-generated-worker.spec.ts, so a vite.config.ts workbox change or an src/pwa/** change passes that command while its only contract test never runs. The column now says so and names `npm run test:pwa-generated-worker`. Added a PWA / offline / cache boundary row. The seam is four modules under src/pwa/, a public cleanup script, ApiCacheControlMiddleware and a behaviour contract doc, and it had zero occurrences in the map after #2350/#2381 and #2382/#2402. Its load-bearing invariant, that no runtime cache may ever admit an API response, was invisible to anyone reading only this file. Corrected four stale rows against main d629129: - Capture row pointed at views/InboxView.vue and views/ReviewView.vue, which are 12-line theme switches. Paper is the default skin (ADR-0038, paperThemeStore.ts DEFAULT_MODE = 'paper'), so the real surfaces are views/paper/PaperInboxView.vue and views/paper/PaperReviewView.vue with their subdirectories; the legacy views are named as opt-out fallbacks. - Transcript row said transcript input is held in LlmRequest.Payload "until #1305 links triage to Transcript". #1305 closed 2026-08-27 via PR #2144, which also deleted the RFAI-02 intent-envelope vocabulary, so the row was sending agents to look for something that no longer exists. - Smart CI row said the shadow lane was "landing with CI-02 #2326". It has landed and runs three checks on every PR. Also names the five smart-ci CLIs including resolve-merge-ref.mjs from #2401 / PR #2404. - Td* primitive count 17 to 18. Also: STATUS is 935 lines and the masterplan 2068, not the "~1.5k" and "~1.7k" the orient section claimed; added the two path-scoped rule files (.claude/rules/ci-control.md, .claude/rules/docs.md), which load by path rather than directory and were unlisted; added OUTSTANDING_TASKS.md, the strategy spine and the decisions index to the orient pointers. Stamped 2026-09-03 with an explicit rule that any edit bumps the stamp, including an add-only edit, since an unbumped stamp claims verification the edit did not perform. Verified: every path in the new row exists on main; vitest.config.ts:15 exclusion and the package.json test:pwa-generated-worker script read directly; InboxView.vue and ReviewView.vue confirmed 12 lines each; #1305 confirmed closed 2026-08-27; Td*.vue count 18; STATUS/masterplan line counts by wc -l. check-docs-governance, check-golden-principles and check-github-ops-governance all exit 0; git diff --check clean; every table row still has four columns. The Paper Color Audit does not scan autodoc/, so the backticked issue refs follow the file's existing convention safely.
Answers a confirmed Codex P1 on this PR. The row I added claimed the runtime caches "never admit an API response". That is false, and it is exactly the class of defect this PR exists to remove: a false safety property in the file agents read instead of the code. Proven, not argued. The deny test only matches a path starting with /api. The admit test is anchored on /assets/ and /icons/ but its middle segment spans any nested path, so an API base nested under either directory is admitted: /api/users/by-username/alice.png -> not cached (deny hits) /taskdeck/api/users/by-username/alice.png -> not cached (no dir match) /assets/api/users/by-username/alice.png -> CACHED /icons/api/users/by-username/alice.png -> CACHED The last two would enter the shared, cross-identity taskdeck-static-assets cache. The row now states the directory-anchored boundary, names the admitted case, and says not to deploy an API base under those two directories or to restate the invariant as unconditional. The same overstatement exists in the source comment at src/pwa/runtimeCachePolicy.ts and is mirrored in vite.config.ts and docs/platform/PWA_OFFLINE_BEHAVIOR.md. That is a code-surface fix in a seam this docs PR does not own, so it is tracked separately rather than fixed here. Also fixed a table break I introduced with the first attempt at this correction: the regex alternation put nine extra pipe characters into a markdown table cell, splitting the row into fourteen columns. Restated without regex syntax. Every row in the seams table is back to four columns, verified by pipe count. Verified: the four cases above executed against the exact regexes from vite.config.ts:115-117; check-docs-governance and check-golden-principles exit 0; git diff --check clean.
Chris0Jeky
commented
Sep 3, 2026
Chris0Jeky
left a comment
Chris0Jeky
left a comment
Owner
Author
There was a problem hiding this comment.
Fresh-context review of exact head 6b5ff7af43ee6022cd893cd2933861cb53066208 against merge-base 4c6ce5f83e7e393c4d5790d7dd4afe9f2d97af15: 0 CRITICAL/HIGH findings; no review merge blocker.
The diff is exactly the six claimed Review rail, locale, and test files. It removes the dead Apply-rate claim while retaining ReviewMiniCadence; Similar Past and Today metrics are untouched. The current-base merge added only #2299's six disjoint Inbox files and did not alter any owned path.
Focused Review tests passed 163/163, catalog tests passed 19/19, and typecheck, scoped ESLint, build, and diff checks passed. The remaining visual observation is non-blocking: the truthful “This week” heading can remain when no cadence bars exist. No browser/Playwright pass was run.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
3 tasks
Round two, from an independent verification of every remaining claim in the
new PWA row. The row was substantially accurate; these are the places it
claimed more than the code enforces. Two of them would have caused a wrong
change.
The important one is another false green, the same class this PR exists to
remove. src/pwa/runtimeCachePolicy.ts ships NOTHING: Workbox serializes the
urlPattern callbacks out of vite.config.ts, so the inlined copies there are
what reaches sw.js. An agent closing the /assets/api/... gap in the policy
module alone, then running exactly the commands my Verify column listed,
would have seen green from src/tests/pwa/, from startupCacheBoundary.spec.ts
and from test:pwa-generated-worker, and shipped a security fix that changed
nothing. The column now states the mirror relationship and names
tests/runtime-cache-policy-parity.spec.ts, which is the only test that pins
the two copies together.
The second would have deleted working code. The row said "a one-shot read of
registration.waiting would race and never deliver", unqualified, while
legacyApiCacheWorker.ts:191 deliberately does that read to catch a
replacement already waiting because the user dismissed the update banner. The
real claim needs "at the moment update() resolves". Left as written, an agent
tightening the seam removes :191; a user who dismissed the banner then gets no
updatefound on an unchanged script, the 12s deadline expires, the purge
reports failure, and router/index.ts:390 clears credentials on every
navigation. The row now marks that read as deliberate and says not to delete
it, and states that the migration is fail-closed and gates identity, which it
did not say at all.
Also corrected in the row: the cache is not origin-anchored either
(predicates test url.pathname only and cacheableResponse is
{statuses: [0, 200]}, so an opaque third-party response under a matching path
is admitted); "two runtime handlers" scoped to Workbox runtimeCaching, since
share-target-handler.js writes a third cache; dropped "build-owned", which
implied a first-party scope the predicate does not have; and #2411 now cited
by number rather than "tracked separately".
docs/platform/PWA_OFFLINE_BEHAVIOR.md is corrected in the same commit because
the row names it as the behaviour contract while it still carried the exact
absolute claim the row exists to qualify, on lines 17 and 25. Following the
row's own pointer landed on "never admits an API response". A correction that
is undone one hop away is not a correction. No open PR touches that file,
src/pwa/ or vite.config.ts, so there is no lease conflict.
Verified: every claim re-checked against code by an independent pass;
ApiCacheControlMiddleware.cs:10 confirmed to set the literal string
"no-store, private"; vitest.config.ts:15 exclusion and package.json:27
confirmed; runtime-cache-policy-parity.spec.ts confirmed present and NOT in
the exclude list, so it does run in the default suite. Both markdown tables
re-checked by pipe count. check-docs-governance, check-golden-principles and
check-github-ops-governance exit 0; git diff --check clean.
Not verified: no test or build executed; no built dist/sw.js inspected.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b5ff7af43
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two merges landed without canonical entries, and one of them made an existing STATUS sentence untrue. The false sentence is the important half. The #2202 entry said of the Legacy batch-triage reconciliation: "this is not a poll, so an item finishing after that fetch still needs the existing refresh paths." PR #2299 (merge 4c6ce5f) made it a poll. Left standing, the source of truth would have contradicted shipped behaviour on the exact seam #2230 is about. Retracted in place with a pointer rather than deleted, so the supersession is legible. Added the #2388 entry: an 8-second deadline on each background Review queue request, the last trustworthy queue preserved behind a polite degraded notice after three consecutive transient failures, cleared by any successful poll or explicit load, with 403 authority and teardown/supersession semantics intact in both skins. The Paper hardcoded-English stale copy stays a known MEDIUM on #2214 item 4. That entry also records something worth knowing: PR #2388's body asserted "docs/STATUS.md now records PR #2388" and it did not. Merge 4b80756 changed nothing in this file, and no mention of the deadline or the degraded state existed anywhere in it. Verified with git show on the merge and by grep. Added the #2299 entry with the observedPostEnqueueList guard that fixed the twice-parked defect, and, deliberately, why #2230 stays open: the poll's 60-second wall is shorter than the server's worst-case drain (at most MaxBatchSize items total per tick then a QueuePollIntervalSeconds sleep, shipped 5 and 5, shared between capture and non-capture work, against a 50-item accepted batch), and expiry is silent, so a large batch can expire back into the cached Triaging detail the issue exists to remove. The two Codex P2s are recorded as declined-unreachable with the citation. Verified: check-docs-governance and check-golden-principles exit 0; git diff --check clean; both new entries and the retraction present by grep; the merge SHAs, the absence of any STATUS change in 4b80756, and the shipped worker settings all re-read this session. Not verified: no test, build or application run. The batch-drain timing is arithmetic over committed configuration, not an observed run.
4 tasks
Chris0Jeky
commented
Sep 3, 2026
Chris0Jeky
left a comment
Chris0Jeky
left a comment
Owner
Author
There was a problem hiding this comment.
Current-base interaction recheck of exact head b9ab692df972f04d5ecdd20f88d7f759c145ba02 against merge-base a32b4818df04fffeab348ed63012fafd6bd18631: 0 CRITICAL/HIGH findings; no new merge blocker.
The incoming base is exactly #2410's nine backend files and changes none of the six #2205 frontend paths. The PR diff remains exactly those six paths. Current-head Review tests passed 163/163, catalogs passed 19/19, and scoped ESLint, build/typecheck, source guard, and diff checks passed.
The already triaged “This week” without bars observation remains a valid non-blocking P2 and is declined for this bounded cycle. Exact-head hosted CI is running.
…icting Answers a confirmed Codex P2. Recording #2388's delivery made the adjacent #2298 entry false: line 91 still listed per-request deadlines and stale/degraded queue disclosure as open on #2214 while the new entry recorded both as shipped. In the file that is the declared source of shipped reality, two adjacent entries disagreeing about remaining scope would send an implementer to build something that already exists. I declined this pruning when opening the PR, on the grounds that #2388's own claim about what it removed could not be verified. That reasoning still holds for the claim; it does not hold for these two items, which I verified directly against main rather than against the PR body: REVIEW_QUEUE_REQUEST_DEADLINE_MS = 8_000 useReviewProposals.ts:53 raced via Promise.race :590, cleared :592 consecutiveQueueRefreshFailures :543-560 degraded indication cleared on success :519-520 So exactly two items are struck, each with its evidence inline, and the strike is attributed to #2388 with its merge SHA so the supersession is traceable rather than looking like a silent edit. The stale copy's Paper localization stays open as #2214 item 4, because it is hardcoded English. Deep-link re-authorization, count-neutral replacement announcements and the low-impact accessibility/diagnostic residuals are untouched and still listed. Nothing else was pruned. Verified: the four line references read from main this session; check-docs-governance and check-golden-principles exit 0; git diff --check clean.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9ab692df9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Map the PWA cache boundary and fix a false-green in the seam map.
…d-plan Treat superseded Smart CI plans as non-red in shadow
…-text Reconcile capture text before disposition stamps
Bound board detail loads and cancel stale reads
…to codex-2205-remove-apply-rate-resume
This was referenced Sep 3, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
No decisions yetempty claimTests
npm run typecheck: passednpm run build: passed, 810 modules; pre-existing ineffective dynamic-import warning onlygit diff --check: passedReview
Fresh-context review of exact head
b577db14627c5212d0625ed939bc2e29337fe30bagainst currentorigin/main07534d9c653e91e827ffc19a78ada78b731bc498found no CRITICAL or HIGH defect. The branch contains that main head and the live comparison is exactly the six claimed Review rail, locale, and test files. PRs #2417 and #2419 are disjoint from this seam. Automatic Codex review completed with no finding.The truthful
This weekcadence heading without bars remains a declined nonblocking observation. Similar Past and Today Apply-rate metrics are separate and unchanged.Docs
No canonical documentation changed. This is the accepted removal path recorded on #2205. The canonical STATUS correction is already handed to its active owner.
Verification boundary
Closes #2205