feat(magazines): render magazine labels as Magpul dot-matrix glyphs by unclesp1d3r · Pull Request #90 · unclesp1d3r/mag_stacker

unclesp1d3r · 2026-08-02T21:32:44Z

Renders a magazine's label in the detail view as Magpul PMAG Gen M3 paint-pen dot-matrix glyphs, so an owner can copy the pattern onto a floorplate instead of translating characters out of a PDF by hand. Closes #20.

This ships dark — read this first

src/data/magpul-glyphs.txt deliberately contains zero glyph rows. Magpul's diagram has not been transcribed yet, and an empty glyph table suppresses the matrix entirely, so merging this changes nothing an owner sees. That is the point: shipping a guessed dot pattern would tell someone to put permanent paint on hardware in the wrong places.

Turning the feature on is a data-only follow-up — transcribe the fixture, extend the model list — provided the diagram confirms a 3-column cell. Issue #20 describes "approximately 4 columns"; if that turns out right, the row format, the GlyphCell type, and the geometry all need rework. Confirm the column count before transcribing the whole font.

How it works

Seven units, each its own commit:

Glyph table (glyphs.ts) — one glyph per line, five 3-wide rows of #/., parsed once at module load. Malformed rows throw rather than yielding a partial font, so a bad transcription fails CI instead of rendering a wrong pattern.
Floorplate lookup (floorplate.ts) — free-text brandModel normalizes to one dense uppercase token, matched against a built-in list by substring containment. Unrecognized models fall back to 4 cells and say so.
Resolver (dot-matrix.ts) — one pure function returning hidden | matrix | unrepresentable. The glyph table is a parameter, which is what makes every rule testable today against a synthetic font while the shipped one is empty.
Owner-scoped read path (service.ts) — the detail page was passing the viewer's Magpul mode; the requirement keys on the magazine owner's. The new lookup runs strictly after the authorization gate so it cannot become an oracle for a magazine you cannot see.
Component (dot-matrix-label.tsx) — inline SVG, one <circle> per dot, role="img" with a single accessible name.

Product decisions came from a brainstorm that went through three multi-persona review rounds; the plan (docs/plans/2026-08-02-001-feat-magazine-dot-matrix-label-plan.md) is the source of truth for all fifteen requirements. Two decisions in it are marked user-directed: a floorplate's cell count means one face, and the model list is built in, not owner-editable.

Two bugs review caught

Cross-brand false match. A bare 762X51 caliber token matched any magazine whose model string carried that caliber — PTR-91 7.62X51 resolved as a confirmed 4-cell PMAG floorplate with no caveat, for a magazine with no dot matrix at all. Worse than a miss, since only a miss carries the warning. Caliber tokens are now banned from the list.
SVG sized from capacity, not content. The canvas used the floorplate's cell count while drawing only as many groups as the label has characters, so any short label painted dots on the left and dead space on the right.

Also fixed a pre-existing e2e flake: the date-range picker renders two month grids with outside-days, so a page-wide day lookup was a strict-mode violation on dates near a month boundary — it failed depending on when the suite ran.

Accessibility

Painted and unpainted dots are distinguished by radius as well as colour. They measure only 2.67:1 against each other in the dark theme, so a colour threshold could not carry WCAG 1.4.11's "visually distinct" clause. A test asserts both dot tokens clear 3:1 against --card in both themes and fails if either is declared in only one — Tailwind v4 silently no-ops an unknown utility, so nothing else would catch it.

Test plan

just ci-check green (lockfile, biome, format, typecheck, pre-commit, 833 unit/integration, 41 e2e)
Acceptance examples AE1–AE9 each have a named test in dot-matrix.test.ts
Owner-vs-grantee flag resolution tested in both directions
Contrast floor asserted in both themes
e2e asserts the shipped-dark state through the real render path, with no skipped or fixme'd tests
Rendered-matrix assertions land with the glyph transcription — no matrix can render before it

Findings that were reviewed but not applied are recorded in docs/residual-review-findings/.

