Bleed plan: promote dimension-derived measurement, demote probes to advisory-only (task #134) - #82
Merged
Conversation
…dvisory-only (task #134) Task #134's real-image calibration proved the probe walk's ~2x overshoot is structural, not a tunable threshold: a card's own border is typically the same flat color as its bleed extension (deliberately, so print misalignment doesn't show a seam), and a color-run walk can't tell them apart. Sweeping RGB_DISTANCE_THRESHOLD across a 4x range moved the sample median under 3%, ruling out "threshold too loose". resolveBleedPlan's plan-input hierarchy flips: the source image's own pixel dimensions (classifyBleedAspectRatio, mirroring the backend's classify_bleed_edge aspect-ratio method/constants exactly) are now PRIMARY - a card's file dimensions don't depend on its border color, so they're immune to the same confound. Probes are demoted to two advisory roles with zero trim authority: per-side ambiguity still forces the prior/manual-override fallback (preserves the original degenerate/ full-art handling), and a new detectBleedAsymmetry flag marks a card for manual review without touching the plan (no UI consumer wired yet, same as E-2's degradedQueries). OVERSIZED_MULTIPLE keeps its bad-DPI-guard meaning, now checked in both directions since a dimension-derived value can go negative in a way a probe run length never could. resolveBleedPlan gains sourceWidthPx/sourceHeightPx params - the one production call site (bleedExtension.ts) already has the decoded bitmap's own width/height in hand. Full design decision + fixture rationale in docs/proposals/proposal-b-bleed-normalization.md's "Owner design decision" section; diagnosis in docs/reports/2026-07-18-bleed-calibration-134.md (already on master). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014dB7mGPQyoML2Ri5ZrprdK
…ion-basis-134 # Conflicts: # docs/lessons.md # frontend/src/features/pdf/bleedNormalize.test.ts
…fix ExportImages.tsx twin bug useCardDocumentsByIdentifier is keyed by every project member identifier, including ones whose CardDocument hasn't finished loading into the store yet (selectCardDocumentsByIdentifiers maps those to undefined). The declared return type claimed otherwise, hiding this from every caller's type-checker - task #135 found and fixed one resulting crash (PDFGenerator.tsx's BleedOverrideSettings) without the compiler's help; ExportImages.tsx had the identical unguarded cardDocument.sourceType access, latent and untested. Tightened the hook's (and its underlying selector's) declared return type to CardDocument | undefined so the compiler surfaces every real call site. Fixed what it found: - ExportImages.tsx: guarded the eligibility filter (the actual twin bug), added a regression test that reproduces the crash directly (confirmed it fails without the guard) and asserts only the loaded card is queued. - CardResultSet.tsx: a local annotation claimed `CardDocument | null` when the real type (and the `!= null` check already guarding it) was `| undefined` - corrected, no behavior change. - PDF.tsx, SCMPDF.tsx, PDFGenerator.tsx, clientSearchService.ts: widened cardDocumentsByIdentifier's type through the render pipeline to match; all of these already filtered out undefined entries safely at runtime (paginate* functions, SCMPDF's resolve()) - purely a type-annotation correction, no logic added. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014dB7mGPQyoML2Ri5ZrprdK
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.
Description
Fixes the live-harm risk task #134's calibration flagged:
resolveBleedPlanwas computing trim-inward plans on the common case (28/30 of the calibration sample), cropping ~2-3mm into real bled cards' border content.Task #134's real-image calibration (
docs/reports/2026-07-18-bleed-calibration-134.md, already on master) proved the probe walk's ~2x overshoot is structural, not a tunable threshold: a card's own border is typically the same flat color as its bleed extension (deliberately — so print misalignment doesn't show a seam), and a color-run walk can't tell them apart. SweepingRGB_DISTANCE_THRESHOLDacross a 4x range moved the sample median under 3%, ruling out "threshold too loose" before it could be mistaken for the fix.Owner-directed design decision, implemented here: measurement demoted, dimensions promoted.
resolveBleedPlannow classifies the source image's own pixel dimensions viaclassifyBleedAspectRatio— the same aspect-ratio method and reference constants (TRIM_ASPECT_RATIO≈0.7159 /BLEED_ASPECT_RATIO≈0.7350, abstain past 0.03) as the backend's already-validatedclassify_bleed_edge. A non-abstain classification drives the plan directly viadimensionDerivedBleedMM—(px_dim − trim_mm·dpi/25.4)/2, symmetric per axis. On the calibration's own 28-card bleed sample this resolves to near-no-op plans (correct — they already carry ~target bleed).detectBleedAsymmetry, exported and unit-tested, flags a card for manual review without altering the plan — no UI consumer wired yet, same pattern as E-2'sdegradedQueries.OVERSIZED_MULTIPLEkeeps its bad-DPI-metadata-guard meaning, now checked in both directions (a dimension-derived value can go negative in a way a probe run length never could).bleedNormalize.test.tsrewritten: probes-demoted regression fixture, a real-dimension fixture using Evil Twin's actual 460dpi calibration numbers asserting a near-no-op plan, axis-independence, bidirectional oversized-guard, and the preserved ambiguity-forces-fallback case — 19itblocks in this file (up from 8), all passing.docs/lessons.md+docs/proposals/proposal-b-bleed-normalization.mdupdated with the design decision and its generalized lesson (measurement blind spots that are invariant across a threshold sweep point at the method, not the constant).Tracked, not built by this change: the bottom-edge/typeline asymmetry the calibration observed but didn't explain; the card-back false-positive shape.
willLikelyGenerateBleed(PR-3's preview badge hedge) is unaffected — it's a categorical hedge that never calledresolveBleedPlanor touched pixel data.resolveBleedPlangainssourceWidthPx/sourceHeightPxparams — the one production call site (bleedExtension.ts'snormalizeCardBleed) already has the decoded bitmap's ownwidth/heightin hand, no new fetch or decode needed.Rebase update: type-tightened useCardDocumentsByIdentifier + fixed ExportImages.tsx twin bug
Rebased onto master after PR #73 merged (brought in
BleedOverrideSettings, which task #135 fixed for an unguardedcardDocument.sourceTypeaccess on a sparse identifier map). Bundled in the follow-up the deviation in that fix flagged but didn't sweep in:useCardDocumentsByIdentifier's declared return type tightened to{ [identifier: string]: CardDocument | undefined }— it's keyed by every project member identifier, including ones whoseCardDocumenthasn't finished loading yet, but the old type claimed otherwise, hiding this from every caller's type-checker.ExportImages.tsx(would crash "Export Images" if clicked before every card's document loaded) — fixed, with a regression test that reproduces the crash directly (confirmed it fails without the guard).CardResultSet.tsx's local annotation corrected (CardDocument | null→| undefined, matching its own already-correct!= nullguard — no behavior change).PDF.tsx,SCMPDF.tsx,PDFGenerator.tsx,clientSearchService.ts: widenedcardDocumentsByIdentifier's type through the render pipeline to match reality — all of these already filtered out undefined entries safely at runtime, so this is a type-annotation correction only, no new logic.Checklist
npx tsc --noEmitclean.npx jest --runInBand: 30 suites / 334 tests passing.npx eslinton all touched files: 0 errors, 3 pre-existing unrelated warnings.docs/proposals/proposal-b-bleed-normalization.md,docs/lessons.md.Generated by Claude Code