Repository navigation
docs(scanner): record #219's rulings and measurement mode's local-only rule (#231) - #236
Merged
Merged
Conversation
The replay page and its upload skip sign-in, so they are held to this machine on top of measurement mode being on (#218 ruling 6). The existing example covered the strip and photo routes only; removing the replays' before_action now fails this one. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The comment said the worker is kept across Turbo visits, but the scanner's head asks for a full load on every visit so the camera is released (spec 007 FR-2), which creates the worker again. #219 ruling 2. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…y rule (#231) The policy's extra directives, one OCR worker per page load and the read-set preference are rules decided by #219; measurement mode's strip, photo and replay routes answer local requests only, by #218 ruling 6. Each names its issue, #218 and #219 are in Sources, and the no-spec-source marks are replaced. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ule and the 404; narrow what is pinned (#231) Implementation review: spec 007's contract already lists the policy header and marks the replay and strips local-only, and spec 009's contract gives Other printings' 404, so only the photos route has no spec text. The worker is never terminated, and the read-set spec doesn't pin English-only. The tenancy doc's photos/replays/strips row no longer says ruling 6 has no source. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ain open (#231) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Keeps both sides in tenancy-and-accounts.md: main's #230 wording, minus ruling 6's clause, which the scanner doc records; three no-spec-source behaviours remain. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
plainprogrammer
marked this pull request as ready for review
November 9, 2026 17:08
This was referenced Nov 9, 2026
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
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.
Closes #231
Refs #218, #219
Records the maintainer's rulings from #219 (1–3) and #218 (6) in
docs/architecture/scanner.md, corrects one comment, and adds one guard example. No spec and no ADR (routing confirmed by the coordinator); no shipped spec is edited (spec 027 D4).What changed
docs/architecture/scanner.mdobject-src,base-uri,form-action,frame-ancestors,font-src,img-src'sdata:andblob:) are recorded as policy decided by Scanner: three behaviours no spec describes (CSP's extra directives, the OCR worker's lifetime, a name candidate's printing) #219 ruling 1, alongside spec 007 AC-2.4, its contract and ADR 0001.spec/models/mtg/reading_spec.rb.app/javascript/scanner/recognition.js: the header comment no longer says the worker is "kept across Turbo visits".spec/requests/scanner/measurements_spec.rb: one new example for the replay page and upload (non-local gets 404, a signed-out local request works).app/controllers/concerns/scanner_page.rb,spec/requests/footer_spec.rb: comments cite Scanner: three behaviours no spec describes (CSP's extra directives, the OCR worker's lifetime, a name candidate's printing) #219 ruling 1.docs/architecture/tenancy-and-accounts.md: ruling 6 leaves the "no spec source" list (three behaviours remain there). I mergedorigin/mainafter Sign-up refuses a missing password confirmation; record the rate-limit store as a rule #230 and kept its wording.docs/lessons/2026-11-09-no-spec-source-claims-need-the-contracts-and-sibling-docs.mdEvidence the guards can fail (temporary edits, reverted, not committed)
spec/requests/footer_spec.rb("under the scanner's Content-Security-Policy") already matches the whole header exactly, includingdata:andblob:. Withpolicy.frame_ancestors :selfdeleted fromScannerPage, both of its examples fail (2 examples, 2 failures).before_action :require_local_requestdeleted fromReplaysController, only the new example fails (18 examples, 1 failure). The existing AC-5.6 example covers the strip and photo routes only.Implementation review
Read-only, Opus. Verdict: NEEDS CHANGES, all wording or test-comment findings, fixed in
9fe4912ande947f03. It checked the four measurement controllers, the CSP directives and its spec, the OCR worker's lifetime,name_printings, and the spec citations.tenancy-and-accounts.mdstill said ruling 6 had no source (a table row and the counts); fixed.reading_spec.rbpins (English-only isn't pinned, and a card with no English printing isn't a candidate); the new example's comment gave sign-in as its reason while the example was signed in (now it signs out before the last request).No second review round was run: the fixes were wording and one example line.
Verification
bin/rspec --dry-run: 2839 examples, 0 failures; the fullbin/cipassed in the pre-push hook on4c99247.Owed to the maintainer
🤖 Generated with Claude Code