The range picker renders two month grids and react-day-picker draws outside-days, so a day near a month boundary carries the same accessible name in both. A page-wide lookup was a strict-mode violation on exactly those dates, which made the spec fail depending on when the suite ran. Scope the day button to the grid for its own month. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>

Requirements from ce-brainstorm (three review rounds), enriched to implementation-ready. Resolves the how for GitHub issue #20. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>

One glyph per line, five 3-wide rows of painted/unpainted marks, parsed once at module load. Malformed rows throw rather than yielding a partial font, so a bad transcription fails in CI instead of rendering a wrong pattern someone paints onto a floorplate. The fixture ships with zero glyph rows until Magpul's diagram is transcribed. An empty table is a valid state, and it is what keeps the feature dark until the data lands. Covers R1, R2 (U1). Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>

Free-text brandModel normalizes to one dense uppercase token, then matches an ordered built-in list by required-substring containment. Entries are keyed on the distinctive family marker alone, never a brand token: MAGPUL does not contain the substring PMAG, so requiring it would reject the shorthand 'Magpul GL9' and silently drop a 2-cell magazine to the 4-cell fallback. Covers R3, R4, R5 (U2). Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>

One pure function returning a hidden | matrix | unrepresentable union, so the presentation layer branches once instead of threading nullable fields. The glyph table is a parameter, which is what makes every rule testable today against a synthetic font while the shipped one is empty. Font coverage is checked against the whole stored label before any truncation, so a label carrying an unsupported character is unrepresentable even when its trailing digits would have fit. Covers R6-R10 (U3). Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>

One circle per dot, role=img with a single accessible name, so the ~60 dots need no per-dot aria-hidden. Painted and unpainted dots differ in radius as well as fill: they measure only 2.67:1 against each other in the dark theme, so radius is what actually carries R15's 'visually distinct' clause and survives a future token retune. --dot-painted/--dot-unpainted alias --foreground/--muted-foreground. --border reads as the repo's 'faint but present' token but fails WCAG 1.4.11 against --card in both themes (1.26:1 dark, 1.40:1 light); the new contrast test asserts the 3:1 floor so a retune cannot silently break it -- Tailwind v4 no-ops an unknown utility without erroring. Geometry is fixed at 16px pitch / 10px painted / 6px unpainted, giving 216x74px at 4 cells, inside the card's ~248px content width at 320px. Covers R11, R12, R13, R15 (U5). Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>

The detail page passed the viewer's magpulMode; R6 keys on the magazine owner's. getMagazine now resolves the owner's flag and returns it, with the query strictly after the authorization gate so it cannot become an oracle for a magazine the actor cannot see. A missing owner row throws rather than defaulting to false, mirroring the write path. The prop is renamed magpulMode -> ownerMagpulMode so the change of meaning is visible at every call site rather than silently repointed. That prop also feeds the edit form's label mask, but editing is owner-only, so viewer and owner are already the same account whenever the form renders -- this removes a latent divergence, it does not fix a live defect. The detail view now places the matrix below the field list. Splicing it inside the <dl> would break the definition-list content model and shift the last-row border, changing the view even with Magpul mode off. Covers R6, R4, R9, R10, R14 (U4, U6). Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>

The glyph table ships empty, so the matrix is suppressed and the spec asserts that through the real render path: the label renders as text and no img-role graphic appears. Overflow is measured via scrollWidth at 320px rather than asserted as 'no layout break', which passes trivially on an empty page. Both tests delete their magazine through the UI before finishing. The personas are shared with magpul-mode and theme specs, and alphabetical file ordering runs this spec first, so leaving rows behind would break magpul-mode's cold-start assertion on every full-suite run. Assertions on a rendered matrix land with the glyph transcription; no test is skipped or fixme'd to hold their place. Covers R6, R13, R14 (U7). Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>

The owner-magpulMode query was written out verbatim three times in this file. Extract resolveOwnerMagpulMode and call it from createMagazine, updateMagazine, and getMagazine. In getMagazine it now runs concurrently with attachCompatibility -- both are independent once the authorization and existence gates have passed, and both still run strictly after them. Pull the console-error tracker and the overflow measurement into e2e fixtures; theme.spec.ts adopts the tracker so each has two real consumers. responsive-overflow.spec.ts keeps its inline check, which also probes maxScrollX and would not be a drop-in. The contrast test now strips comments and matches braces by depth rather than taking the first closing brace, so a nested at-rule or a comment mentioning a theme selector cannot silently slice the wrong CSS. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>

