Tier 8, waves 7-14: i18n, blocks, embed assets, data contract, CSS, lint gates, tests - #25
Merged
Conversation
M7. Seven different "West Africa" defaults had accumulated for one region, so the same six countries opened at a different scale depending on which block you were reading. `P.WEST_AFRICA_VIEW` and `P.FIT_OPTS` are that view and that padding; the three maps whose subject really is wider — source provenance, scholarship provenance, the admin-boundary overlay — pass their own and say why. The globe toggle shipped on ten of twelve maps while ROADMAP §4 and §10 both list globe projection as "won't do", because the factory defaulted it on. It is opt-in now, and nothing opts in. Rotation and pitch are off on flat thematic maps, so the compass that resets a bearing nothing can change and the dial that visualises an angle that is always zero are off too. The references map had two fullscreen buttons doing the same thing to different elements. M8. MapLibre's own strings stayed English on the French site — a screen reader on islam.zmo.de/s/afrique_ouest heard "Zoom in", "Enter fullscreen", "Close popup". Every key MapLibre 6.6 reads, verified against the pinned bundle's own defaultLocale, is translated. Eleven of twelve map hosts also carried no accessible name at all, so the reader reached an interactive canvas called "application"; `config.title` is now both `Map.Title` and the host's aria-label. M9. `ns.prefersReducedMotion` has existed since v1.30 and the canvas graphs honour it; no map did. Eleven animated camera moves go through `P.mapMotion(ms)`, which returns 0 under the preference. M6. Every map on every page carried `preserveDrawingBuffer: true` — the whole site paying on every frame for a PNG export almost nobody runs. The toolbar reads the canvas inside MapLibre's own `render` event instead, which is the moment the buffer is both filled and still readable; that made `resolveDataUrl` a promise, which both callers already were. M5. The choropleth wrote its counts into the GeoJSON as `_iwac_count`, so every count change re-ran `setData` and MapLibre re-parsed and re-tiled ~200 KB of polygons in the worker — the keywords-attention slider does that per tick and per play step. The counts are a `match` expression on the country name now: geometry uploaded once, a count change is one `setPaintProperty`. M3. The laïcité places view stopped leaking a WebGL context in v1.59.0 but still built a new one per activation, and browsers cap live contexts around sixteen and silently lose the oldest. The view is parked between switches, like scary-terms'. Its ranked-places list — the keyboard route to the same data — is rebuilt on update, because a parked view that refreshed the map and not the list would have left the two disagreeing. M19. The popup bounds were ~150 lines: six overridden methods, a `resize` listener per open popup, and three layout constants transcribed from MapLibre's stylesheet and left citing 6.3 while the pin moved to 6.6. `.iwac-vis-map` is a size container now and `iwac-maplibre.css` states the two guarantees directly — half the map's usable height minus the tip, two thirds of its usable width. One line of JavaScript survives and it is not styling: MapLibre picks its anchor by comparing the marker against `maxWidth`, the OPTION, not the box CSS produced, so left at 320px it chose a left/right anchor one threshold early and hung the popup outside a narrow map. `addTo` reads back the resolved width and hands MapLibre that number. The browser spec that pins the guarantee at all seven anchor thresholds passes; it caught two real errors on the way here — the usable box is the map minus MapLibre's 16px anchor inset, not the whole map, and a `min-width` floor beats a `max-width` cap. M21. Nothing outside the asset partial stated the current pin, so ARCHITECTURE.md now says the partial is the only place versions live and that the historical 5.24 entries are records of a decision, not claims about today.
E2. The bar chart race animated bar lengths in fixed slots while the labels swapped instantly, which is a bar chart that changes. Three things were wrong at once: `realtimeSort` was never set, each frame sliced its own top ten and re-supplied the category axis, and every frame was `notMerge`, so there was nothing to animate from. It is now ECharts' documented recipe — every term against a fixed axis ordered by final total, `realtimeSort` doing the ranking, `yAxis.max` bounding the podium at ten, `label.valueAnimation`, timings matched to the 1 s tick with linear easing, and an advance that MERGES `series[0].data`. `C.scaryTermItems` builds those items for both paths, because a merge replaces the array wholesale and bare numbers would have dropped every per-term colour on the first tick. Under prefers-reduced-motion the theme still sets `animation: false` and the frames snap, which is the right reading for a chart whose content is motion. E11. Two heatmaps painted their axis labels `muted` where the registered theme paints every other axis on the site `inkLight` — not a duplicate but a contradiction, and the heatmaps were the only charts whose axis text sat below everyone else's contrast. Gone, along with the `axisLine` and `visualMap.textStyle` restatements the theme already owns. `'#fff'` on the segmented bar's in-bar percentage was a genuine hardcode measuring 3.15:1 on the palette's orange; `C.inkOn(fill)` picks between the theme's two extremes by contrast, which is what the treemap has done per tile since v1.53. The `fontSize` literal appeared at four values for one decision and is `P.AXIS_FONT_SM` — in panels.js, not chart-options.js, so a block that skips the chart bundle gets a number rather than `undefined`. E12. `progressive: 2500 / progressiveThreshold: 3000` on the semantic landscape was configuration that did nothing: ECharts compares the threshold against ONE SERIES' length, and every facet splits ten thousand points into buckets of a few hundred. Deleted rather than tuned. `large: true` would engage and is rejected — it drops per-point emphasis, which on a map whose purpose is inspecting individual points is the feature, not the cost. The other half of E12 was real: the collaboration network was bounded by `min_degree`, which limits how CONNECTED a node must be, not how many survive — a denser dataset raises every degree and keeps more. The front end lays that graph out with `layoutAnimation: false`, synchronously, on the main thread. `MAX_COLLABORATION_NODES = 400`, most prolific kept, count of dropped nodes in the payload. E13. `echarts.init` took no options, so ECharts worded anything it generates itself in its own default language rather than the site's. `registerChart(el, render, initOpts)` passes the locale and forwards whatever else a caller wants — `renderer: 'svg'` for a small static chart is now available rather than unreachable. E15, decided rather than deferred. ADOPTED: `legend.selector` on the two legends dense enough to need it (twelve term-trends series, thirty LDA topics), with translated button labels. SKIPPED, with reasons recorded: the `matrix` coordinate system, which closes ROADMAP 4.6 — its one distinguishing feature is column super-headers and neither matrix wants one; piecewise `visualMap` on the centrality heatmap, because that cell is a MEAN on a 1-5 scale and five discrete pieces would present a continuous quantity as category membership. E16. `C.itemTooltip(title, lines)` is the shape fifteen formatters built by hand. It takes the title and the lines separately on purpose: the title is always a datum and is escaped here, the lines are HTML the caller composed, and folding them into one string is how an unescaped title eventually ships. `C.tooltipDot` is ECharts' own marker for a row a formatter draws itself.
…panels
S10. The i18n lint read one file — about 55 % of the module's strings. The
other 45 % live in thirteen dictionaries that call `addTranslations`, the
newest code in the module, watched by nothing. It reads all fourteen now,
pairs them, and reports a per-block key that shadows a shared one.
It found thirteen shadowings on its first run, twelve redundant and one
real: `Top subjects` rendered as *Principaux sujets* from the item-set
dashboard and *Sujets récurrents* from everywhere else, and which one a
page got depended on bundle order. Every shadow is gone; parity is
checked on the merged dictionary, because that is what `t()` resolves
against. Both new rules are proved able to fail.
S14. `t()` interpolated and nothing more, so a dashboard reporting one
article said "1 articles", and French "0 article" could not be expressed
at all — English and French disagree about zero, which is why this is an
`Intl.PluralRules` lookup and not an `n === 1` test written once and
wrong on one of the two sites. `key_one` / `key_other` are tried before
the bare key, so nothing had to be migrated and a string that does not
vary needs no variants. The three hand-rolled exceptions with three
different conventions are gone. `P.labelColon` gives French its
non-breaking space before the colon — `Pays :`, not `Pays:`.
S12. The keyword explorer read its current page back out of the
PAGINATION LABEL with a regex and re-attached its Add / Remove buttons
from a MutationObserver on the tbody. A change to that copy, the page
size or the French wording broke the buttons silently. `P.buildTable`
takes `render: function (row, td)` now and builds the cell from the row
it is already holding; `api.page()` exposes the cursor for anyone who
needs it. Forty lines of DOM archaeology deleted.
S13. Embed panel slugs were positions in document order, and three
blocks do not always render the same panels: sentiment-atlas inserts per
model, periodicals drops one for an old bundle, and a panel built after
the settle window was never counted. Any of those silently renumbered
every permalink after it. `P.buildPanel(…, { key })` names a panel and
enumeration prefers the name; the seventeen panels in those two blocks
are named.
S16. `compact` was sampled once per draw, so rotating a phone, opening a
panel to fullscreen or dragging a window past 600 px left the dense
layout on a narrow chart until something else happened to redraw.
`registerChart`'s existing ResizeObserver re-runs the render when the
answer changes — and only then. `P.isCompact(el)` reads the ELEMENT's
width; the arenas grid read `window.innerWidth` and so laid a 400 px
embed out in three columns.
S20. Three empty states bypassed the shared banners and so announced
nothing to a screen reader. The scary-terms matrix built an ECharts
`graphic` text that re-resolved `--muted` and the font family by hand,
two lines from two calls to the shared option that does the same thing.
S21. Spatial's pinned popup waited 720 ms for a 700 ms `easeTo` — two
numbers kept in step by hand, and three quarters of a second of nothing
under prefers-reduced-motion. It waits for `moveend`. The embed layer
fired seven synthetic `window.resize` events over 1.5 seconds hoping one
landed after the reflow, waking every unrelated listener on the page six
times over; `ns.resizeCharts()` re-measures what this module tracks, and
a ResizeObserver answers when the layout actually settles.
S23. `P.fetchJSON` de-duplicates concurrent requests for one URL —
item-set-dashboard and compare-newspapers both fetch the same index, and
every minimal-item container on a page fetches the same summary. Only
the in-flight promise is shared, so this is de-duplication and not a
cache: a failure is not remembered. The choropleth's in-flight half is
retired; its RESULT cache and spatial's LRU are not the same thing and
stay, with a note saying so.
S24. The shared dictionary was 104 KB on every block page, including
article pages that use about a tenth of it. 116 keys belong to exactly
one block — the five the finding names — and now live in
`charts/<block>/i18n.js`, loaded first in that block's bundle.
`shared-core.min.js` drops from 134.8 KB to 118.6 KB on every page.
Which keys could move was derived, not guessed: every `t('literal')` in
`asset/js` was attributed to the bundles that carry the file, and a key
was only a candidate when every reference resolved to one block bundle
AND the key was not named as a bare string anywhere outside it — a
per-block `TYPE_I18N` map elsewhere would have made it look local while
being reached dynamically. Thirteen candidates failed that second test
and stayed.
The move is CHECKED, which is what makes it safe to do at all.
`check-i18n.js` gained a reachability rule: `addTranslations` merges at
runtime, so a per-block dictionary is only in memory on a page that
loaded that block, and a key moved into the wrong one renders as itself
— silently, and only on the pages that do not load that block. The rule
walks every literal `t()` key in every bundle and fails when it is not
reachable from shared plus that bundle's own dictionaries.
It found a live-adjacent bug on its first run, before any key moved:
`charts/shared/concordance.js` is in `shared.ui` and reached for
`concordance.tagged_hint`, which lives in the LAÏCITÉ dictionary. Only
laïcité uses the component today, so nothing is broken in production —
but the string names "Laïcité" specifically, so the hint is the caller's
to supply now, and the second block to use the shared concordance gets a
tooltip rather than a raw key.
S19. The last two `<select>` elements in the module rendering as the
browser's default control got `.iwac-vis-control`; a test now walks
every `P.el('select')` and fails on a bare one.
S4. Five collection-overview panels each built their own Country facet,
so picking Bénin on the timeline left the Gantt, the languages bar and
the word cloud on "all countries" — the same choice made five times, and
any two panels free to disagree about what the reader was looking at.
The laïcité dossier held four separate country keys for the same reason:
Togo on the timeline had to be picked again on the map, the arenas and
the concordance.
`P.linkFacet` is the contract: a panel says how to READ its current
country and how to APPLY one, and publishes from its own facet's
onChange. A block store holds the value. `P.buildLinkedFilterBar` is the
chip under the block heading that says a filter is active and clears it
— without it a linked selection is invisible, and four panels narrowing
with no visible cause is worse than four panels not linked at all.
Two things the shape has to get right, and does:
- The loop. A facet bar's `setActive` fires its own `onChange`, so a
panel applying a value that came FROM the store would publish it
straight back, and two linked panels would ping-pong. The applying
flag stops it; a test asserts each listener moves exactly once.
- The ordering. `apply` normally reaches back for the object
`linkFacet` is still returning — the caller writes
`var link = P.linkFacet({ apply: … })` and its apply calls
`link.publish`. So the initial adopt is a microtask; running it
synchronously is a ReferenceError in strict mode and a silent no-op
behind an `if (link)` guard, neither being what a caller meant.
A panel that renders late — the lazy map, the deferred word cloud —
adopts whatever the block already holds rather than opening on "all".
The two multi-mode panels clear back to Global rather than to an empty
country picker, and ignore a country they have no slice for, so linking
can never blank a panel.
The country bar and the treemap are selectors now: they were the two
charts on the page that named a country and did nothing when clicked.
Clicking the active one clears the filter.
Not linked, deliberately: the collection map's "By country" facet is a
mode toggle plus a TYPE picker that draws a choropleth of every country,
not a single-country filter — there is no country in it to share. In
laïcité, the four keys are kept in step through the reducer that already
holds the block's cross-field rules, and only for a country the dossier
actually has: an unknown value would narrow a view to nothing.
P8. `extract_year` called `pd.to_datetime` before its regex, and it is called from ~40 sites several passes deep over 12k rows. Nearly every date here is "YYYY", "YYYY-MM" or "YYYY-MM-DD", so the ISO shape is matched first and everything else still goes to pandas: 275x faster on that shape, and verified output-identical against the pre-change function across 31 inputs. The two bare `except Exception` around it now name what can actually be raised — a bare catch also swallowed a KeyboardInterrupt and returned None as though the date were unparseable. P10. The output contract was convention. `scripts/validate_data.py` makes it a contract: every expected bundle parses, carries a generation timestamp, and has the top-level keys its panel reads. The key table is mirrored by hand from the consuming JS and says so — a regression net for the shapes that exist, not a generated schema — and the script has a self-test that proves each check can fail, wired into the lint workflow where it needs no data. It found two bundles with no provenance at all. `laicite-trends.json` has a timestamp now; `scary-terms-temporal.json` is a bare year -> counts map with nowhere to put one, so it is exempt by name rather than by inference. All four metadata idioms are accepted — forcing one shape would mean re-reading every consuming panel — but "no timestamp" and the `+00:00` form are not. The three fan-outs that emitted `+00:00`, plus two more raw `isoformat()` sites, go through `generate_timestamp()` now, which is what Tier 5 already claimed. P11. `tests/python/requirements.txt` pinned numpy 2.5.1 while numba holds the generators below 2.5 and the lock resolves 2.4.6 — the unit tests ran on a numpy the generators cannot use, which is the one configuration a test suite must never be in. They match, and `lint:python-lock` fails when they drift again. `scripts/README.md` told local users to install the lock with `--require-hashes`, which cannot work off Linux/3.12/x86-64: its hashes name wheels for the runner. CI gained a failure summary; a 90-minute job that fails otherwise leaves nothing but a red tick. P15. The argparse prologue was hand-rolled in 19 of 31 generators. `add_standard_args` + `parse_standard_args` replace it — 327 lines deleted — and the migration is verified rather than eyeballed: a harness captured every generator's parsed defaults and full option list before and after, and all 31 are identical. The seven bundles that ship pretty-printed are documented as deliberate rather than left looking like oversights. P16. Four generators counted a raw `country`, so "Benin", "Bénin" and "benin" were three countries in the same chart. `iwac_utils.IWAC_COUNTRIES` is the one list; the spatial generator's `FOCUS_COUNTRIES` reads it. P18. `copy_to_build` had no callers and targeted a `build/data` this repo does not have. `check-python.js` hard-coded one contributor's absolute Python path — it looks for `$PYTHON`, then the project venv, now. P19. Tests for two of the three largest untested generators. They found a real bug on the first run: `compute_timeline` read the `country` cell whole while `compute_country_distribution` one screen down splits the pipe, so a value like "Bénin|Togo" became its own stacked series — the country bar and the timeline of the same block disagreeing about what a country is.
H5. Nineteen of the twenty-one page-block templates were the same single call to `iwac-block-shell` with a different literal array — the CSS sheet, the `needs` flags, the bundle, the loading message. That put a block's asset declaration somewhere other than every other fact about it: label, description, embed slug and embeddability all live in `BlockRegistry`, which the block layouts, the embed controller and `check-blocks.js` all read. "Which blocks load MapLibre?" was nineteen files to open. The array is a `shell` key on the registry row now, and those nineteen render through one `_generic.phtml`. The two blocks that do more than declare keep their own templates — collection-overview server-renders its summary from the snapshot on disk, on-this-day reads a layout setting — and the routing is on the key's presence, so adding logic to a block means adding a template, not editing a method. `check-blocks.js` grew with it, and its new rules are the interesting part: a block with NEITHER a shell nor a template renders nothing, and one with BOTH renders whichever the router picks. Both now fail the build, as does a shell that declares its own `embedSlug` — `_generic` passes the registry key, so a second copy could only disagree with it. Its registry parser had to learn indentation: the arrays a shell nests were being read as rows of their own. 62 more PHP checks. H2. The sync job's temp trees are job-scoped and cleaned in a `finally`, which does not run on a SIGKILL or an OOM kill — and the cleanup only ever removed the CURRENT job's siblings. Every hard-killed sync left roughly 18k files under `files/iwac-visualizations.tmp/`, forever. The job sweeps them at start, under the exclusive lock it already holds, and only entries matching the job-scoped names it creates. It also refuses to extract without room: twice the expanded size, which the archive scan already computes, because the staging tree and the outgoing tree coexist until the second rename. A partial extraction is the worst outcome available — it passes the marker check and publishes a truncated dataset. And it retries a download once, on the three transport errno values only: 7, 28, 56. A 300 MB CDN transfer fails sometimes for reasons that have nothing to do with the archive, and a single blip used to mean a red job and a manual re-run; an HTTP 404 or a TLS failure still fails immediately, because those will fail again.
H6. The on-view loader was ~50 lines of string-concatenated JavaScript built inside `iwac-assets.phtml`, which made it the only inline script on a block page. A host with a `script-src 'self'` Content-Security-Policy therefore broke every block on the site, with no way to fix it: Omeka's headScript has no nonce plumbing. It was also the one piece of JavaScript deciding when everything else loads, and the only piece with no lint and no tests — `assets.test.js` had to parse it back out of rendered PHP concatenation to check anything about it. It is `asset/js/iwac-lazy.js` now, bundled and loaded like any other script, and each block's payload travels as `<script type="application/json" class="iwac-vis-lazy-manifest">` — inert under any CSP, because it is data. The tests read the file and supply the payload the way the browser does; a new test asserts neither partial calls `appendScript` and that the JSON is emitted with the HEX flags that keep a `</script>` in a filename from breaking out. The embed layout was the same story twice more: 32 lines of inline `<style>` and a 28-line inline `<script>`. They are `asset/css/ iwac-embed.css` and `asset/js/iwac-embed-height.js`. The brand accent splits along the line that matters — the per-request VALUES stay inline as a `style` attribute on <html>, which `style-src-attr` governs and a policy blocking `<style>` elements commonly still allows, while the RULES that consume them, including the dark-mode lightening that cannot be an attribute because it is a different selector, move into the sheet. The escaping comment that block carried is still true and still needed, and now says why an attribute is the one place `escapeHtmlAttr` is right. H7. Embed responses carry `Cache-Control: public, max-age=300`. They are public, read-only and fetched by third-party pages this module does not control, and every view was hitting Omeka's full bootstrap. Five minutes lets a data sync show up promptly and absorbs a page embedding several panels of one block. The data-sync admin page is `global_admin` only. A `site_admin`'s authority is over one site; the job that page dispatches replaces the whole `files/iwac-visualizations/` tree that every site on the installation reads, with a several-hundred-megabyte download in front of it. That is not a per-site operation.
C4. `npm run lint:css-dead` is the detector the finding asks for. The hard part is that most unmatched class names are not dead — they are composed at runtime, `'iwac-vis-table__cell--' + (col.render || 'text')` — so the allowlist names the STEMS a concatenation actually builds, each with the file that builds it. A blanket "ignore anything with a modifier" would have silenced every real finding. It found six dead rules, not the four the audit counted by hand: `.iwac-vis-article__body`, `.iwac-vis-entity__body`, `.iwac-vis-minimal-item__body`, `.iwac-vis-reference__body`, `.iwac-vis-publication__body` and `.iwac-vis-scary-details-list` — five `__body` classes left behind by a rename that gave every dashboard block a different root class. `.iwac-vis-entity__body` shared a rule with `.iwac-vis-person__body`, which is live, so only the selector went. C3. Eight `z-index` literals across six sheets — 1, 2, 5, 6, 30, 1000 and a -1 — with nothing saying which sat above which. They are three roles: `--iwac-vis-z-raised` for a thing that lifts off its panel, `--iwac-vis-z-overlay` for a thing drawn over the panel's content, `--iwac-vis-z-popover` for the fullscreen panel, which must clear the site header. A block needing a fourth layer is describing a fourth role and should name it, not reach for a bigger number. The two `!important` declarations turn out to be one of each. The entity-networks chip dot is the sanctioned case and now says so: `entity-networks.js` writes that colour as an INLINE style because the palette is read from the live theme, and an inline style beats every selector. The person-dashboard one was not needed at all — those selectors are two classes deep and already outrank the single-class `display: flex` that makes a bare `[hidden]` insufficient. C5. `laicite.css` was 1,458 lines, larger than iwac-maplibre.css plus five block sheets together, for a block with fourteen views — so a change to the concordance meant scrolling past the arenas. Five sheets now, in cascade order, named by the registry (`blockCss` already took a list). Verified rule-for-rule: 190 in, 190 out, none lost, none gained. C2, the adopt half. `overscroll-behavior: contain` on all eleven scroll containers: inside an embed the page behind is the HOST's, so reaching the end of a table used to scroll the article the widget sits in. `text-wrap: balance` on both heading levels. Not adopted, per the finding: `@layer`, which would silently invert every module override against the unlayered host theme.
Tier 8 / B2, the four gaps left after eslint shipped in v1.62.0. The registry reader (B2's named example). `check-blocks.js` parsed a PHP array with a regex, and that regex kept being the wrong shape: rows had to be anchored on eight-space indentation so the arrays a `shell` nests would not read as rows of their own. `php -r 'echo json_encode(...::BLOCKS)'` returns the actual array. The regex stays as a fallback so a contributor without PHP still gets the other twenty checks — and because both readers now exist, the guard compares them and fails if they disagree. CI runs the fallback (node only); without the comparison, local and CI would be checking different things. Verified: 21 rows, identical either way. ruff replaces pyflakes, same standard. ruff.toml selects `F` (ruff's pyflakes port) plus `E9` — the previous gate rule for rule, so no Python file had to change. What it buys is that the rule set, the exclusions and the file discovery live in one checked-in config that both `npm run lint:py` and the workflow read. The old explicit `pyflakes scripts/` had already drifted: it linted the generators and never `tests/python/`, which ruff now covers. Everything past `F`/`E9` stays off on purpose — a gate that opens by flagging four hundred line lengths is a gate people learn to skip. stylelint, two rules: `no-duplicate-selectors` and `declaration-no-important`. Five hits, all real, none a bug: three sanctioned `!important`s (the reduced-motion a11y override, and the one that beats an inline style in entity-networks) and two token-only rules deliberately grouped with the arithmetic that reads them. Each is now annotated with its reason rather than the rule being switched off, so the guard still catches a fourth `!important` or a genuine silent override. The entity-networks comment claiming to be "the ONE sanctioned !important in the module" was wrong; fixed. PHPStan level 5, riding on the Omeka checkout `omeka-integration.yml` already downloads and checksums — this module has no composer.json on purpose, so static analysis has to be pointed at a real host. Sixteen errors on the first run; fifteen are Laminas `__call` dispatch, which no analyser can see, and are ignored by NAME rather than blanket, so a typo in one of this module's own methods is still caught. Verified by planting one. The sixteenth is true and stays visible in a comment: every block declares `embeddable => true`, so the whitelist filter is constant. The flag stays — `embeddable()` is also the directory-traversal guard for `common/block-layout/<slug>`, and a security control is not dead code because it currently admits everything. Also: `_h5.txt`, a scratch file from the H5 wave, was tracked at the repo root and shipped inside the release archive. Deleted, and the three new dev configs are export-ignored. php-cs-fixer (B2 (5)) is deliberately not done — it is a formatter, the PHP here is already consistent, and it would trade a large mechanical diff for no defect it can catch that `php -l` and PHPStan do not.
Tier 8 / B3, the three of its five that were still missing. (2) — every bundle a template names resolves — is enforced by check-blocks.js since v1.62.0, in both directions; (5), the source-to-pot-to-po chain, is `lint:i18n-pot`. Flake reporting landed with check-flakes.js. SyncData::perform() against real ZIP fixtures. The suite tested the static predicates — isSafeArchiveEntryPath, isSafeUnixArchiveAttributes, expectedDigestFromSidecar — which is not the same claim as "perform() calls them". A refactor that dropped the zip-slip loop, or moved it after extractTo(), would have left all of those green. So the job now runs for real against archives built at run time by ZipArchive, with a filesystem underneath: happy path, a `../` entry, an absolute path, a symlink, a missing marker, a truncated body, an empty download, a 404 HTML page, the stop switch before the swap, the concurrency lock, and the orphan sweep a SIGKILLed run leaves behind. It asserts on what ends up on disk — that a refused archive publishes nothing, and that a successful one REPLACES the live tree rather than merging into it. `download()` and `verifyDigest()` became protected. That is the whole production change and the reason is in a comment on both: they are the two methods that reach the network, so they are the seam a fixture is handed through. Everything between them and the atomic swap is the shipping code. Checked that the checks can fail: five separate mutants of SyncData — the zip-slip guard, the symlink refusal, the marker check, the first rename, the orphan sweep — each turn the suite red with the message naming the thing that broke. 190 checks to 226. hijri.js: pure, fourteen consumers, no tests, and the one place in the module where being wrong looks right — an engine without the Islamic calendar makes Intl fall back to Gregorian silently, so a chart prints a well-formed date off by 580 years. Two years of days round-trip against an independent Intl reader (731 of them), and a fake Intl that answers in Gregorian is refused rather than trusted. EmbedController::blockAction across its parameter matrix, in omeka_boot.php rather than run.php because params(), layout() and getResponse() are real Laminas plugins reading a real MvcEvent — stubbing them would test the stubs. Six rejected slugs including two traversal attempts, eight ?theme cases, thirteen ?primary cases (the value reaches a style attribute, so a passthrough would be an injection), the :panel passthrough, and every registry block dispatching to a partial that resolves — following the H5 rule, `_generic` for the nineteen that declare a shell. It goes through dispatch() and not a direct blockAction() call: the request and response the controller reads are its own properties, set only by dispatch, so calling the action directly would hand it a blank request and every query-parameter assertion would pass by reading nothing. This part is CI-verified only — it needs a booted Omeka with a database.
Tier 8 / D2, D3, P17 and P9.
D2 — a design record that has drifted is worse than none, because it is
read as authoritative, and this repo has the receipt: CLAUDE.md's token
guidance sat a full redesign behind the code, naming a removed font family
and calling two real published tokens "phantom". So `lint:design-record`
now ties `.impeccable/design.json` and DESIGN.md to what ships: the five
`--iwac-vis-model-*` canonicals against iwac-core.css, their tonal ramps
against their own midpoints, the 64 mirrored theme tokens against
tokens.json, orphans in both directions, and DESIGN.md's slot count. Five
mutations, five different failures — the CSS drifting, a new slot with no
entry, a stale mirror, a ramp that stops running through its own colour,
and prose that disagrees about the count. The prose itself is deliberately
not checked: why slot 4 is a darkened cyan-leaning blue is the part a
record is for, and pinning the numbers is what buys it credibility.
D3 — the two stale claims, corrected in place rather than only in the
v1.63.0 section a thousand lines below. Tier 2 and Tier 5 described
`P.attachGraphClickThrough` as wired; it lost its callers in v1.22 and
v1.63.0 deleted it. Tier 3's "the `+00:00` outlier is gone" was true of
`world_map` and not of the tree — three fan-outs still emitted it. P10
closed those and `validate_data.py` now rejects the form, so that claim is
finally true of everything and enforced rather than asserted.
P17 — `compute_newspapers` and `compute_newspaper_coverage` ran the same
loop over the same two subsets. They had also already drifted: the coverage
one hard-coded `("articles", "publications")` while the other read
`NEWSPAPER_SUBSETS`, so a third subset would have changed one output and
not the other with nothing to say so. One `_scan_newspapers` keyed on
`(name, subset)` — the finer grain, from which the coarser is exact — and
two shapers. 60 comparisons against the pre-change functions on generated
frames covering pipe-separated names, "Unknown" in both cases, NaN names,
missing columns, absent subsets and ties on every sort key: 0 differ, and
3.4x faster on 15k rows.
P9 — and here the audit's premise is wrong, so this does not do what it
asked. "Replace the 55 `iterrows()` sites" is not a win: measured on a
12k x 46 frame, `to_dict("records")` beats `iterrows` only once the loop
body reads enough columns to amortise it —
1 read per row iterrows 0.26s records 0.30s (WORSE)
5 iterrows 0.32s records 0.31s (a wash)
15 iterrows 0.49s records 0.31s (64%)
46 iterrows 1.89s records 0.65s (35%)
So `iwac_utils.iter_records()` went in with that table in its docstring,
and it is used at the eight wide readers only — the dashboard aggregator
(11 columns), the article fan-out (14), the sentiment atlas (10), the
laicite scan (every text field). Four narrow loops were converted and then
converted BACK: `aggregate_prevalence` reads two columns, the index-subset
scans two or three, and there this would have cost more than it saved.
Where a narrow loop is genuinely hot the fix is the column-list `zip` form,
which is what P17 above used.
The other half of P9 was real and is fixed: `index_overview` ran
`index_df["Type"].apply(_entity_type_label)` over the whole index
SEVENTEEN times per build, because three of the five type-filtered views
called it inside a loop over the five types. Computed once and threaded
down as an optional argument — a parameter and not a column on the frame,
since a derived column would ride along into the `.copy()` subsets and out
into whatever they serialise. Every view still works called on its own.
18 comparisons for the iter_records conversions and 60 for the index hoist
(including a non-RangeIndex, where a boolean-mask hoist would go wrong):
0 differ.
Also: the embed dispatch helper added in the last commit needs a fresh
plugin manager per call. `Plugin\Layout` caches the first event it is asked
for and never looks again, and the application's ControllerPluginManager is
shared — so every later dispatch wrote its layout variables onto the first
dispatch's view model, and CI read null for every ?theme and ?primary case.
Tier 8 / H4. Module.php carried four unrelated concerns in 381 lines; it is
250 now, and the two that left are the two with their own subject matter.
Three backed enums, `src/Sentiment/{Polarite,Centralite,Subjectivite}.php`.
The vocabulary was five arrays: three mapping controlled-vocabulary item id
to label, two mapping label back to an ordinal, with nothing but convention
keeping the key sets aligned — a new value added to one and not the other
would have read as a rating of 0 rather than as a mistake. A `match` over
`$this` is exhaustive, so that is now an error at the point of the change.
`Module::getPolariteLabel()` and the four others stay as one-line shims:
`SentimentExtractor` and `article.phtml` call them statically, and threading
a module instance into a view partial to reach an enum is a worse trade than
five shims.
Proved equivalent rather than assumed: a harness parses the five arrays out
of `HEAD:Module.php`, rebuilds each from the enum cases, and compares — then
runs all five static helpers side by side over every item id from 78028 to
78056 plus null/0/-1/999999, and every label plus the empty string, null,
nonsense and a wrong-case one. All equivalent; a one-digit mutation to an
ordinal turns it red. 37 permanent checks went into run.php for the parts
worth keeping: the item ids, the sizes, distinct non-empty labels (they are
the key `ordinalForLabel` resolves by), both scales gapless 1-5, polarity's
"Not applicable" still deliberately off the scale at 0, and every id outside
the vocabulary resolving to null rather than to a rating.
The `@translate` markers moved with the labels. `extract-pot.js` scans
`src/`, so all 132 msgids are unchanged — its own docblock said they were
"declared once in Module.php", which is now the enums.
`src/Mvc/EmbedFramingListener.php` takes the ~100 lines of CSP parsing.
Header rewriting is HTTP plumbing with its own parsing rules, and the only
part of Module.php with pure functions worth testing on their own — which
`run.php` already did, through `Module::relaxFrameAncestorsPolicies()`, so
that name survives as a delegating shim alongside `relaxEmbedFraming()`,
which `onBootstrap` attaches and the integration test calls.
`declare(strict_types=1)` added to the 30 files under `src/` that lacked it
(4 already had it), plus Module.php. PHPStan level 5 is clean after, and
clean at level 6 too.
Tier 8 / S17. `draw()` cleared `viewHost` on every state change, so choosing a different actor type, arena country, sentiment model or reference type tore down a view whose chrome had not changed. Four costs followed, none of them visible in a screenshot: the chart transition was lost (a disposed instance cannot animate into its replacement), `registerChart` re-registered new hosts on every change, the host collapsed to zero height between the clear and the append so the page jumped, and the `role=status` region was re-announced in full. The orchestrator change is six lines: a view whose builder exposes `update(state)` gets repainted in place; one that does not falls through to the rebuild, unchanged. That keeps this incremental — the nine other views behave exactly as before, and gaining `update` later is a per-file change. Four builders gained one, the four the finding names: - actors — the panel chrome and the heatmap HOST are built once; only the option and the ranked list change, so the heatmap animates between filters. A filter matching nothing hides the host rather than removing it, because removing it would mean a new host on the way back. - arenas — only the series data changes; the grid, the axes and the panel count are the same, so the ten small multiples merge rather than snap. The layout arithmetic moved into `layoutOption()`, shared by mount and update. - references — the clearest case of pure waste: the year chart is the whole literature's growth curve and is not filtered at all, so changing the type select disposed and rebuilt a chart the change does not touch. Now only the work list is swapped. - sentiment — all six panels are built once, including the two a model may have no data for; those are hidden with `hidden` rather than omitted, and resized when they reappear, because a chart inside `display: none` measures zero. `notMerge: true` here and not elsewhere: a different model can carry a different set of decades and a different newspaper list, so merging would leave the previous model's categories standing. Five tests hold the contract on a small DOM stub — node identity and which instance received the option are exactly what separates a repaint from a rebuild, and both survive a stub that answers `appendChild`, `replaceChild`, `innerHTML = ''` and `hidden`. Two mutations checked: an `update` that re-registers instead of calling `setOption`, and one that skips the list repaint. Each turns two tests red. The parked map view also benefits — its `update()` now receives the state rather than reading a captured reference, and re-entering it no longer detaches and re-attaches the WebGL canvas.
The embed 404 assertion was testing the framework, not the module. Once blockAction() sets a 404 status, Laminas' own dispatch listeners replace the result with `error/404` before dispatch() returns — which is the right thing to happen in production, and means the returned template says nothing about what the module decided. The six rejected-slug cases now call blockAction() directly and assert on what it chose: the not-found template, marked terminal, with a 404 on the controller's response. A fresh controller per call, because the response is a controller property and a 404 would otherwise leak into the next one. The dispatch() path stays for the parameter matrix, where it is the point. `build-tree.js` read `git ls-files`, which reads the INDEX. So a new file counted only once it had been `git add`ed, and running `npm run build:tree` before staging wrote a tree that `lint:tree` then rejected after staging — which is how the tree guard turned CI red twice in this session while passing locally each time. It reads the working tree now (`--cached --others --exclude-standard`, existing files only), so both orders agree and the local answer matches CI's. Verified: an unstaged new file fails the guard, a gitignored one still does not appear, and `asset/data/` is still represented only by its two committed exceptions.
Ticks the 46 findings waves 7-14 closed, records what landed differently from its write-up, and annotates the six that stay open with why in place — B1 step 2 and the ECharts self-host wait on the owner's ROADMAP 5.4 decision, M20 is an Info "keep it", E12 and E15 were considered and rejected in v1.64.0, S15 was measured and declined, and B5's matrix is superseded by P1's single-process runner. The wave note leads with the three premises that did not survive measurement, because the number is what the next reader needs, not the instruction: P9's iterrows replacement is worse below five column reads per row, S15's ~700 lines of boilerplate is 66 buildPanel calls after v1.63.0 absorbed the rest, and B5's parallelism was hiding a per-process load that no longer happens. Version bumped in all five declarations.
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
1 participant
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.
Continues the Tier 8 roadmap. Opened early so CI can exercise the Omeka integration additions (the embed parameter matrix needs a booted Omeka with a database, which is not reproducible locally). Still adding commits.