Proposal B PR-2 + PR-3: manual-override UI + preview badge (recovered stack) - #73
Merged
Merged
Conversation
The branch this was originally scoped against (claude/print-preview-
xml-2) sits at the Proposal D commit with zero Proposal-B-specific code
or design doc - the queue's "resume Proposal B to completion" gloss
("export-time per-side bleed normalization") is compatible with two
genuinely different features with very different risk profiles, and
guessing which one blind isn't a reasonable substitute for a spec that
never reached this session.
Surveys the current uniform-scale bleed handling in PDF.tsx, the
backend's already-validated whole-image bleed/trimmed classification
(reusable technique, but binary and currently unpopulated in
production), and proposes two candidate designs: automatic per-edge
detection+auto-correct (higher risk - a wrong heuristic silently
mis-crops real print orders, no validated real-image sample to trust
it against yet) vs. manual per-side controls in the existing Proposal A
WYSIWYG preview (recommended MVP - no heuristic-correctness risk,
additive to layout.ts/PagePreview.tsx/PDF.tsx). HOLD pending the
owner's choice between them.
…ring)
Implements the approved spec (docs/proposals/proposal-b-bleed-
normalization.md, recovered after a courier loss): measures each card's
real per-side bleed via probe-median + IQR-ambiguity sampling
(bleedNormalize.ts), resolves a trim/extend plan against the target
bleed with a fallback prior and three manual-override modes, and
synthesizes the corrected image via canvas crop+edge-extension
(bleedExtension.ts). Wired into PDF.tsx's PDFCardImage for full-
resolution Google Drive/local-file renders, replacing the old uniform
proportional rescale for those cards.
26 new tests (all 6 required synthetic fixtures + override modes +
geometry math + pdfImage.ts's new getPDFImageBlob split), zero
regressions in the existing 267.
Two real bugs found and fixed via actual render verification, not just
unit tests: (1) a confident-but-wrong measurement could ask to trim
more than a small source image actually has, producing a negative
canvas dimension - clamped defensively in computeBleedExtensionGeometry.
(2) @react-pdf/renderer's own stylesheet parser has a genuine bug where
a single-token transform value ("none") throws inside its layout engine
without ever propagating as a rejection, silently hanging the whole
render - caught only by running tests/PDFGenerator.spec.ts's real
Playwright suite (which hung at timeout) against a stashed before/after
baseline; fixed by omitting the transform key instead of passing
"none". Documented in docs/lessons.md as a reusable cross-session
finding.
Still not built (flagged, not silently dropped - see the proposal
doc's "Shipped vs. not yet built"): the main-thread batch resolution of
bleedPriors via APIGetTagConsensus, the manual-override UI + project-
state persistence, and the WYSIWYG preview badge.
Main-thread, concurrency-bounded batch fetch of each export card's appropriate-bleed machine-vote lean via the existing APIGetTagConsensus endpoint - no new endpoint, per the approved spec. Populates PDFProps.bleedPriors (built in PR #66, previously always undefined in real exports) so ambiguous sides use a real per-card lean instead of always falling through to the safe "unresolved" default. Runs on the main thread (PDFGenerator.tsx's downloadPDF/saveToDrivePDF, before the render worker is invoked) because APIGetTagConsensus's CSRF header needs document.cookie, which doesn't exist inside pdf.worker.ts's Worker context - the resolved plain map crosses that boundary the same way every other PDFProps field already does. New: common/concurrencyLimit.ts (a general-purpose bounded-concurrency map, kept separate from GoogleDriveService's own private Semaphore to avoid expanding this PR into an unrelated refactor), features/pdf/ bleedPriorResolution.ts (netPolarity -> BleedPrior mapping, per-card failure tolerance so one bad lookup never fails the whole export). 13 new tests, 282/282 passing overall. Verified against the real render path, not just unit tests: tests/PDFGenerator.spec.ts has no tagConsensus mock at all, so every lookup genuinely fails during that suite - it still passes at the same timing as before this PR, confirming the failure-tolerance path works end to end, not just in isolation. Full report: docs/reports/proposal-b-pr1-bleed-prior-batch-resolution.md
Per the owner's explicit instruction: PR-1's batch resolution issues one request per unique export card (~517 for the owner's own large project). Acceptable for v1; a batch-consensus endpoint is the eventual answer if this ever actually hurts, not worth building speculatively ahead of evidence.
Per-card Auto/Force bleed/Force trimmed control in the PDF export panel (new "Bleed Overrides" AutofillCollapse section, listing only cards bleed normalization can apply to), persisted in project state per decision 4 (survives reload) via a new manualOverrides field on projectSlice + localStorage helpers + a listenerMiddleware listener, mirroring favoritesSlice's existing pattern exactly. Wired into PDFGenerator's bleedOverrides prop. XML round-trip flagged, not built, per instruction.
Hedged "Bleed will be generated" badge on PagePreview.tsx's fast/CSS preview, per the audit's suggestion-vs-confirmed vocabulary. New pure willLikelyGenerateBleed(prior, manualOverride) mirrors resolveBleedPlan's precedence minus the per-side measurement (not available to the cheap preview); PDFGenerator.tsx resolves priors for just the visible preview page, debounced, and combines with PR-2's manual overrides. Proposal B is now complete end to end. Also: docs/lessons.md entry for the page.reload()/second-load-event Playwright hang found while verifying PR-2.
WilfordGrimley
changed the base branch from
claude/e3-bleed-override-ui
to
master
July 18, 2026 17:53
…view-badge # Conflicts: # docs/features/pdf-generator.md # docs/lessons.md # docs/proposals/proposal-b-bleed-normalization.md # docs/reports/proposal-b-pr1-bleed-prior-batch-resolution.md # frontend/src/features/pdf/PDFGenerator.tsx # frontend/src/features/pdf/bleedNormalize.test.ts # frontend/src/features/pdf/bleedNormalize.ts
WilfordGrimley
changed the title
…st unloaded CardDocuments BleedOverrideSettings's eligibility filter accessed cardDocument.sourceType without checking for undefined - useCardDocumentsByIdentifier's underlying selector (selectCardDocumentsByIdentifiers) maps every project member identifier to its CardDocument, including identifiers whose document hasn't finished loading into the store yet (mapped to undefined). The fast preview's own eligibility filter (fastPreviewEligibleIdentifiers, same file) already guarded against this; this one didn't, throwing "Cannot read properties of undefined (reading 'sourceType')" the instant the panel rendered before every card had loaded - exactly what PagePreview.spec.ts's fast preview tests do (no debounce, first paint). Diagnosed via a saved Playwright error-context.md showing a Next.js dev-mode "Unhandled Runtime Error" overlay sitting on top of the (CSS- visible but never actually rendered) page-preview element underneath - the same false-positive-visible diagnostic pattern documented in docs/lessons.md from an earlier unrelated bug this session. Also reformats tests/PDFGenerator.spec.ts (unrelated prior formatting drift, not touched otherwise). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014dB7mGPQyoML2Ri5ZrprdK
…flicts Both conflicts (proposal doc, PDFGenerator.tsx) are additive-both-sides: PR-3's own shipped-narrative + task #134's calibration findings in the doc (combined, not competing); PR-2's BleedOverrideSettings component vs #81's ImageFailureConfirmModal component in the TSX (both kept, unrelated features). tsc clean, bleedNormalize.test.ts 14/14, PagePreview.spec.ts 3/3 (previously the real failure from checkpoint-2 - now passing, the frontend session's fix confirmed landed).
This was referenced Jul 18, 2026
Merged
WilfordGrimley
added a commit
that referenced
this pull request
Jul 20, 2026
Top summary paragraph said manual-override UI/persistence + preview badge "remain", contradicting the same file's own "Shipped vs. not yet built" section (both shipped as PR #73). Correct the summary and bump README's status table row from BUILDING to PARTIAL to match the three genuinely open items (XML round-trip, two edge cases).
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
2 participants
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.
Recovery note: this PR's original base branches (
claude/e2-bleed-prior-batch-resolution,claude/e3-bleed-override-ui) were deleted when their own PRs (#69, #71) got squash-merged/closed by another concurrent session — GitHub auto-closes a stacked child PR when its base branch disappears, and won't allow reopening (seedocs/lessons.md's "A stacked PR's base branch gets deleted out from under it" entry). Retargeted this PR tomasterdirectly before its own base (e3) could suffer the same fate. This PR now carries PR-2's and PR-3's full content in one diff againstmaster, plus a resolved merge conflict against master's own independent recovery of PR-1 (#72) — verified clean,mergeable_state: "clean".Full reports:
docs/reports/proposal-b-pr2-bleed-override-ui.md,docs/reports/proposal-b-pr3-preview-badge.md.Description
PR-2 — manual-override UI + persistence: per-card Auto/Force bleed/Force trimmed control in the PDF export panel ("Bleed Overrides" collapsible section,
PDFGenerator.tsx), persisted in project state (survives reload) via a newmanualOverridesfield onprojectSlice+ localStorage helpers + alistenerMiddleware.tslistener, mirroringfavoritesSlice's existing pattern. XML round-trip intentionally not built — flagged per instruction.PR-3 — WYSIWYG preview bleed badge: a hedged "Bleed will be generated" badge on
PagePreview.tsx's fast/CSS preview, consuming PR-1's resolved priors and PR-2's manual overrides via a new purewillLikelyGenerateBleed(prior, manualOverride). With this, Proposal B is complete end to end.Checklist
npx tsc --noEmit: cleannpx eslint: 0 errors, 0 new warningsnpx jest --runInBand: 306/306 passingnpx playwright test tests/PDFGenerator.spec.ts --workers=1: 7/7 passing (one isolated re-run needed for a resource-contention flake unrelated to this PR's changes)