Matching is substring containment over free-text brandModel, so the bare '762X51' entry matched any magazine whose model string carried that caliber -- 'PTR-91 7.62X51' resolved as a *confirmed* 4-cell PMAG floorplate, with no unrecognized-model caveat, for a magazine that has no dot matrix at all. That is the failure the module warns about: a wrong count on a matched entry is worse than a miss, because only the miss carries the caveat. Drop the entry; such magazines now fall back to 4 cells and say so. Tokens must be Magpul model designations, never a caliber, which is brand-agnostic by definition. The SVG was also sized from the floorplate's capacity while drawing only as many cell groups as the label has characters, so any label shorter than the floorplate painted dots on the left and left dead space on the right. Size from what is drawn; the accessible name still names capacity. The e2e cleanup now runs in a finally block. The personas are shared and the specs run serially in alphabetical order, so a failed assertion left a magazine behind and broke magpul-mode's cold-start step with an error pointing at the wrong spec. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>

coderabbitai · 2026-08-02T21:33:02Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

Added owner-scoped Magpul mode handling and integrated accessible DotMatrixLabel SVG rendering into the magazine detail view.
Added glyph parsing, floorplate lookup, label resolution, themed dot tokens, and fixes for cross-brand caliber matching and calendar lookup scoping.
Added bun:test coverage for parsing, resolution, floorplates, service behavior, geometry, and contrast. Added Playwright coverage for rendering, mode behavior, overflow, console errors, and calendar behavior.
No schema changes or migration steps are required. The glyph table is empty, so dot-matrix rendering remains disabled until the official glyph diagram is transcribed.

Walkthrough

Changes

Magazine detail pages now use the magazine owner’s Magpul mode. The PR adds dot-matrix resolution, accessible SVG rendering, theme tokens, empty-glyph handling, fallback messages, and Playwright coverage. The glyph table remains intentionally empty, so no matrix image renders until glyph data is added.

Changes

Magazine dot-matrix label

Layer / File(s)	Summary
Glyph and label resolution `src/domain/magazines/`, `src/data/`, `docs/plans/...`	Adds strict glyph parsing, floorplate cell-count lookup, label normalization, capacity handling, trailing-digit fallback, typed resolver outcomes, and domain tests.
Owner mode and detail integration `src/domain/magazines/service.ts`, `app/(app)/magazines/[id]/page.tsx`, `app/(app)/magazines/magazine-detail-view.tsx`	Resolves `ownerMagpulMode` after authorization and passes it to the detail view and edit form.
Dot-matrix SVG rendering and theme tokens `app/(app)/magazines/dot-matrix-label.tsx`, `app/globals.css`, `src/domain/magazines/__tests__/dot-matrix-contrast.test.ts`	Renders accessible dot grids, hidden results, fit errors, and unverified-cell messaging with dark and light theme tokens.
Browser validation and shared E2E fixtures `e2e/fixtures/`, `e2e/magazine-dot-matrix.spec.ts`, `e2e/theme.spec.ts`, `e2e/magazine-inventory-filter.spec.ts`, `docs/residual-review-findings/`	Adds console-error, overflow, mode-on, mode-off, empty-glyph, mobile, and cleanup coverage. It also scopes calendar lookups to the target month.

Sequence Diagram(s)

sequenceDiagram
  participant MagazineDetailPage
  participant getMagazine
  participant MagazineDetailView
  participant DotMatrixLabel
  participant resolveDotMatrix
  MagazineDetailPage->>getMagazine: request magazine and ownerMagpulMode
  getMagazine->>MagazineDetailPage: return magazine, permission, ownerMagpulMode
  MagazineDetailPage->>MagazineDetailView: pass owner mode and label data
  MagazineDetailView->>DotMatrixLabel: render label
  DotMatrixLabel->>resolveDotMatrix: resolve label and capacity
  resolveDotMatrix->>DotMatrixLabel: return hidden, matrix, or unrepresentable result

