Repository navigation
Conversation
A class filled from an SRD class was renaming the grouped-equipment picks (arcane/ druidic focus, holy symbol, instrument, pack): the live equipment-option builds those nested selections as a plain 'Arcane Focus' with no opt-out, but the recompiler routed them through new-starting-equipment-selection, which prepends 'Starting Equipment: ' and appends a '<none>' option. So 'Arcane Focus' came back as 'Starting Equipment: Arcane Focus' with a spurious pick-nothing escape. The nested name also feeds the selection's minted key, so the rename would destabilise any later key-addressed override. Branch the recompiler: grouped picks mirror equipment-option exactly (group name, no prefix, no <none>); weapon-class picks keep the prefixed builder, which is what live uses for them. Tighten the round-trip test to compare EVERY starting-equipment selection name (nested included, not just top-level) and the quantities on choice grants (a set of item keys ignored Fighter's 'longbow + 20 arrows' count) — both previously masked the rename and a possible qty regression.
print-character-sheet-style? arrives from an unauthenticated POST and was used verbatim to build a resource name: (str "fillable-char-sheetstyle-" print-character-sheet-style? "-6-spells.pdf") Omitting the field produced "fillable-char-sheetstyle--0-spells.pdf", io/resource returned nil, and .openStream on nil threw. A 500 anyone could trigger by leaving a field out. A string value did the same. Only the four ids with a template on disk are accepted -- 28 files, four styles by seven spell counts -- and anything else falls back to the first. Confirmed against the running server: a valid style, a traversal attempt, an empty body and an out-of-range number now all return 200 rather than two of them returning 500. Path traversal was never very reachable here, since the fixed "-N-spells.pdf" suffix means an attacker has to land on a classpath resource ending in exactly that. The crash was the real defect. Tests assert the id set matches the templates actually present, so adding a style without its files, or removing files without the set, fails rather than 500-ing at runtime. Also brings dev/e2e_boot.clj onto this branch, byte-identical to the copy on fix/custom-item-classification so the two merge cleanly. Without it there is no way to stand this server up and check any of the above.
The decompiler recognised a nested sub-choice only when its options matched a known
weapon/equipment pool exactly, and dropped anything else — so a future SRD change or a
homebrew with a 'pick one of these specific items' choice would lose that equipment with
no error. Replace the silent drop with a real fallback: an unrecognised sub-choice is
ENUMERATED option-by-option (a new :choose {:name :options} form the compiler builds as a
plain nested selection), so it round-trips instead of vanishing. Only a selection with no
representable options at all is genuinely undecompilable, and that now throws with context.
Neither path fires for the shipped 12 SRD classes (all sub-choices are pools) — this is a
precaution kept against future/homebrew input. Covered both ways: an enumerated non-pool
sub-choice round-trips compile->decompile->recompile, and a degenerate empty sub-choice
throws. Also exposes decompile on a built option so the round-trip is testable directly.
Represent a homebrew class's starting equipment as an SRD base plus a minimal diff instead of a full copy. derive-ledger diffs an edited form against the base; resolve-ledger applies base + ledger back to the full form. Addressing is by STABLE KEY minted deterministically from names (name-to-kw, de-duped within parent) — keys are COMPUTED at diff/resolve time, not persisted in the builder's working data, so nothing about the existing form changes. v1 granularity: fixed grants by item-key; groups added/removed whole; group name; options added/removed/replaced whole (captures a quantity-only change; grant-level minimality is a noted future refinement). Resolution is fail-soft — an op whose target is absent from the base is skipped and surfaced in :warnings, never applied blindly. Order is not meaningful. Engine only — not yet wired into export/import or the builder UI (those commit a persisted format and are the next checkpoint). Covered by a round-trip property test over every edit kind (add/remove/replace of fixed grants, groups, options, sub-choices, quantity changes), fail-soft resolution, diff minimality, and a real SRD base (fighter) round-trip.
Measured while hardening /character.pdf. No code change. The four sheet styles have unrelated size problems: style 1 is 1,407 AcroForm fields and no images, style 2 is already lean at 241KB, and styles 3 and 4 are full-page rasters at print resolution whose size is earned. Style 1's 28 templates are all the same form with different numbers of spell pages bound in; 92 checkbox widgets attach per page present, leaving the rest pointing at nothing. Pruning fields whose widgets have no page saves 74.7% on the no-spellcaster sheet but only 6.4% across all 28, and nothing at all on the other styles. Also records a conclusion that was wrong and how the test missed it: pruning by name removed the death save checkboxes, and comparing renders of an EMPTY form could not see it, because an unticked box draws nothing. The test has to tick every box and render every page.
Counting all widgets rather than just checkboxes: 1,810 of 1,931 are orphaned on the non-caster sheet, and exactly 214 attach per spell page present. That linear pattern is the signature of one 9-page master saved repeatedly with pages deleted -- deleting a page leaves its fields in the AcroForm. Distinguishes those from widgets that merely lack the /P back-pointer, which is what pdf/fix-widget-page-refs! repairs. The two sets happen to coincide here, but the function cannot see true orphans since it walks annotations already on pages. Worth stating so nobody assumes it covers this. Records the better fix: prune at export in write-fields! rather than regenerating 28 assets. Also corrects the colour assumption. Style 3 is NOT black and white -- it has blue gradients and gold accents, and 9.78% of sampled pixels carry real colour. It is stored in DeviceCMYK, four channels, for something viewed on screen and printed at home; RGB PNG is 51% smaller losslessly.
Chroma measured per pixel on a rendered page 1. Styles 1, 2 and 4 come back at max chroma 3-4 out of 255, which is render noise -- they are genuinely black and white. Only style 3, the official sheet with the teal/gold ram crest, carries real colour, and only in about 8% of its pixels. This corrects an earlier note here that lumped style 4 in with style 3 as colour art that grayscale would destroy. Style 4 is in fact the largest template at 2.55 MB and stores visually grayscale pages as DeviceRGB JPEG -- three channels holding identical values. DeviceGray is lossless by inspection and is a bigger absolute saving than the style 3 CMYK change, with no quality tradeoff to weigh.
Checked all four style families rather than just the first. Styles 2-4 have zero orphaned widgets and scale their field count with page count, which is what a correctly built variant looks like. Style 1 alone carries its full 1931 widgets in every variant regardless of page count, leaving 94% of them pointing at no page on the non-caster sheet. Style 1 is also the only style the app offers: integrations/sheet-styles returns a single entry, with the others gated behind fork overrides. So the earlier notes about style 3 CMYK and style 4 RGB-for-gray describe real savings on assets no open-app user can select. Kept for reference, but neither is the place to start. Scope drops from 28 files to 7, and the runtime prune only has to handle one family. Also records that the server accepts sheet styles 1-4 while the UI offers only 1 -- deliberate, not an oversight, and static assets either way.
…are tidy Compared field names rather than counts. Style 4 has no backstory field and no equipment field, both of which pdf_spec emits. write-fields! looks up by exact name and skips misses with a bare (when field ...) -- no warning. So a user on that style fills in backstory and equipment, exports, and gets a PDF without them and no indication why. Styles 2 and 3 have a narrower version of it around cantrips-6/7/8 and some spells-N-M names. Checked these are absences rather than renames: the extra names in style 4 are Notes, other, Conditions and Insanities, Pronouns, Carry Capacity, none of which the code writes to. A rename the code does not know about is indistinguishable from a missing field anyway, since lookup is by exact name. This reverses the earlier note calling those styles correctly built. Style 1 is bloated but complete; the others are lean because they are incomplete, so they are not a model for rebuilding it. Leaves the runtime orphan prune as the only change here that cannot lose data, and flags the silent skip as the reason this went unnoticed.
Replace the minted-key ledger with a delta addressed entirely on identifiers that already
exist in the data: fixed grants diff by ITEM KEY (changed/added in :fixed :set, removed in
:fixed :del); choice groups match by their existing :name, and a group the user touched is
stored WHOLE (:groups :set) since the nested OR-menus are rarely edited and not worth a
sub-diff. No name-to-kw, no minted :key anywhere — the resolved output is the plain
:equipment-selections the existing functions already consume.
This serves the actual goal: a user tweaking a class saves just the tweak, not a full copy
(adding one item is {:fixed {:set {:weapons {:dagger 1}}}}). Resolution is fail-soft by
construction. Engine + round-trip tests only; export/import wiring is the next checkpoint.
…nly in files)
The delta is a pure on-disk encoding: everything live (app-db, localStorage, the builder)
holds the full :equipment-selections form the existing functions already consume, so the
consumption path is untouched.
- collapse-class / expand-class (ledger ns): whole-class transforms between full form and
{:starting-equipment {:base <kw> …delta}}. No-op without a base marker / delta key;
unknown base leaves the class untouched (never drops equipment); a class still on the
legacy :*-choices shorthand is left in full form.
- Export: map-plugin-classes collapses classes in the single serialize funnel
(save-orcbrew-blob!) plus the two direct-blob bypasses (save-to-json, all-plugins-pretty).
- Import: expands classes at both ingestion points (store-imported-sources,
apply-shared-content) BEFORE the load-floor spec gate, so the library only ever holds full
form and the delta never has to satisfy the class spec.
- Fill: 'start from an SRD class' now records :starting-equipment-base so export can diff.
- Defensive: the plugin-classes consumption sub expands too (no-op unless a delta somehow
reached the library), so a missed ingestion path can't silently drop equipment.
Verified: JVM round-trip incl. whole-class collapse/expand (35 assertions), full suite green,
cljs compiles, and a browser round-trip on the real filled Fighter class — fill+tweak ->
collapse (delta only) -> expand reproduces the full class identically, 0 page errors.
routes.clj picks one of seven pre-baked variants per style by counting spellcasting classes, and the unused spell fields are what all the ghosts are. Spiked the alternative against the real assets: ship a base plus one spell-page master, clone it as needed, build the fields in code. Cloning a page by referencing the source contents and resources shares the artwork -- the master alone is 421 KB and the master plus six clones is 422 KB. Fields build per cloned page with arbitrary names and render correctly, verified by filling a level-1 slot on a cloned page. Roughly 12 KB per page. Records three traps, each of which would have cost real time. Splitter drops the AcroForm outright. PDFMergerUtility duplicates the background image per copy and renames colliding fields, which would silently break every name pdf_spec emits. And orphan pruning has to filter widget lists rather than whole fields, since some fields own widgets across pages -- doing it per field left 101 behind. Seven files per style become two, ghosts stop existing rather than being pruned, and the six-class ceiling disappears. Wiring it into routes.clj is not done.
Adds dev/sample_character.clj, which builds a level 20 evoker with every spell level filled from the real wizard spell list and writes it through the actual write-fields! path. Empty forms hide almost everything -- comparing empty renders is exactly how the death-save checkboxes got missed earlier in this work -- so template changes need a sheet with data on it. Filling one surfaced three bugs on style 1, the default and only public sheet: Page 1 fields sit one box off from the printed labels. The tall right column is the features-and-traits field, printed under a heading that reads EQUIPMENT; the equipment field sits under one that reads TREASURE. Everyone's features have been printing under the wrong label. spells-3-11-1 does not exist in style 1, though styles 2 and 4 have it. The level 3 column offers 13 rows and skips the 11th, so a wizard with a full complement of third-level spells silently loses one to a dead row. Save-modifier widgets are 14.4 by 8.6 points and clip two-digit bonuses. Every level 20 caster prints a truncated modifier in their casting stat.
Adds warlock 5 / sorcerer 5 / wizard 5 / cleric 5 alongside the level 20 wizard, and restructures the fixture to be spec-driven so both share the field-building code. One spellcasting class only exercises the -1 field suffix; four exercise -1 through -4 and the sheet4 template, which nothing else covered. It reproduces all three previously recorded bugs independently and corrects the scope of one: the undersized modifier boxes are not a saves problem. This character clips Deception, Persuasion and the Charisma save on a single page. Any modifier reaching +10 truncates, which is routine at high level. Confirms working: the four-class string auto-sizes into CLASS & LEVEL, hit dice wrap, long subclass names in the spell page header auto-size, and warlock pact slots stay separate from the shared ones. Notes one thing that looks wrong but is not -- slots at 4th through 8th with no spells under them is correct when caster level is 15 and no class exceeds 5th.
…elds Adds bard 2 / cleric 2 / druid 2 / paladin 4 / ranger 4 / sorcerer 2 / warlock 2 / wizard 2 -- twenty levels across eight spellcasting classes, where the sheet has room for six. Classes seven and eight have nowhere to print, and write-fields! skips names it cannot find without comment, so 50 values vanish: both classes' names, abilities, save DCs, attack bonuses, every slot and every spell. A player would print this character and find two of their eight classes missing, with nothing on the page or in the logs explaining it. build! now diffs the field map against the template before writing and reports what has no home. That is the check write-fields! should be doing at runtime, and it immediately caught the spells-3-11 gap on the other two fixtures without anyone looking for it. Also records that page 9 of every six-caster export is an entirely blank features continuation page, and that long class strings auto-shrink rather than clip -- the failure mode the modifier boxes should copy.
…re errors The template's field names do not describe their contents. features-and-traits receives the EQUIPPED ITEM LIST and prints under the EQUIPMENT label; features-and-traits-2 holds the actual features and actions on the continuation page; treasure holds unequipped items on page 2; and pdf_spec emits no :equipment key at all, so that box carries only coins. Two previously recorded bugs were mine, not the product's. The page 1 fields are NOT offset from their printed labels -- the fixture put features text into the equipment field and the render looked wrong. And page 9 is not blank in real exports; the fixture simply never set features-and-traits-2, which pdf_spec has always emitted and routes.clj has always sized. Both came from reading field names and inferring behaviour instead of reading the code that fills them. The fixtures now populate these keys the way the app does and carry a comment naming the trap, so the next person reading features-and-traits does not assume it means what it says. Unaffected and still standing: the missing spells-3-11, modifiers clipping at +10, the silent skip in write-fields!, and the six-class ceiling.
… test UI: when a class was filled from an SRD class, the starting-equipment section shows "Based on the <Class> class — only your changes are saved on export" with a Detach button. Detach (::class5e/detach-starting-equipment-base) drops the base link so export writes the equipment in full; the equipment itself (the full form the builder holds) is untouched. Test: test/browser/starting_equipment_ledger_e2e.js drives the real app and asserts the whole delta boundary in the live runtime — fill records the base; collapse writes a delta-only form (~265 chars) carrying just the tweak; validate-import accepts that text; expand restores the full equipment identically; and the Based-on banner + Detach behave (marker cleared, equipment kept). 10/10 checks pass, 0 page errors.
Compared a real level 20 wizard PDF from the live site against the fixtures. The ghost fields ship: 1931 widgets, 1596 orphaned, an exact match for the local 1-spells measurement. And the file size is not the artwork -- images are 329 KB of 2679 KB, so 88 percent of the download is AcroForm structure. Pruning page-less widgets from that exact file takes it to 1313 KB, a 51 percent cut, verified three ways: no differing pixels across all 16 pages, all 248 valued fields present and unchanged afterward, and no surviving field left with an off-page widget. The value diff is the check that matters; pixel comparison alone is what missed the death-save checkboxes earlier. Confirms spells-3-11-1 is genuinely absent in production, and confirms the field-name mapping recorded in the retraction. Adds a further naming inversion the live file revealed: the ability boxes hold the modifier under the plain name and the score under the -mod name. The fixtures had these backwards; corrected. Also documents a production bug this branch already fixes. Every negative number on a live sheet renders as a missing-glyph box, because the baked appearance stream encodes the hyphen as 0xAD rather than 0x2D -- WinAnsi lists hyphen at both codes and the reverse lookup picks the soft-hyphen one, which renderers do not draw. Current output bakes 0x2D and renders correctly.
… PDF Claimed every negative number on a live sheet rendered as a missing-glyph box. A screenshot from a real PDF viewer shows the hyphen rendering fine. The tofu came from PDFBox's own renderer, which is what produces every screenshot in this document -- a limitation of the measuring instrument reported as a defect in the thing measured. The verifiable parts stand: the stored value is plain ASCII, and the appearance stream encodes the hyphen as 0xAD where this branch encodes 0x2D. Both are legitimate; WinAnsi lists hyphen at both codes and PDFBox's reverse lookup picks 0xAD. That is a note, not a fix, and the branch cannot be credited with fixing a bug that did not exist. Records the standing lesson, now the third instance in this file: PDFBox renders are valid for comparing two PDFBox renders, so the prune verification is unaffected -- same renderer on both sides. They are not evidence about what a user sees, and glyph or layout claims need a real viewer first.
After retracting a bug that turned out to be a rendering artifact, the other render-derived claim needed the same scrutiny. Re-checked by comparing the clip rectangle baked into the appearance stream against the string width at the font size in that stream: the clip is 12.4pt, a two-character modifier is 9.1pt and fits, a three-character one is 13.6pt and does not. The clip is an re/W/n operator inside the content stream, so this is what every viewer draws rather than a PDFBox artifact. The bug is real, and the threshold is any three-character modifier -- +10 and above, or -10 and below -- rather than +10 specifically. Nothing on the production sheet exceeds +9, which is why it shows no clipping there and why a high-level fixture is needed to see it at all.
This went wrong twice: first claiming every negative renders as a missing-glyph box, then over-correcting to nothing-to-fix once a real viewer proved otherwise. The measured position. Production bakes the hyphen as 0xAD where this branch bakes 0x2D. Both are within spec since WinAnsi lists hyphen at both codes. But 0xAD is the soft-hyphen slot, and two consumers break on it: PDFBox's renderer declines to draw it, and flattening then extracting the text yields eight U+00AD characters where minus signs belong -- so copying a character into another tool, or reading it with a screen reader, gives a number stripped of its sign. Branch output has none. A real viewer does draw it, with slightly wider spacing, so the damage in a normal viewing path is cosmetic and the original alarm was unjustified. 0x2D costs nothing and breaks neither consumer. Also records a check that proved nothing either way, so it is not repeated: unflattened text extraction never sees form values, because PDFTextStripper does not read widget appearance streams.
The banner was a hand-rolled .bg-light box. Reuse the existing persistent-callout plumbing instead: the .bg-warning class (styles/core.clj — tinted bg + border + radius) with the fa-info-circle / .orange icon idiom and .flex.align-items-c layout that shared-content-banner and the library health status already use. Same severity vocabulary, same look as the app's other contextual banners; no new styling. Behaviour unchanged (regression test still 10/10).
…ames Adds a working proof of concept (not wired in) that builds a 2-page base plus eight spellcasting class pages from one master: 10 pages, 1830 fields, zero duplicate names, 757 KB. Today's six-class template is 9 pages, 1407 fields, 1596 orphans and about 1.2 MB, and cannot represent a seventh or eighth class at all. More classes, more pages, fewer bytes, no ghosts. Unique naming is the crux rather than a detail. Fields sharing a fully qualified name are the same field in PDF and share one value, so ticking a checkbox ticks its same-named twin on another page. Generated pages must mint their own names. That also explains the orphans: the template carries hundreds of uniquely numbered checkbox fields because someone needed per-page-unique names for overflow pages, pre-made them, then cut the variants by deleting pages -- which leaves the fields behind. The ghosts are the residue of solving this by hand. The same mechanism covers spell pages added as a caster levels up, and a features-and-traits continuation that grows instead of being one fixed page. Also corrects the hyphen note. The guess that a last-write-wins encoding table picked 0xAD over 0x2D was tested and is wrong; this PDFBox maps hyphen to 45 only. The live file's 0xAD comes from an older PDFBox carrying the spec's duplicate entry, an upstream bug since fixed, so the branch gets the right byte from the version bump and should not be credited with fixing it.
Adds docs/kb/pdf-form-techniques.md covering what is proven to work and what is not: cloning a page by referencing its artwork rather than copying it, why every generated page must mint unique field names, why Splitter and PDFMergerUtility are both dead ends, and why orphan pruning has to be per-widget. Also records measurements that were previously spread across commit messages: text capacity per box at 8pt, what overflow actually does (the value survives and every line is drawn, then the clip rectangle crops it silently), the three- character modifier clip, and the field names that hold something other than what they say. Corrects the capacity figures. The earlier ones measured the first widget of each field, which is the orphaned one; measuring the on-page widget gives a much larger continuation page (~1476 words, not ~230) and shows the real pressure is the small page 1 boxes -- ideals, bonds and flaws hold about 14 words each. Adds a plan ordering the work so each step ships and verifies on its own, starting with the orphan prune and the reporting that would have surfaced all of this years ago. Flags one open question for review: whether small boxes should shrink then spill, or truncate with a visible marker. Silent cropping, today's behaviour, is the only option ruled out.
…he on-disk delta
- views: the legacy '(convert to editable form)' notice was a hand-rolled .bg-light box; it
now uses the same .bg-warning + fa-info-circle/.orange callout as the 'Based on' banner.
Both callout comments reference the severity vocabulary (library-management-and-conflicts.md)
and the delta format (starting-equipment.md) so the plumbing is easy to grep for.
- kb: document the start-from-a-class override delta in starting-equipment.md — the on-disk
{:starting-equipment {:base :fixed :groups}} format (data integrity: the delta lives only
in files, everything live is full form), keyless addressing, collapse/expand boundary,
fail-soft, the banner/Detach UI, and coverage.
The fields auto-size. routes.clj/font-sizes is consulted only when flattening, which write-fields! documents and testing confirms: 20 words with an explicit 8pt still bakes at 12pt. The template's /Helv 0 Tf means auto-size and PDFBox shrinks text to fit. So text is not cropped at a fixed length, it shrinks -- features-and-traits-2 goes 11pt at 600 words, 8pt at 1200, 5pt at 2400 -- and auto-size floors at 4pt, past which it clips as well. The failure mode is worse than plain cropping: the sheet silently becomes illegible before it starts losing anything. That makes the cutoff a minimum legible font size rather than a character count. Records word budgets per box at 7pt and 6pt floors, measured by binary search on the baked appearance stream, and revises the plan's overflow step accordingly. This supersedes the capacity table committed earlier, which assumed a fixed 8pt that only applies on the flattened path and so understated every box.
Three changes to pdf.clj, each independently useful. prune-orphan-widgets! drops widgets belonging to no page and then fields left with none. The style 1 templates were cut from a nine-page master by deleting pages, which removes the page but leaves the FIELDS in the AcroForm, so most of a sheet's widgets point nowhere. On a production level 20 export that is 1596 of 1931 widgets and about 88% of the download -- the artwork is only 329 KB of 2.6 MB. Such a widget cannot render or be filled, so removing it is lossless. Measured on that export: 1407 fields and 2679 KB become 333 and 1313 KB, with all 248 valued fields present and byte-identical and zero differing pixels across all 16 pages. The single-class fixture drops from 1385 KB to 406 KB. write-fields! calls it on the way out, after writing values and only when not flattening, since flatten consumes the form anyway. The pruning is deliberately per-widget. Several fields own widgets on more than one page, and keeping a whole field because any one widget is live leaves the others orphaned. write-fields! now returns and logs the field names the template had no field for. These were skipped in silence, which is how styles 2-4 shipped with no backstory field, how style 1's missing spells-3-11 went unnoticed, and how a character with more than six spellcasting classes loses two of them and 50 values without an error. Nobody could check nil. fit-text answers whether text fits a box at a legible size. These fields auto-size -- the template default appearance is /Helv 0 Tf -- so an overlong value is not cropped at a fixed length, it shrinks toward a 4pt floor and only then clips, leaving a sheet that is illegible before it starts losing words. The cutoff is therefore a minimum font size, set to 7pt. Wrapping reuses the existing split-lines rather than adding a parallel implementation; its width is in inches because string-width divides by 72. widget-box measures a widget that is actually on a page. Every field in these templates carries two and the first is the orphan, so the obvious call measures the wrong box -- that produced a capacity table off by a factor of six. Adds five tests: pruning preserves every value, the exporter leaves no orphans, unplaceable names come back sorted, fit-text round-trips head and tail without loss and handles blank and nil, and widget-box finds the large box. Full suite 298 tests, 1534 assertions, green.
The namespace docstring was a migration diary -- PDFBox 3.x upgrade notes from a one-off change any reader can see in git -- and the same 2.x-to-3.x point was made three separate times, in the docstring, an import comment and the font constants banner. None of it told a reader anything about these templates. Replaced with the four things that actually catch people out, each measured and cross-referenced to docs/kb/pdf-form-techniques.md: fields auto-size rather than crop, most widgets belong to no page, field names do not describe their contents, and same-named fields share one value. The PDFBox version notes survive as three lines at the end, which is all they were worth. Also tightens the comment blocks added with the prune and fit-text work, which restated context the docstring now carries once, and keeps only what is not obvious from the code: why pruning is per-widget rather than per-field, why widget-box cannot take the first widget, and the measured word budgets. Drops a redundant java.net.URL import by using the one already declared, and imports HttpURLConnection rather than fully qualifying its type hint. No behaviour change. Full suite 298 tests, 1534 assertions, green; all three fixtures build unchanged.
Some of what went in with the last two commits was written for someone reviewing the change rather than someone maintaining the code: a cross-reference to a numbered note that will break when the list is reordered, before-and-after file sizes from one August measurement, and a docstring paragraph that enumerated three historical bugs and then argued for its own existence. The filter applied: a comment earns its place if it changes what a maintainer does. History, measurements and justification belong in the knowledge base and the commit log, and are already there. So write-fields! now says what it returns and that a missing field is common rather than always an error, naming the three cases in a clause instead of three sentences. The orphan note says why removal cannot lose anything and drops the specific byte counts. The word budgets are no longer inlined; the comment points at the knowledge base and says to compute from the widget. That comment also claimed a test pins those budgets, which was not true when written, so this adds one. It binary-searches the real capacity of each box and fails if it drifts by more than a factor of two -- enough to catch a template being re-cut, loose enough not to fail on a word. Full suite 298 tests, 1534 assertions, green.
The comments described how the code came to be rather than what it does. The namespace docstring opened by addressing the reader, the field-name mismatch was written as a claim to be argued, and two section banners restated their own docstrings. Now each comment states behaviour and names the function that handles it. The namespace lists the four template properties a caller has to work around, with the misleading field names as a plain mapping table pointing at the pdf_spec functions that build them. Function docstrings give arguments, return value, and the one non-obvious implementation constraint each: prune filters widget lists because a field may span pages, widget-box measures an on-page widget, fit-text converts points to the inches split-lines expects. Background and measurements stay in docs/kb/pdf-form-techniques.md, referenced once at the end of the namespace docstring. No behaviour change. Full suite 299 tests, 1541 assertions, green; fixtures build unchanged.
GC is ruled out. The 1068 ms block coincides with ALLOCATING 31 MB (134->165), while the two actual collections (-16 MB, -19 MB) landed on fast switches (118 ms, 95 ms). So one switch realises a large amount of content in a single blocking task rather than pausing to collect. Adds the class-option / make-levels / spell-selection / memoized-spell-option / entity-build counters to the tab probe, reset per switch, to name what runs on the expensive one. Counters were previously only wired to the class dropdown, which is not the interaction that freezes. Run in flight.
The spy columns came back empty on every tab switch including the 1160 ms freeze. That is only a finding if the counters could have registered at all -- otherwise it is a dead probe reporting silence, which this repo has been burned by before. Prints the armed counter names after builder load, and drives a class dropdown pick as a positive control: that is known to call memoized-spell-option. If the control registers and the tab switches still show nothing, then no builder function runs during the freeze and the 45 MB it allocates is render work. Run in flight.
Reproduced on mega-64 at 4x throttle flipping Race<->Class: one switch in eight blocks 1160 ms while neighbours run ~100 ms, allocating 45 MB. Not GC -- the long task allocates, while the actual collections all landed on fast switches. Not any builder function either: class-option, make-levels, spell-selection and entity/build all read zero during a tab switch, and a positive control in the same run proves the counters fire. It is 75 memoized-spell-option calls plus rendering what they produce, ~104 ms of a ~1000 ms block being the calls themselves. Fires the first time a spellcasting class's options render, hence intermittent. Records why three earlier probes missed it: they drove the class dropdown rather than the tab switch, ran unthrottled, and reported totals instead of the longest single task. Both missing variables were in the original report.
Counter-based attribution needs a guess about which function to count, and the guesses have been wrong three times running (GC, the spell-option memoize, class bodies), each costing a run to disprove. This profiles the blocking switch itself and reports where the time actually goes. Profiles two steps at 4x throttle: landing on the Class tab, and picking a spellcasting class -- the event measured to allocate ~45 MB and block ~1 s. Run in flight.
The first profile missed the freeze -- both steps ran fast (291 ms, 390 ms), so it described a normal interaction, not the 1160 ms block. Reading it as an explanation of the freeze would have been the same error as before. Profiles 5 Race<->Class round trips continuously and prints the longest single task next to the ranking, labelled FREEZE CAPTURED or not, so a fast sample cannot be mistaken for an explanation of a slow one. For the record, the normal class pick ranked: option-cfg 80 ms, selection-cfg 65 ms, hit-points-selection 49 ms, level-option 49 ms, modifiers 45 ms, hashing 68 ms -- building the class's LEVEL options, not rendering spell options.
Advanced compilation munges names, so window.cljs.core and window.re_frame do not exist in a prod bundle and the app-db plugin count silently reports 0 forever -- every import would time out rather than fail loudly. Falls back to the localStorage blob's size, which is build-independent. Either signal growing means the import landed. Needed to run the freeze probes against a production build, to check how much of the measured 1279 ms block is a :optimizations :none artifact.
Built prod (advanced compilation) and re-ran the same probe on the same pack at the same throttle. Switch 2 -> Class/Level still blocks: 654 ms on prod against 1068-1279 ms across four dev runs, same position and same allocation shape (+23 MB vs +31-45 MB). So it is a real product bug, not a :optimizations :none artifact -- advanced compilation halves it and does not cure it. Adds SKIP_CONTROL=1, and records the confound that made it necessary: the probe's positive control picks a caster BEFORE the tab loop, which realises the expensive content up front and suppresses the freeze. The first prod run included it and showed nothing; comparing that to the uncontrolled dev runs would have wrongly concluded the freeze was a dev artifact.
Self time cannot see this block. Allocation-heavy code parks its cost in (program) and GC, so every named app frame came back under 100 ms while a 1051 ms freeze went unattributed. Inclusive time names the function that CONTAINS the work instead. Also warms to the state that precedes the freeze (one Class visit, one Race visit) and profiles only the second Class visit, rather than averaging a freezing switch together with nine fast ones. Prints whether the freeze was captured, so a fast sample cannot be read as an explanation of a slow one. Context: with the spy armed and no control, the 1051 ms block fired ZERO counters -- no memoized-spell-option, class-option, spell-selection, make-levels or entity/build. So it is not spell options, which was the leading guess.
Inclusive-time profiling of the freezing switch puts all 1274 ms inside ONE synchronous React render: run-queue -> flushSync -> beginWork -> deref-capture -> do-render, 1047 ms of it, with option-cfg / selection-cfg / level-option as the self time underneath. So the template is rebuilt during a render. Earlier counters read zero because they wrapped class-option and make-levels -- the wrong layer. Adds template, template-selections and level-option. Working hypothesis this tests: leaving the Class tab unmounts the last subscriber to the template subscription, re-frame disposes it, and returning recomputes it from scratch mid-render. That matches the otherwise odd first-visit-cheap / second-visit-expensive shape.
…at is used FREEZE ROOT CAUSE. Rendering the Class tab calls level-option 2820 times in one synchronous React render -- 906 ms of a measured 1099 ms block, allocating 45 MB. 2820 is 141 classes x 20 levels: it builds every level of every class in the library, including the 140 the character has not taken. The amplifier is chunking. clojure.core/range returns a CHUNKED seq, so (map f (range 1 21)) realises all 20 levels as soon as anything touches the seq -- calling seq or first on it is enough -- and rendering the class list touches every class's levels. A list is not chunked, so only what is actually consumed gets built. Attribution came from inclusive-time profiling: the whole block sits inside one reagent render (run-queue -> flushSync -> deref-capture -> do-render), with option-cfg / selection-cfg / level-option underneath. Earlier counters read zero because they wrapped class-option and make-levels, which are a different layer and genuinely do not run on a tab switch. Effect NOT yet measured -- build and probe in flight. Follow-up commit carries the after-number, or reverts.
codeGlaze
force-pushed
the
perf/homebrew-builder-loop
branch
from
September 5, 2026 23:20
936e151 to
d3b3203
Compare
No effect: still levelOption 2820x907ms and a 1094 ms block, identical to before. So the consumer fully ITERATES all 20 levels rather than merely touching the seq, and chunking was never the amplifier. Reverting rather than leaving a no-op change carrying a comment that asserts a wrong mechanism. The 2820 calls and the freeze are still real and still unexplained; next step is capturing the stack at a level-option call to name the caller instead of inferring it.
Four inferences about this freeze have now been wrong: the spell-option memoize, class bodies, GC, and chunking. The chunking one produced a one-line "fix" that changed nothing and had to be reverted. Reading code is not identifying this caller, so record the stack at the first call of the burst instead.
The instrumentation is injected as a JS template literal, so the backslash-n I wrote inside split() became a REAL newline and broke the script at parse time. The spy then armed for nothing and the run died on an undefined counter map. Uses String.fromCharCode(10) so nothing depends on an escape surviving two layers, and tolerates an undefined counter map instead of throwing. The 'spy armed for:' line added earlier is what made this obvious rather than another run of silent zeroes.
Every class's 20 level options are realised during the Class tab render: 2820 calls (141 classes x 20 levels), 939 ms of a 1128 ms block, 46 MB allocated. The seq at options.cljc:3098 is lazy, so it costs nothing at template-build time -- which is why builder open never showed it and why class-option and make-levels counters read zero -- but rendering the class list forces it for every class in the library, including the 140 not taken. Stack confirms LazySeq realisation inside one synchronous reagent render. Reproduces on dev and prod. Records the candidate fixes, the one that was tried and reverted (unchunking: no effect, the consumer iterates fully), and a table of five wrong diagnoses with what killed each. Every wrong turn came from reading code; every correction came from a measurement. Also records two probe defects that cost runs: a control that suppressed the freeze it was meant to measure, and an escaped newline that silently disarmed the instrumentation.
…ible V8 caps stacks at 10 frames by default, so the capture bottomed out inside LazySeq internals (sval) and never reached the code that forces the seq. The slice was asking for 13 frames that were never collected. Raises the limit to 200 and keeps 60 frames.
__loStack was set on the first call ever, which happens during builder load, so the stack described the load rather than the freeze. It did show the load path (entity/build -> collect-modifiers-2 -> get-all-selections-aux-2 realises level options), but the freezing switch records zero entity/build calls, so a different consumer forces them there. Resets per switch and prints only for a switch that actually blocks.
FREEZE ROOT CAUSE. make-options-map was memoized on the options seq itself. Looking up that cache has to hash the key, and hashing 141 class options walks each one's nested selections -- including every class's lazy 20-level :options seq. So a cache LOOKUP realised 141 x 20 = 2820 level-options in one synchronous reagent render: 939 ms of a measured 1128 ms block, 46 MB allocated. The work being cached is a zipmap over ~141 items. The key cost orders of magnitude more than the result. Found by capturing the stack at a level-option call during the freezing switch specifically, which named class-level-selector's make-options-map call. Five earlier diagnoses inferred from reading code were all wrong (the spell-option memoize, class bodies, GC, spell data, range chunking); each died to a measurement. See docs/kb/perf-homebrew-builder-loop.md. Effect NOT yet measured -- build and probe in flight. Follow-up carries the after-number or reverts.
…he library ROOT CAUSE of the years-old builder freeze. cljs.core/memoize keeps its cache in a PersistentArrayMap and looks it up with get, which linear-scans comparing argument lists using =. set-class was memoized on [i options-map], and options-map holds EVERY class in the library. So each lookup deep-compared 141 class options and, in doing so, forced their lazy 20-level :options seqs: 2820 level-option calls, ~1 s blocked, 46 MB allocated, in one synchronous reagent render. The value being cached is a three-line event-handler closure. The key cost three orders of magnitude more than the result. filter-classes had the same shape (caching a one-line predicate) and is unmemoized too. make-options-map's comment is corrected: it is the same anti-pattern, but removing its memoize alone changed nothing, so it was not the cause. Found by capturing a deep stack at a level-option call during the freezing switch: reagentRender -> memoize's G__ delegate -> get -> array_map_index_of -> = -> equiv_map/equiv_sequential -> seq -> sval -> level_option. Effect NOT yet measured -- probe in flight. Follow-up carries the after-number or reverts.
…tion too set-class was one of three memoizes in this render path keyed on every class in the library: delete-class takes options-map, add-class takes a seq of class options. Removing set-class alone left the freeze unchanged (2820 level-option calls, 1125 ms), so the others were still deep-comparing on every lookup. Also corrects the set-class comment, which asserted a measured root cause that the next run disproved. The mechanism is confirmed -- memoize's array-map lookup deep-compares argument lists and forces the lazy level seqs, seen in the freeze's own stack -- but removing one site was not sufficient, and the comment should not have claimed otherwise. Effect NOT yet measured -- probe in flight.
Replaces the interim root-cause section (which named level-option realisation without explaining what forced it) with the confirmed mechanism: cljs.core memoize stores its cache in a PersistentArrayMap and looks it up with get, a linear scan comparing argument lists with =. Three handler factories were memoized on arguments containing every class in the library, so each lookup deep-compared 141 class options and forced their lazy 20-level seqs. Measured: switch 2 to Class went 1125-1283 ms to 100 ms, level-option calls 2820 to 0, session heap peak 189 MB to 141 MB. Suites green: JVM 309/1704, CLJS 240/726. Notes why it survived years of use -- it scales with homebrew volume, needs CPU contention to show, and the expensive operation is a cache lookup, so profiles pointed at rendering rather than at the caching.
…try points The perf docs referenced verification-discipline.md, which did not exist. Writes it from what this investigation actually cost: six diagnoses inferred from reading code, all wrong, each killed by a measurement; and nine probe defects that produced confident wrong answers (a control that suppressed the thing it measured, instrumentation that could not intercept, a dead probe reporting silence, with-redefs unwinding before async work, escaping through two layers, V8's 10-frame stack cap, a once-per-session capture describing page load, self time on allocation-heavy code, and a model that was not the app). Adds docs/kb/README.md indexing every KB doc and every browser probe with what it answers, and links it from CLAUDE.md, the reagent tenets and the documentation tenets so an agent arriving cold can find it. All intra-KB links verified to resolve.
Production build, same pack and throttle as the run that previously froze at 654 ms: switch 2 to Class now 92 ms, worst switch across the session 140 ms, session heap 102->111 MB instead of 102->152 MB. No freeze. Suites green: JVM 309/1704, CLJS 240/726.
…tional test Comments: one authoritative block above set-class-fn explaining the mechanism and the measured cost, with short DO NOT MEMOIZE markers pointing at it from each of the five trouble sites, so the defect cannot be reintroduced by someone who only reads one of them. Scan: all 43 (def x (memoize f)) sites classified by argument shape and each large-argument site traced to its callers. Four are DEAD -- three in entity.cljc and remaining-adjustments in character_builder.cljs are defined but never called. memoized-spell-option is measurably ~10x SLOWER than no cache (75 calls x 21 ms memoized vs x 2 ms passthrough), because its key includes the whole spells-map. make-inventory-item and export-pdf-handler are live and key on large structures. Planned in four ordered steps for a separate branch, with an analysis of what removing a memoize can break (prop identity, re-render skipping, stateful closures). Functional test: the suites do not click anything, so the four handlers that were unmemoized had no coverage. class_handlers_functional_e2e.js drives them and asserts app-db. Run in flight -- no merge until it passes.
The first run skipped add-class and delete-class because the selectors were guessed. The add button reads 'Add Levels in Another Class' and delete is an fa-minus-circle icon; both taken from character_builder.cljs rather than assumed. Those two handlers are among the three that were unmemoized, so skipping them left the change unverified. set-class, set-class-level and built-character passed on the first run.
All five checks pass, including add-class and delete-class which the first run skipped on a guessed selector.
…builder-loop # Conflicts: # test/browser/README.md
Owner
Author
|
Fully contained in integration (branch tip is an ancestor of origin/integration); it reaches develop through #32. |
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.
salvage-plugin-items / salvage-library-items split a source into its valid and invalid items; reconcile-rejected-items maintains the quarantine so an item is never both live and set aside. content-specs gains valid-item-for-load?, the per-item mirror of the source floor.