Possibly related PRs

unclesp1d3r/mag_stacker#25 — Provides related Playwright infrastructure used by the new E2E fixtures.
unclesp1d3r/mag_stacker#26 — Establishes the owner-scoped magpulMode behavior consumed by this change.
unclesp1d3r/mag_stacker#38 — Provides the magazine detail route and service wiring extended by this change.

Suggested labels: enhancement, backend, frontend, documentation, testing, priority:medium

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (3 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The implementation and tests are present, but the empty glyph table means issue `#20`'s core requirement to render supported Magpul glyphs is not met.	Transcribe and validate the authoritative glyphs for 0-9, A-Z, and '-', then add rendered-matrix assertions before closing the issue.
Out of Scope Changes check	⚠️ Warning	The date-picker lookup change in e2e/magazine-inventory-filter.spec.ts is unrelated to the linked dot-matrix feature.	Remove the unrelated date-picker change or link it to a separate issue and submit it independently.
Docstring Coverage	⚠️ Warning	Docstring coverage is 73.08% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title follows Conventional Commits and accurately describes the magazine dot-matrix label feature.
Description check	✅ Passed	The description clearly covers the change, linked issue, implementation, risks, and test plan, but omits the template checklist and AI assistance disclosure.

✨ Finishing Touches

✨ Simplify code

Create PR with simplified code
Commit simplified code in branch 20-render-magazine-label-as-a-magpul-paint-pen-dot-matrix-in-the-detail-view

Warning

Review ran into problems

🔥 Problems

These MCP integrations need to be re-authenticated in the Integrations settings: Notion

_{Comment @coderabbitai help to get the list of available commands.}

Copilot

Pull request overview

Adds the full “Magpul dot-matrix label” resolution + rendering pipeline for magazine detail pages, while intentionally shipping dark (empty glyph table suppresses rendering) until the authoritative glyph data is transcribed. It also corrects the detail read path to use the magazine owner’s magpulMode and includes test/CI guards (domain-unit coverage, contrast checks, and E2E coverage), plus an unrelated E2E strict-mode fix for the date-range picker.

Changes:

Introduces pure domain modules for glyph parsing, floorplate cell-count resolution, and label→matrix resolution (discriminated union), with comprehensive unit tests.
Updates getMagazine to return ownerMagpulMode and wires the detail page/component to use owner-scoped behavior.
Adds dot-matrix UI component + CSS tokens + contrast guard tests; adds ships-dark E2E coverage and refactors shared E2E helpers.

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/domain/magazines/service.ts	Adds owner-scoped `magpulMode` resolver and returns `ownerMagpulMode` from `getMagazine`.
src/domain/magazines/glyphs.ts	Implements glyph-table parsing and exports `MAGPUL_GLYPHS` parsed at module load.
src/domain/magazines/floorplate.ts	Adds brandModel→cell-count resolver with an explicit model token list and fallback semantics.
src/domain/magazines/dot-matrix.ts	Adds pure resolver returning `hidden
src/domain/magazines/tests/service.test.ts	Adds integration coverage for owner-vs-viewer `magpulMode` behavior in `getMagazine`.
src/domain/magazines/tests/glyphs.test.ts	Adds parser tests for glyph fixture format and shipped “ships dark” assertions.
src/domain/magazines/tests/floorplate.test.ts	Adds unit tests for model normalization and cell-count resolution, including collision guards.
src/domain/magazines/tests/dot-matrix.test.ts	Adds unit tests for acceptance examples and boundary cases using synthetic glyph tables.
src/domain/magazines/tests/dot-matrix-contrast.test.ts	Adds CSS-token parsing + contrast assertions for dot tokens in both themes.
src/data/raw.ts	Embeds `MAGPUL_GLYPHS_RAW` (generated reference data module).
src/data/magpul-glyphs.txt	Adds the (currently empty) checked-in glyph fixture source file with format documentation.
app/globals.css	Adds dot token aliases and bridges them into Tailwind color variables.
app/(app)/magazines/dot-matrix-label.tsx	Adds client component rendering SVG dot matrix + caveats/messages.
app/(app)/magazines/magazine-detail-view.tsx	Wires the dot-matrix component into the magazine detail view and renames prop to `ownerMagpulMode`.
app/(app)/magazines/[id]/page.tsx	Passes `ownerMagpulMode` from `getMagazine` into the detail view.
e2e/magazine-dot-matrix.spec.ts	Adds ships-dark E2E coverage for dot-matrix wiring, overflow, and console errors.
e2e/theme.spec.ts	Refactors console error tracking to shared helper.
e2e/magazine-inventory-filter.spec.ts	Fixes date picker strict-mode flake by scoping day selection to the correct month grid.
e2e/fixtures/console-errors.ts	Adds shared console/pageerror tracking helper.
e2e/fixtures/overflow.ts	Adds shared overflow assertion helper.
docs/plans/2026-08-02-001-feat-magazine-dot-matrix-label-plan.md	Adds the implementation plan/spec for this feature and its requirements/units.
docs/residual-review-findings/20-render-magazine-label-as-a-magpul-paint-pen-dot-matrix-in-the-detail-view.md	Records review findings intentionally not applied in this PR.

coderabbitai

Actionable comments posted: 4

🧹 Nitpick comments (1)

app/(app)/magazines/dot-matrix-label.tsx (1)
74-156: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Add component-level tests for DotMatrixLabel.

This component has no direct test coverage in the reviewed files: SVG sizing math, the aria-label text, and the two distinct Callout fallback branches (unrepresentable, !cellCountVerified) are all untested outside the domain resolver. The end-to-end test can only observe whether the label renders, not whether the accessible name, geometry, or fallback copy are correct.

Add unit or component tests that assert: the aria-label text for a representable label, the SVG width/viewBox for a known cells.length, and the correct Callout text for both fallback branches.

As per coding guidelines, **/*.test.{ts,tsx,js,jsx} requires that "new behavior needs coverage." Based on learnings from the external tools context, the PR's own residual findings note that "SVG markup, accessibility copy, overflow messaging, and fallback branches have no component-level tests."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/`(app)/magazines/dot-matrix-label.tsx around lines 74 - 156, Add
component-level tests for DotMatrixLabel covering a representable label’s
aria-label, SVG width and viewBox derived from the known cells.length, the
unrepresentable branch’s Callout message, and the !cellCountVerified branch’s
caveat text. Use stable fixture inputs and mock or reuse the resolver data as
needed so each assertion verifies the component output rather than only resolver
behavior.
Sources: Coding guidelines, MCP tools

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/plans/2026-08-02-001-feat-magazine-dot-matrix-label-plan.md`:
- Around line 390-394: Update the DotMatrixResult rendering requirement to
specify that the SVG viewBox dimensions are derived from the rendered cells
array length, not the floorplate’s full cell count. Keep the sizing behavior
that prevents unused cells from appearing for short labels, and revise only the
affected wording in the component-rendering step.

In `@src/data/raw.ts`:
- Around line 119-143: Unify the glyph fixture so MAGPUL_GLYPHS_RAW and
src/data/magpul-glyphs.txt cannot diverge: choose one canonical source and make
the production export and tests consume it consistently. Update the
MAGPUL_GLYPHS loading path and the existing .txt parsing test accordingly, then
add a parity test that detects any mismatch between the source and
exported/loaded glyph data.

In `@src/domain/magazines/dot-matrix.ts`:
- Around line 22-39: Extend the DotMatrixResult unrepresentable variant with a
reason distinguishing unsupported characters from capacity overflow, and update
resolveDotMatrix to return the appropriate reason in both failure paths. Update
dot-matrix-label.tsx’s buildDoesNotFitMessage presentation logic to render an
accurate message for each reason while preserving the existing capacity message
for doesNotFit.

In `@src/domain/magazines/glyphs.ts`:
- Around line 83-98: Validate each glyph key in the parser before duplicate and
row processing, using the shared Magpul label character-set symbol rather than
only checking character length. Reject unsupported keys such as "." or lowercase
characters with the parser’s malformed-input error path, and add tests covering
unsupported glyph keys while preserving valid glyph parsing.

---

Nitpick comments:
In `@app/`(app)/magazines/dot-matrix-label.tsx:
- Around line 74-156: Add component-level tests for DotMatrixLabel covering a
representable label’s aria-label, SVG width and viewBox derived from the known
cells.length, the unrepresentable branch’s Callout message, and the
!cellCountVerified branch’s caveat text. Use stable fixture inputs and mock or
reuse the resolver data as needed so each assertion verifies the component
output rather than only resolver behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: f0760a98-369f-481d-97ae-bf23ccb43a1e

📥 Commits

Reviewing files that changed from the base of the PR and between 6323744 and 8fa55ef.

📒 Files selected for processing (22)

app/(app)/magazines/[id]/page.tsx
app/(app)/magazines/dot-matrix-label.tsx
app/(app)/magazines/magazine-detail-view.tsx
app/globals.css
docs/plans/2026-08-02-001-feat-magazine-dot-matrix-label-plan.md
docs/residual-review-findings/20-render-magazine-label-as-a-magpul-paint-pen-dot-matrix-in-the-detail-view.md
e2e/fixtures/console-errors.ts
e2e/fixtures/overflow.ts
e2e/magazine-dot-matrix.spec.ts
e2e/magazine-inventory-filter.spec.ts
e2e/theme.spec.ts
src/data/magpul-glyphs.txt
src/data/raw.ts
src/domain/magazines/__tests__/dot-matrix-contrast.test.ts
src/domain/magazines/__tests__/dot-matrix.test.ts
src/domain/magazines/__tests__/floorplate.test.ts
src/domain/magazines/__tests__/glyphs.test.ts
src/domain/magazines/__tests__/service.test.ts
src/domain/magazines/dot-matrix.ts
src/domain/magazines/floorplate.ts
src/domain/magazines/glyphs.ts
src/domain/magazines/service.ts

Review caught that the 'shipped fixture parses' test read MAGPUL_GLYPHS_RAW, not the .txt it named -- so editing the file and forgetting to regenerate raw.ts would pass CI while production kept the stale font. Transcribing that file is the follow-up this whole feature waits on, which makes silent drift the one failure it cannot afford. The test now reads the file from disk and asserts it matches the embedded copy. src/data/calibers.txt and manufacturers.txt have the same untested drift; left alone as out of scope. Also from review: the contrast test's hex extraction matched 3-8 digits while the parser only handles 6, so an unsupported token would have thrown opaquely instead of naming itself; the e2e console tracker attached after the add-magazine flow had already run, so it could not have seen errors from it; and the plan still documented the caliber-only token entry the code now bans, which would have walked the next reader back into the cross-brand false match. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/domain/magazines/__tests__/dot-matrix-contrast.test.ts`:
- Around line 77-78: Update extractHexToken so its regular expression requires a
non-hex boundary after the six-digit value, preventing an 8-digit color such as
`#11223344` from matching `#112233`. Add a regression test in the existing
dot-matrix contrast test suite confirming 8-digit hex values are rejected.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: d906e5fc-361d-4b4a-9c6c-73733ceac956

📥 Commits

Reviewing files that changed from the base of the PR and between 8fa55ef and 1a09725.

📒 Files selected for processing (4)

docs/plans/2026-08-02-001-feat-magazine-dot-matrix-label-plan.md
e2e/magazine-dot-matrix.spec.ts
src/domain/magazines/__tests__/dot-matrix-contrast.test.ts
src/domain/magazines/__tests__/glyphs.test.ts

🚧 Files skipped from review as they are similar to previous changes (3)

src/domain/magazines/tests/glyphs.test.ts
e2e/magazine-dot-matrix.spec.ts
docs/plans/2026-08-02-001-feat-magazine-dot-matrix-label-plan.md

The parser only checked that a glyph key was one character, so a transcription slip could introduce a '.' or a lowercase glyph. R9 treats a label carrying a character absent from the font as unrepresentable, so a stray accepted glyph would make a grandfathered nonconforming label render instead -- defeating R9 on exactly the labels it protects. Keys now validate against MAGPUL_LABEL_ALLOWED_RE rather than a second copy of the character set. The hyphen stays valid, with a test saying so. Also corrects U5 in the plan, which still described the SVG as sized from the floorplate's capacity after the code moved to sizing from the cells actually drawn. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>

Tightening the extraction to {6} in the previous commit left a subtler hole: the pattern matched the leading six digits of an #RRGGBBAA value, so the contrast helper would validate a colour the stylesheet never declares and still pass. Added a hex-digit boundary plus regression tests for the 8-digit and 3-digit cases. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>

Read off Magpul's published dot-matrix PDF by rendering it at 300 DPI, recovering the 30x20 dot lattice, and sampling every position -- then verified against the sheet by the owner. All 36 glyphs, no eyeballing. Two things the sheet settled. Its digit row runs 1-9 then 0, so reading it left-to-right as 0-9 would have shifted every digit by one. And it carries no hyphen and no punctuation at all, which R1 assumed it did: issue #21 still permits '-' in a label, so a stored label containing one is now unrepresentable under R9. Left as an open product question rather than decided here. R2's 3x5 cell is confirmed rather than assumed, so the geometry and row format stand as built. The feature is no longer dark. The unit test pinning an empty table and the e2e asserting the matrix is absent both encoded the pre-font world; they now assert AE1, AE2/AE8 and AE6 against a real render. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>

…ph font (#92) Closes the two open items left after #90 — and supersedes #91, which carried the same findings doc on a branch that went stale when #90 squash-merged. ## The verification gap (the higher-risk half) `glyphs.test.ts` spot-checked the exact dot pattern for `0` and `1`. The other **34 glyphs were checked for presence, not shape** — a row shift or a bad PDF crop in any letter passed every test in the repo. Nothing anywhere looked at rendered SVG dot positions either; AE1's e2e read the accessible name (`U S 0 4`), not the pattern. - All 36 glyphs are now asserted against `EXPECTED_GLYPHS`, recorded in **Magpul's own sheet ordering** (digits `1`-`9` then `0`) rather than the fixture's, so a bad regeneration can't pass by matching the artifact it was derived from. A companion test asserts the expected table still covers all 36, so deleting a row can't quietly shrink the guard. - Confirmed to bite: corrupting a single dot in glyph `G` turns two tests red. - **`resolveDotMatrix` had never run against the shipped font.** Every resolver test used a synthetic fixture that carries a hyphen Magpul never drew. A second describe block re-runs the acceptance examples against the real `MAGPUL_GLYPHS` — which immediately showed AE4 (`AR-X`) taking a *different rule* under each table. - AE1's e2e now reads `r` off all 60 circles and compares the drawn pattern to a string restated from the sheet. That's the only assertion spanning **fixture file → parser → resolver → geometry**. - R4/R9 copy was exported and rendered but read by nothing. Now asserted at the builder level (`dot-matrix-messages.test.ts`) and as rendered text in the e2e. ## The hyphen question — resolved by dropping The floorplate has no hyphen cell, so a hyphen can never be painted. But #21 permits one in a stored label, so `A-1` was ordinary user input that rendered nothing and reported *"This label does not fit this magazine's floorplate"* — which is false; it never reached the length check. **R8 already drops characters**: `US04` on a 2-cell GL9 paints `04` and discards `US`. Refusing to paint `A-1` over one hyphen while happily painting `04` for `US04` was the inconsistent position. So an unpaintable character is now dropped and the rest is drawn, with a caveat naming what was left out. The collision this admits (`A-1` and `A1` paint alike) already existed under R8, where `US04` and `XY04` both paint `04`. **Rejected:** narrowing #21's allowed character set, which would invalidate stored labels and need a migration. `unrepresentable` still gains a `reason` discriminant — it stays reachable for a label with *nothing* paintable in it (`--`), and that case now gets its own message instead of the false "does not fit". That also closes the parked CodeRabbit finding from #90. ## Test plan - [x] `just ci-check` green — **45 e2e passed** (up from 42) - [x] Glyph guard proven to fail on a one-dot corruption, then reverted - [x] New e2e: hyphen drop + caveat, R4 unrecognized-model caveat wording, R9 does-not-fit wording - [x] Plan amended: R1, R2, R9, new R9a, AE4/AE5/AE5b, KTD8, Verification Contract - [x] Stale "ships dark" comments corrected in four source files ## Still open (recorded in `docs/residual-review-findings/`) - **Per-model cell counts** — `MODEL_CELL_COUNTS` carries only the GL9 and LR/SR seeds; everything else renders under R4's unverified 4-cell fallback. Now the largest unverified area in the feature. - `calibers.txt` / `manufacturers.txt` have the same `.txt`↔`raw.ts` drift the glyph fixture had before #90. --------- Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>

unclesp1d3r added 11 commits August 2, 2026 15:45

docs(plan): add magazine dot-matrix label rendering plan

dd718e4

Requirements from ce-brainstorm (three review rounds), enriched to implementation-ready. Resolves the how for GitHub issue #20. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>

docs(review): record residual review findings

8fa55ef

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>

Copilot AI review requested due to automatic review settings August 2, 2026 21:32

unclesp1d3r linked an issue Aug 2, 2026 that may be closed by this pull request

Render magazine label as a Magpul paint-pen dot matrix in the detail view #20

Closed

4 tasks

Copilot started reviewing on behalf of unclesp1d3r August 2, 2026 21:33 View session

coderabbitai Bot added backend documentation Improvements or additions to documentation enhancement New feature or request frontend priority:medium testing labels Aug 2, 2026

Copilot AI reviewed Aug 2, 2026

View reviewed changes

coderabbitai Bot reviewed Aug 2, 2026

View reviewed changes

Comment thread docs/plans/2026-08-02-001-feat-magazine-dot-matrix-label-plan.md

Comment thread src/data/raw.ts

Comment thread src/domain/magazines/dot-matrix.ts

Comment thread src/domain/magazines/glyphs.ts

coderabbitai Bot added the shared label Aug 2, 2026

coderabbitai Bot reviewed Aug 2, 2026

View reviewed changes

Comment thread src/domain/magazines/__tests__/dot-matrix-contrast.test.ts Outdated

unclesp1d3r added 3 commits August 2, 2026 18:02

docs(review): record deferred findings from PR #90 review

a483cd9

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>

coderabbitai Bot removed the shared label Aug 2, 2026

unclesp1d3r self-assigned this Aug 2, 2026

unclesp1d3r enabled auto-merge (squash) August 3, 2026 01:12

unclesp1d3r merged commit 1202a7b into main Aug 3, 2026
7 checks passed

unclesp1d3r deleted the 20-render-magazine-label-as-a-magpul-paint-pen-dot-matrix-in-the-detail-view branch August 3, 2026 01:14

This was referenced Aug 3, 2026

docs(review): record findings from the post-transcription review #91

Closed

feat(magazines): drop unpaintable characters and verify the whole glyph font #92

Merged

Navigation Menu

Uh oh!

feat(magazines): render magazine labels as Magpul dot-matrix glyphs - #90

feat(magazines): render magazine labels as Magpul dot-matrix glyphs#90
unclesp1d3r merged 16 commits into
mainfrom
20-render-magazine-label-as-a-magpul-paint-pen-dot-matrix-in-the-detail-view

unclesp1d3r commented Aug 2, 2026

Uh oh!

coderabbitai Bot commented Aug 2, 2026 •

edited

Loading

Reviews paused

❌ Failed checks (3 warnings)

Review ran into problems

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

unclesp1d3r commented Aug 2, 2026

This ships dark — read this first

How it works

Two bugs review caught

Accessibility

Test plan

Uh oh!

coderabbitai Bot commented Aug 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Changes

Sequence Diagram(s)

❌ Failed checks (3 warnings)

Review ran into problems

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Aug 2, 2026 •

edited

Loading