feat(analysis): count the CSS, wasm and font bytes a package ships with Lightning CSS - #2
Merged
Merged
Conversation
ehsan18t
force-pushed
the
bundler-b2-asset-counting
branch
2 times, most recently
from
July 18, 2026 18:14
bc7e0f8 to
218ee06
Compare
The design doc says asset counting graduates to its own spec plus plan when built. This is that plan for the bundler-b2-asset-counting branch: the lightningcss pin (=1.0.0-alpha.71, bundler feature, browserslist off), the classify-at-the-boundary then process-post-build then compress-per-artifact-and-sum architecture, the custom SourceProvider that captures @import children for cache freshness, the esbuild oracle re-baseline (sum JS plus the sibling CSS output, add the @uiw/react-md-editor fixture, re-derive the tolerance empirically), the counted per-type breakdown across the protocol and UI, and the six ordered phases with their risks.
B2 folds a package's shipped CSS, wasm, and font bytes into the Import Cost. CSS must be processed the way it ships (resolve `@import`s, minify), which needs Lightning CSS. Pin it exactly at `=1.0.0-alpha.71` (the crate has only ever shipped alphas and the API breaks between them), with `default-features = false` and the `bundler` plus `serde` features (`bundler` carries an ungated `#[serde(untagged)]`, a feature-gating hole in this alpha); `browserslist` stays off so output is deterministic and target-free. Add `daemon/src/pipeline/assets.rs`: a `TrackingProvider` that wraps Lightning CSS's `FileProvider` and records every path it reads (the entry and each resolved `@import` child) for cache freshness, and a `bundle_css` that resolves the `@import` tree into one minified stylesheet and returns the shipped bytes plus the read set. Any bundler failure is an `Err` so the caller can fall back to raw-byte disclosure, never below today. Tests pin the inlining, minification, path capture, and the error path. Lightning CSS is not reachable from rolldown or oxc, so it stays out of the coordinated fingerprint closure; it is a standalone exact-pin, version-tested in compiler-stack-coordination.test.mjs against a single source of truth in compiler-stack.config.mjs, and the CLAUDE.md testing policy is widened to name it. Allow MPL-2.0 in deny.toml: Lightning CSS and its Servo-derived cssparser and parcel_selectors are file-level copyleft, safe to ship as an unmodified library. Adjust four service.rs assertions from a slice-to-array-ref comparison to slice-to-slice: the array-ref form fails integer inference once the wider dependency graph is in scope. The assertions are unchanged; only the literal types are now explicit.
Dropping dependency debuginfo (f4abcda) left the daemon's own behind, and across roughly 36 statically-linked test binaries that is what now dominates peak memory. Adding lightningcss for B2 pushed the pre-commit clippy past the edge on a tight machine: clippy-driver did not report a lint, it CRASHED with STATUS_STACK_BUFFER_OVERRUN, on a different test binary each run, which is the signature of resource exhaustion rather than a real diagnostic. Set the test profile to line-tables-only. That keeps exactly what a failing test needs, the file and line behind every frame of a panic backtrace, and drops the type and variable detail that only a step-through debugger uses, which is not how these tests are diagnosed. The daemon's own dev builds keep full debuginfo, so debugging the binary is unaffected. The hook's clippy and a full test build now both complete at full parallelism, and all 39 test binaries still pass.
A package's real cost is not only its JavaScript. The engine measured the JS chunk and recorded a reachable stylesheet's raw bytes as an `uncounted_asset`, disclosed beside the result but never folded into the Import Cost and never processed as it would actually ship. Every CSS-shipping package, and every package shipping wasm or fonts, therefore reported a headline number materially smaller than what the import really adds to a bundle: a systematic undercount across a whole category of packages (B2). The load boundary now classifies every non-JavaScript module it sees as a stylesheet, wasm, font, or passthrough, and stubs the first three to `ModuleType::Empty` so the JavaScript chunk still measures exactly. Binary assets were previously handed back to Rolldown untouched, where they could perturb or fail the very build whose number must be exact; they are intercepted now, and fingerprinted at read time like any other module they measure. The pipeline then processes each asset the way it really ships. Every reachable stylesheet becomes ONE artifact via Lightning CSS, which resolves the `@import` tree from disk and minifies it: that mirrors how CSS ships (a single file per entry) and how the esbuild oracle emits a single sibling `.css`, and it dedupes a stylesheet two imports both `@import` rather than counting it twice. Several stylesheets are combined behind a synthetic entry that imports each. wasm and fonts have no processor: their shipped size is their raw bytes, compressed. Each artifact is compressed on its own and the sizes are summed, never concatenated first, because they are separate files that ship separately (ADR-0005). Both size paths fold them in: the single import in `analyze.rs` and the per-runtime File Cost loop in `file_size.rs`. Freshness follows the bytes. A stylesheet's `@import` children were never graph modules, so nothing had fingerprinted them and an edit to one would not have invalidated the size it fed; Lightning CSS's `SourceProvider` is wrapped to capture every path it opens, and those reach the fingerprint set. The entry itself is already captured at read time and is filtered out rather than re-hashed after the build, which would widen the window the read-time capture exists to close. Nothing regresses. Any processing failure (a parse error, a dangling `@import`, an unreadable file) falls back to disclosing that asset's raw bytes, which is exactly the behaviour this replaces, so the result is a strict improvement or a tie (ADR-0006). The `uncounted_assets` stage now means "could not be processed" rather than "not counted", and its absence is what will let a CSS-shipping package leave Medium confidence. The CSS test is inverted accordingly: it now measures a CSS-shipping package against an identical one with no stylesheet and requires the first to be strictly larger in raw, minified, and compressed bytes, which is the whole point and is what no test could previously state.
Counting a package's CSS, wasm, and font bytes made the number right, but it also made it opaque: a UI kit's Import Cost is part JavaScript and part stylesheet, and nothing said so. The result now carries an `asset_breakdown`, one row per asset kind present, each already summed across that kind's artifacts. These bytes are ALREADY inside the five sizes; the breakdown says how the number is composed rather than adding to it, which is exactly what the old `uncounted_assets` disclosure was not. It crosses the wire as a flat row of counts (`kind`, then the five byte fields) because the disk cache encoding is positional and a flat row is the shape both sides read most plainly. `engine::AssetKind` gains the serde derives and its snake_case spellings are the contract the extension matches on. The hover renders an "Included assets" section naming each kind at the selected compression, and nothing at all for the common case of an import that ships none. Confidence needed no code: `engine_confidence` returns High only when there are no diagnostics, so a stylesheet that processes cleanly no longer emits one and no longer costs the package its confidence, while one that falls back to disclosure still does. That is the rule the design asked for, satisfied by construction rather than by a special case. The esbuild oracle is re-baselined to match. Under `bundle: true` esbuild gathers a graph's CSS into a sibling `.css` output beside the JS chunk, so `outputFiles` holds more than one entry and the JS is no longer guaranteed at index 0; the harness now classifies by extension and compresses each artifact on its own before summing, mirroring the daemon (ADR-0005). A `reduce` over an empty CSS list is zero, so every existing pure-JS benchmark is unchanged. `@uiw/react-md-editor` joins the real fixtures as the only published package whose ESM entry truly does `import "./index.css"`: it is what makes a missed `@import`, a double count, or an asymmetric inline show up as an oracle delta instead of as a wrong number in the product. The badge fixture flips with it: a real CSS-shipping package must now count its stylesheet and report the contribution, and seeing the disclosure there would mean Lightning CSS could not process a real published stylesheet, which is the one thing that fixture exists to catch. Its Medium row is unchanged and its derivation now says why: it declares side effects, which is what holds it below High, not its stylesheet. `ANALYZER_REVISION` moves to `rolldown-1.1.x+5`, because every CSS-shipping package's number changes and no `+4` entry carries a breakdown. Two names are reconciled with the total-naming guard rather than excused: the oracle's local reducers are renamed off their `sum` spelling, and the second `uncounted_assets_diagnostic` is allowlisted beside its adapter twin, because each names the bytes of ONE package and neither sums across imports.
The oracle only ever summed esbuild's JS chunk, so once the daemon started counting stylesheet bytes (B2) the two sides were measuring different things and the comparison meant nothing. It now classifies outputs by extension and compresses each artifact on its own before summing, mirroring the daemon (ADR-0005). An `outdir` is required for that to work at all, and is not cosmetic: esbuild REFUSES to bundle a CSS-importing graph without an output path, and without one even a pure-JS build names its output `<stdout>` rather than `*.js`, so there is nothing to classify. Nothing is written to disk, so no byte count moves. A scoped package name carries a slash, which turned the generated entry filename into a nested path whose directory does not exist; the name is now sanitized, since only the specifier inside the file matters. Measured result: the CSS benchmark lands at 24.8%, and it is NOT a counting error. The minified totals agree within 1% (1118802 ours vs 1127883 esbuild's), so both sides fold in the same stylesheet exactly once; a double count or a missed import would move that uncompressed number too. Only the compressed figure diverges, because the daemon compresses brotli at quality 4 (it runs per keystroke) while the oracle uses quality 11, an asymmetry that already costs every JavaScript benchmark 2.6 to 15 percent and is amplified on highly-compressible CSS. So the global 25% tolerance stays where it is, because it still has to gate the JavaScript set against a real regression, and the CSS fixture carries its own documented 35% instead. Loosening the shared gate to fit CSS would have hidden exactly the regressions it exists to catch. The derivation comment is rewritten with the measured numbers and the quality asymmetry that explains them.
An adversarial review of the asset-counting change found a way to kill the daemon outright, and it was reachable by hovering an import. Lightning CSS detects an `@import` cycle by the path spelling the provider hands back, and the built-in resolver is a naive `with_file_name` join that never normalizes `..`. A cycle crossing a `../` therefore produced a LONGER, DISTINCT key for the same file on every hop, the dedup never fired, and the recursion overflowed the stack. That is not a failure the daemon can contain: a stack overflow is not an unwind, so `catch_unwind` never runs and the process dies with every in-flight request. `node_modules` is untrusted input and an `@import` cycle is silent in browsers and in every real bundler, because they dedupe on a resolved URL, so a package can ship one and never know. The multi-stylesheet path was the exposed one, which is the common UI-kit case. A single canonical entry was accidentally immune: `\\?\` is verbatim and Rust normalizes `..` eagerly on verbatim paths. The synthetic entry's blanket backslash-to-slash rewrite destroyed exactly that protection, because `//?/C:` is not a verbatim prefix. That rewrite is gone: paths are now escaped for CSS instead, which also closes an injection a package could have shipped through a filename containing a quote, and stops corrupting a legitimate POSIX path that contains a literal backslash. `resolve` now canonicalizes what it returns, so the bundler's cycle key is an identity rather than a spelling, and the cycle terminates. A regression test pins it: if that test ever aborts the runner instead of failing, the wedge is back. Assets are never graph modules, so none of the engine's limits ever applied to them and a deep-but-honest `@import` chain could still overflow the stack. The provider now bounds one tree to 64 files and 8 MB. 64 is far below where any build's stack gives out, not just release's (a debug build's frames are several times larger, so a budget tuned to release would let the test suite overflow), and far above any real stylesheet's `@import` tree. Two more findings from the same review, both about the blast radius of a failure rather than a wrong number. A remote `@import`, which ordinary packages ship, was a resolve error that sank every stylesheet in the set to raw disclosure; it is external now, exactly as a real bundler treats it, and the rest of the sheet still counts. And a set that fails as a unit now retries per sheet, so one package's `.scss` or unresolvable import no longer reverts CSS counting for every other package in the runtime group. The freshness filter compared a raw canonical `PathBuf` against a forward-slashed fingerprint path, so it never matched and every stylesheet was re-hashed after the build anyway. It compares the same spelling now. The protection was never lost (dedup ordering elsewhere kept the read-time fingerprint), but the comment credited a filter that did nothing. The flagship CSS test declared `sideEffects: false`, which is the ONE shape where a bundler DROPS the stylesheet and this feature's number is wrong, so its green was accidental: it pinned the defect. It now declares `sideEffects: ["*.css"]`, which is what a real CSS-shipping package declares and what makes the bundler retain the stylesheet that is being counted. A drift check binds the asset-kind vocabulary across the wire. The daemon's enum and the extension's union are separately maintained, so a variant added to one side alone compiles cleanly and renders `- undefined` in the hover while the total stays right; the expectation is derived from the daemon's enum rather than typed out.
B2 (non-JS asset counting) has landed, which empties Priority 0: every release blocker found by the module audit is now fixed. Its entry moves to Resolved with what was verified against the esbuild oracle, namely that the minified totals agree within 1% on the one real package whose published ESM entry imports CSS, so both sides fold in the same stylesheet exactly once. Four limits the work leaves behind are recorded rather than forgotten. D7: a stylesheet its own package declares droppable (bare CSS import plus sideEffects false) is counted anyway, which is a wrong number on a shape measured to be absent from the ecosystem (zero of 503 store packages, zero of 44 real CSS shippers), and whose obvious fix would invert into a far worse under-count that undoes the feature. D8: a stylesheet Lightning CSS cannot parse falls back alone to raw disclosure, and one caught in an @import cycle undercounts itself, both never below the pre-B2 floor. D9: an @import tree is bounded at 64 files, where nothing bounded it before and a deep chain could kill the process. D10: every reported brotli size runs high because the daemon compresses at quality 4 while the web serves 11, which predates B2 and is what the oracle re-baseline surfaced. The plan's claim that the provider falls back to oxc_resolver for a bare @import described work that was never built; the plan is corrected rather than the gap papered over.
Two findings from re-reviewing the previous fix, which is exactly why the review loop runs on the fixes too and not only on the original change. When the combined bundle AND every per-sheet retry failed, each stylesheet was disclosed TWICE: the retry reported each failure as it went and then handed back an error, so the outer fallback disclosed them all again. The diagnostic that exists to be honest about what is missing doubled its own count and byte total, reporting four assets and 20000 bytes where the truth was two and 10000. The per-sheet results are now gathered locally and committed once, so the shape cannot recur, and the all-fail path is once again byte-for-byte the pre-B2 fallback. A test pins it; the previous test only covered one-bad-one-good, which takes the success path and never reached the double push. The tree budget was justified against recursion depth but charged on breadth, so a flat set of stylesheets carrying no stack risk at all could trip it. That matters because the File Cost's combined build hands over every stylesheet in a runtime group at once: an ordinary multi-package file would have degraded into the per-sheet path, where sheets sharing an @import are counted once each, which is a number too HIGH rather than merely incomplete. The bound is 256 now, sized against the release build the daemon actually ships, where a chain of N files costs N reads and 256 stops the walk roughly three times short of where the stack gives out. Giving the walk its own large stack was tried first and abandoned on evidence: Lightning CSS drives the @import graph on rayon workers, whose stacks it does not own, so the recursion never ran on the thread the stack was given to. The file count doubling as the depth bound is the design that can actually be verified, and the tests are shaped to match it: breadth proves the bound fires, and a shallow chain proves it does not refuse the real.
The Windows daemon binary was rebuilt to include B2, so its content hash moved. The extension verifies the daemon against this generated table before spawning it, so the table must carry the new hash or the shipped daemon would be rejected. The VSIX grows from 5.49 MB to 6.52 MB, which is Lightning CSS and its CSS parser; the packaged size assertion still passes. Only the win32-x64 entry is refreshed: pnpm package:win32-x64 builds the Windows binary alone, and Windows is the primary supported platform. The linux and darwin entries are refreshed by their own platform builds in the release pipeline.
A branch-wide review found a size that could read high while claiming High confidence, and be cached that way. A package's stylesheets are bundled into one artifact precisely so that an @import two sheets share is counted once. That union is all-or-nothing, so when it fails the set is retried a sheet at a time. Every sheet still counts, and the shared bytes are then inlined into each of them, so the number reads high. The over-count is the accepted cost of degrading. Being silent about it was not. It was silent because the degradation had nowhere to be said. The per-asset failure list was only ever read as the DETAIL of the uncounted-assets disclosure, and that disclosure returns early when nothing is uncounted, which is exactly what the degraded path produces: every sheet counted, nothing uncounted. So the warning the code wrote for itself was dropped on the floor. With no diagnostic, engine_confidence reads High, is_durable passes, and the inflated number goes to disk and is served until the analyzer revision moves. The union's usual reason to fail is the whole set breaching the 256-file budget that each sheet on its own is well inside, and the File Cost bundles every import in a runtime group as one set, so this is reachable rather than theoretical. The fix gives the degradation its own state and its own disclosure (imprecise_assets) rather than making it share a channel with a different fact: bytes present but over-counted is not bytes missing. Both call sites now take every disclosure the processed assets owe, so folding in the bytes and forgetting one is no longer possible. A regression test pins it, and it fails without the fix. Three comments and D9 already claimed this path was disclosed; that claim is now true.
The one benchmark that compares asset counting against an independent bundler could not fail on any CSS counting error. It gated the BROTLI delta, and on this fixture the stylesheet is about 1.7% of the compressed total while the daemon-vs-oracle compressor gap (quality 4 against quality 11, D10) is about 25%: a 14:1 noise-to-signal ratio. Measured against the real numbers, folding the stylesheet zero times reads 24.2%, once 24.8%, twice 25.5%, and all three pass the 35% tolerance. This fixture would have stayed green if asset counting were deleted outright. The tolerance comment claimed 35% was far below what a double count would produce. That was arithmetically false, and it is why this change exists rather than a tolerance tweak. The evidence was there and simply never asserted: the MINIFIED totals agree within 0.81%, which is real proof both sides fold the sheet in exactly once, because neither side compresses that number so the compressor gap is absent from it. It was printed and never checked. The comment even said so, that it was checkable rather than asserted. So the minified pair is now gated at 1.5%, which sits between the honest 0.81% reading and the 2.19% a double count produces: nearly 2x room above the truth, a third below the nearest failure. 2% would also catch a double count, but by under 10%, thin enough for a small drift to hide one. Both preconditions the plan asked for are here too: the fixture's ESM entry must still import a stylesheet, and the oracle must actually emit one, so the benchmark cannot quietly decay into a second JavaScript one. Each new check was verified by making it fail on purpose.
The size block lists all five figures (raw, minified, gzip, brotli, zstd) and then printed an unlabelled asset row under them, so "CSS: 12.3 kB" invited being read as the raw number when it is whichever compression is selected. The heading now names it. The test fixture behind those rows was also physically impossible: it declared 4,700 B of brotli assets inside a 1,500 B brotli total, and 5,000 B of minified CSS inside a 4,600 B minified total, under a comment stating the bytes are already IN the number above. The renderer echoes what it is given, so the test was not wrong, but its data taught the exact opposite of the invariant it documents. Every kind now fits inside the totals on all five axes, with a font that barely compresses and CSS that compresses well, which is how they really behave.
Two new accepted limits, both found by the review and both deliberately not fixed inside this work. D11: an asset the daemon could not read is disclosed and that disclosure is cached, so a transient read error (an antivirus holding the file, a concurrent install) outlives its cause until the analyzer revision moves. It is disclosed and never below the pre-B2 floor; telling a transient error from a permanent one means classifying the io::ErrorKind at the read site, which is worth doing on its own. D12: a File Cost total that omits an unreadable asset is not flagged as a floor. That is a tie with the old behaviour rather than a regression, since before B2 every stylesheet's bytes vanished from a total that was equally cacheable, and raising the flag would newly refuse to cache any file importing one unparseable .scss. D9 now says the per-sheet degradation is disclosed as imprecise_assets, which is true as of this branch rather than aspirational, and records that the byte budget is charged after each read, so it bounds a tree's total rather than any single file's peak. The plan presented an oxc_resolver fallback for a bare @import as its design while its own risks section correctly recorded that it was never built. The body now matches the code and points at D8.
A second review round measured the degraded path and found the disclosure blames the smaller of the two causes. The union does two things: it dedupes an @import two sheets share, and it puts the whole set through ONE compression stream. The warning named only the first. The second dominates. Three hundred tiny stylesheets sharing no @import at all, which is the shape that actually breaches a 256 file bound, sum to roughly 40x the union's gzip and 57x its brotli, because every stream restarts its window and pays its own header. Real stylesheets are larger and fewer, so the real factor is far smaller, but the direction holds and it is not a small correction. Saying "any bytes they share are counted once per sheet" invited reading a 40x figure as a rounding error. This also answers why the disclosure fires on the union having failed rather than on the sheets provably sharing bytes, which looks like the tighter test and is not: sheets that share nothing are the WORST case here, not the safe one, so suppressing the warning for them would stamp the most inflated number this path can produce as High confidence and cache it. D9 said the same thing the message did and is corrected with it.
The daemon's asset disclosure text changed, so the shipped binary changed. The extension verifies the daemon it launches against this hash, so it has to track the bytes it is now packaged with.
Capture the verified correctness, cache, resource, testing, and documentation gaps in the asset-counting branch before remediation begins. The audit distinguishes new release blockers from already accepted limitations and records the targeted verification behind each conclusion.
Discover supported font and wasm artifacts from Lightning CSS dependency metadata, resolve them from the stylesheet that owns each URL, and fold each deduplicated emitted file into Import Cost and File Cost. External, ambiguous, missing, and remote-stylesheet references now disclose the measurement boundary instead of silently preserving High confidence. Add public behavior coverage for nested, shared, missing, external, and ambiguous references plus an independent esbuild fixture that catches both omission and double counting.
Retain CSS, wasm, and font inputs as immutable byte snapshots carrying the read-time fingerprint that describes those exact bytes. CSS child and resource observations now survive success, failure, union retries, and same-path races; missing or conflicting inputs cannot enter durable caches. Validate the combined File Cost against its own exact dependency fingerprints, include asset paths in dependency indexing, and bump the analyzer revision so pre-fix cache entries are rejected.
Apply aggregate source limits to direct and CSS-referenced assets, then isolate post-build processing behind a two-wide deadline-aware boundary so asset work cannot overwhelm the daemon.
Keep exact dependency fingerprints as a daemon-private cache admission check while coordinating only wire-visible File Cost quality fields. Rename asset budget counters so resource accounting cannot be mistaken for a user-facing total.
Classify asset I/O and compressor failures as non-durable partial results, preserve their causes across retries, and reject them from caches, histories, reports, and budget verdicts. Coordinate daemon, extension, and CLI durability policies while preserving configured asset resolution.
Propagate deterministic asset omissions through successful combined builds and per-import fallbacks so partial totals cannot enter the File Cost cache, history, or budget gates. Preserve imprecise over-counts as a separate quality state and version bundle-impact history past pre-fix floors.
Keep deterministic imprecise asset measurements cacheable, but prevent editor, report, and CLI budget verdicts from treating their disclosed upper bounds as exact.
Update CLAUDE.md to make `lean-orchestration` the default execution mode for non-trivial tasks, including where to find setup details. Remove the old `hybrid-execution` default guidance and keep the implementation workflow focused on reproducing reviewer/subagent findings before applying fixes.
Records the two-axis review of the asset-counting design and implementation: five review passes plus three adversarial verification passes over the branch diff. Two defects were proven to produce a wrong number the user is never told about, and two design findings were adjudicated false and are kept in the report marked as such rather than dropped. Also tracks the refactor plan measuring the feature's line-count growth and its deletion targets. The review adds one warning to it: its behavior freeze would pin the two proven defects as correct, so they must be fixed before that freeze is taken.
A build that both breaches a graph limit and fails to read an optional asset reported the transient asset stage, because that arm sat above the breach check - contradicting the ordering comment in the same file, which states that a breach preempts every diagnostic below it. Two things went wrong. The user was told to retry after the filesystem settles for what is a permanent property of the package, and because asset_io is absent from DURABLE_RESULT_STAGES while module_graph_limit is in it, the failure was refused by every durable store, so an oversized graph was rebuilt on every request instead of once. Guarded in both directions: a breach now wins when both are recorded, and the asset stage still wins when no breach was, so the guard cannot be satisfied by never reporting asset_io at all.
A stylesheet's url() graph was classified by whether an extension was countable, so everything else left through a single silent None arm. A shipped image or SVG referenced from counted CSS was dropped: not counted, not disclosed, and still High confidence - a headline advertising itself as the import's full cost while short by an undisclosed amount. It is now disclosed at its real size under uncounted_assets, which restores incomplete and with it the cache and budget gates behind it. The same channel also conflated two opposite facts. An omission - a public-root resource, an unreadable file, or a sheet whose URLs lightningcss could not inspect - was reported as imprecise_assets, the stage that means the number reads high. Pointing a missing-bytes disclosure at an over-count stage stopped incomplete from firing, so a File Cost short by real shipped bytes passed every durability gate and was written to the no-TTL history as that file's permanent baseline. Omissions now report uncounted_assets. A runtime-fetched resource is the third case and is neither of those: a CDN font is real weight for the page but not a byte this package ships, so the measured size is exact. It is now disclosed on external, which is durable and budgetable, rather than on a precision stage that silently removed the budget verdict from every package that imports a web font. A resolvable path that cannot be stat'd stays an unreadable failure instead of becoming a pathless omission, so it keeps its never-fresh evidence and supplying the missing file still invalidates the result. ANALYZER_REVISION moves to rolldown-1.1.x+9: cached +8 entries can be short by an undisclosed image at High confidence, or carry an omission mislabelled as imprecision. FR-018a is rewritten - it still described assets as disclosed and never counted - and FR-018b is added for the url() classification rules. The wire model documents asset_breakdown, the README explains that the number is not only JavaScript, the design doc is marked implemented, and D13-D17 record the scope decisions and the findings left unfixed.
The asset-classification and failure-ordering fixes change the daemon binary, so the integrity hash the extension verifies before spawning it has to move with it. Generated by pnpm package:win32-x64.
Three defects with one root: the asset pipeline ran beside the models that describe a result instead of inside them, so the things those models exist for could not see asset bytes. FRESHNESS. A module whose bytes reach the number but whose read was never fingerprinted — a binary module Rolldown loads through its own loader — was deferred to a post-analysis stat. That stat runs after the measurement, so a module rewritten inside the analysis window pairs a size from the old bytes with a hash from the new ones; the pair is self-consistent, so every later probe answers Fresh and serves the stale size until the file changes again. It is now unverifiable, which can never be fresh, matching what the File Cost path and the full-package memo already did. A manifest keeps its post-analysis stat deliberately — it is a resolution input, not bytes inside the chunk — and so does the failure path, where there is no size for a fingerprint to contradict and making it never-fresh would rebuild a broken package on every request. SHARING AND COMPOSITION. A stylesheet links as an empty module, so its rendered length is zero and the JavaScript contribution walk dropped it. That made a CSS-dominant package's top-module list sum to a fraction of the headline printed beside it, and made a stylesheet several imports pull invisible to shared_bytes — the one mechanism that explains a gap between Combined Import Cost and File Cost. Assets are module contributions now. No wire change was needed: the rows ride in internal_contributions, which the L2 envelope already carries for exactly this reason, so a cached result shares identically to a fresh one. FILE-LEVEL COMPOSITION. The file total began including stylesheet, wasm and font weight with no surface able to say so. FileSizeDocumentResponse now carries asset_breakdown, summed per kind across runtime groups, with a degraded group's per-import fallback contributing its rows too — so the number says what it is made of exactly when it is hardest to read. Also here: the per-sheet retry no longer clones the whole snapshot ledger per attempt (quadratic in the degraded case it exists to serve) and looks up one path at a time; record_limit keeps the first breach rather than the lexicographically smallest message; and the uncounted-asset sentence is written once for both producers, qualifying its total rather than reporting an unstattable asset as 0 bytes. ANALYZER_REVISION moves to rolldown-1.1.x+10: a +9 entry can pair a size with a later hash, and computed shared_bytes without asset weight.
The daemon added a whole diagnostic stage in order to say that a number is a disclosed upper bound, the CLI printed the sentence, and the editor rendered a bare File Cost while its budget silently returned not-evaluated — one figure named as sound by one surface and refused by another on the same run. fileCostQuality gains a third axis. It is not a flavour of short: short means bytes are missing and the number is a floor, while imprecise means every byte is present and some are counted twice. Folding one into the other would tell the user the true cost is higher when it is lower. The status bar and Show Current File Size now say File Cost upper bound and give the reason, and the summary line names what share of the number is not JavaScript, quoted in the same compression as the headline it sits beside. The property test behind this moves from durability to BUDGETABILITY, which is what its own doc comment always described. The two predicates agree on every input it tried and disagree on exactly one it did not: a deterministic over-count is worth caching but cannot pass or fail a threshold. That single gap is how an upper bound came to be named a plain File Cost. An unknown asset kind from a newer daemon now renders its wire name instead of the literal string undefined, and the label lives in one place both surfaces read. The CLI was also a protocol version behind. The daemon accepts a range, so the mismatch never failed loudly — it just negotiated an older protocol and silently dropped whatever the newer one added. A drift check now binds all three declarations to the daemon's.
FR-018c states that asset weight must be visible wherever the number containing it is: both breakdowns, assets as module contributions, and an unknown kind rendering its wire name. The wire model documents the file-level asset_breakdown. FR-032a settles the contradiction between its own headline and its per-import enumeration, and the enumeration wins. A floor F is at most the true cost T, so a violation fires only when F exceeds the limit, which forces T to exceed it — every violation reported from a floor is TRUE, and the only possible error is silence. Refusing to judge a floor would therefore delete true violations and prevent none. An upper bound is the reverse and must never be judged. The requirement now says so, and names the workspace report's budgetViolationCount as the one counter that may under-count and must never be read as proof a workspace is inside budget. known-issues records D15, D16 and D17 as resolved with what actually changed, and adds D18 and D19 for the two things deliberately left alone: a CSS url() may resolve outside its package root, because containment is an invariant this tool holds nowhere else and enforcing it here would stop counting bytes that genuinely ship; and the per-dependency stat loop is already bounded by the stage deadline, so a second accounting mechanism for zero-byte operations would be more machinery than the risk justifies.
The coalescing test slept 50ms to let the follower register as a joiner before releasing the leader. Under load that was not enough: the leader published and removed the flight first, so the follower arrived to an empty registry, became a leader itself, and computed a second time. Measured on an unmodified tree, it failed roughly one run in three. It now waits for the flight's reference count to show the follower has actually claimed it — the map holds one and the leader holds one, so a third means the follower is about to block. The assertion that gates coalescing is unchanged; only the precondition it depends on is now observed rather than guessed. Unrelated to the asset work in the surrounding commits, and separated from it deliberately. A flaky pre-push gate is worse than no gate, because it teaches people to re-run rather than to read.
A directly imported image, icon or media file was a fatal leaf. A PNG is not UTF-8, so Rolldown's loader failed with UNLOADABLE_DEPENDENCY; an SVG *is* valid UTF-8, so it reached OXC and died with PARSE_ERROR: Unexpected JSX expression. Either one made the entire package unmeasurable — the user saw "unavailable" for a package whose JavaScript measures perfectly. Those extensions now classify as AssetClass::Unmeasured and are stubbed to ModuleType::Empty like any other asset. Rolldown shims the bindings for an Empty module, so a default import still links. The bytes are DISCLOSED at their real size rather than counted: they ship, so a size that omits them is a floor and says so, which drops the result to Medium and marks the file total incomplete. The interception list is an allowlist of extensions a bundler's file loader really emits. An unknown extension still falls through to Rolldown, because stubbing something we cannot name would stub a module that might have been real JavaScript. Verified against Rolldown's own loader table and the daemon's resolver extensions: the intersection is empty. The byte accounting mirrors the counted arm exactly — reserve from metadata before the read, post-read growth check, reconcile after, fingerprint before any deterministic failure returns, and release on a duplicate hook invocation so two module ids canonicalizing to one path cannot drift the aggregate counter upward. ANALYZER_REVISION moves to rolldown-1.1.x+11: a +10 entry for such a package is a cached DURABLE failure and would keep being served as unavailable for a package that now has a number.
FileProvider::resolve is infallible — a naive with_file_name join — so a bare @import "pkg/base.css" silently became <dir-of-sheet>/pkg/base.css and came back as a valid-looking file. The failure surfaced at the READ instead, which recorded a failed read of a path that does not and will never exist. Two consequences reached past the one sheet: the request-local asset_io stage was stamped on the whole asset result, and a never-fresh sentinel was written for the phantom path. A package shipping one bare @import could therefore never be cached, and was re-measured on every keystroke over a file nobody will ever create. The resolve now declines such a specifier, so the sheet lands on the deterministic per-sheet fallback and the result stays reusable. Resolving it properly is still not done, and is not merely plumbing: the JavaScript resolver profile has no style main field, no style condition and no .css extension, so pointing it at pkg/base would answer pkg/base.js and measure the wrong file. The ORDER of the check is load-bearing and the first attempt had it wrong. Deciding by spelling alone rejects @import "theme.css" and @import "sub/dir.css" — both ordinary relative URLs, because CSS has no bare-specifier concept and a plain string is a relative URL. That would have dropped a very common shape to raw disclosure. Existence beside the sheet decides; spelling only decides what happens once nothing is there. A missing RELATIVE import keeps its failed read deliberately, since the sentinel on its real path is what makes creating the file invalidate the result. Also here: a property test for the asset pool's admission width under nested rayon, which retires a standing concurrency guess with an observation instead of an argument.
AC-04 recorded that the binary shipping model was asserted, never measured: a directly imported wasm or font is stubbed to ModuleType::Empty, which removes the runtime reference code a real file-loader build emits, and no fixture compared that model to esbuild. The existing asset fixture reaches a font indirectly through a CSS url(), so the direct-import shape was untested. It agrees. On a fixture importing both a wasm and a font directly, we read 16435 B against esbuild's 16480 B minified — 45 B, 0.27%. The cause is exactly the predicted one and was confirmed by dumping esbuild's chunk rather than inferring it: its JS output is 96 B of URL references where our stub is 51 B. The binaries themselves are counted identically by both sides. esbuild has no default loader for .wasm, so the import form is a hard error there without one; the loader config now covers it. expectedAsset became a list so one fixture can assert both artifacts, and the daemon-side check moved from find to filter with an exact-count assertion, which also rejects a duplicate contribution it would previously have accepted. The fixture needed its own byte generator. deterministicBytes builds its stream from the low bits of an LCG, whose period there is 256, so 16 KB of it compressed 22:1 — a binary fixture built on it would have measured a compressible synthetic rather than a shipped binary. The new generator compresses to length+4 at both quality 4 and quality 11, which is why this benchmark's brotli axis can gate honestly where the CSS one cannot.
FR-018a gains the rule that a shipped file is intercepted even when it is not counted, with the reason: left to Rolldown one such import makes the whole package unmeasurable, and the interception list is an allowlist because stubbing an unnamed extension might stub real JavaScript. D6's asset half is marked closed, D21 records the bare-@import freshness fix including the relative-URL ordering mistake that nearly shipped, and D20 retires the nested-rayon guess with the property test that answers it. Two items are declined with evidence rather than left open. D9's survivor-union would trade a disclosed over-count for a possible terminal AssetBudgetFailure, because the CSS work ledger is monotonic and the common union failure is the set breaching together — so the survivors are all the sheets and re-unioning breaches again. D7's blocker was recorded as one missing piece and is actually three: no importer in the load hook, sideEffects patterns collapsed to a bool at parse time, and a contract stating that reading is reporting metadata that decides a badge, never a byte. An earlier note claiming D15 unblocked it was wrong. D10 is measured rather than argued, on the real 139 KB minified bundle instead of a synthetic that compresses to nothing. Quality 11 costs 35x the time and nearly a second per artifact, which closes the just-raise-it option. Quality 9 removes half the overstatement for +33 ms, which is the trade actually on the table and is a latency decision rather than a defect.
The bare-@import change in 284f040 is reverted. Adversarial verification against the running service found it made a missing target deterministic and therefore CACHEABLE: an @import of a file that did not exist yet measured 0 bytes, was served from cache on repeat, and still served 0 bytes after the file was created. The ./-prefixed spelling recovered correctly, so the invariant held for one spelling and broke for the other, pinning a package's CSS at zero indefinitely. The premise was also wrong. CSS has no bare specifiers, so pkg/base.css is an ordinary relative URL and <dir-of-sheet>/pkg/base.css is exactly where that file would live. It is not a phantom, and recording it is correct: creating it later SHOULD invalidate the result. What remains true is the original complaint, now stated accurately in D21: a read that fails with NotFound is a deterministic fact about the package, but it stamps the request-local asset_io stage, so the whole result is refused by every durable store and re-measured on every keystroke. The fix is to split that classification — NotFound keeps its never-fresh path evidence without the transient stage, while permission and lock failures keep it — which preserves invalidation for every spelling. failed_paths drives both today, so they cannot be told apart. The test pinning unprefixed relative imports as counted is kept; it is still true, and it is what caught the earlier spelling-only draft.
Undoing a shipped change had no type of its own, so it had to be labelled fix — which hides the one thing a reader scanning history most needs to see, that something was taken back. The passthrough rule only covers git's own machine-generated Revert "..." subject, which carries no scope and no explanation of why the revert was right. It joins the changelog with its own group rather than being skipped, because a revert is user-visible: something that shipped no longer behaves that way, and that belongs in the changelog as plainly as the change that introduced it. The existing drift test between COMMIT_TYPES and cliff.toml already covers the pairing, and was confirmed to fail when only one of the two is updated.
The image-interception and freshness changes move the daemon binary, so the integrity hash the extension verifies before spawning it moves with it. Generated by pnpm package:win32-x64.
The file said outright that a Resolved entry is "kept for history", which guaranteed it would grow forever. It had reached 1,066 lines, of which 222 described things that are no longer wrong with the product — burying the entries that still are.
A resolved entry now collapses to one row in a Resolved index: identifier, one line, date. The identifier has to survive because roughly a hundred code comments tag their reasoning with one ("(B2)", "before D11"), and a reference that resolves to nothing is worse than the bloat. Everything else lives where it belongs — the behaviour in the SRS, the reasoning in the commit, the guarantee in the test.
The policy section now says this, since the old wording is what produced the growth. A decision NOT to fix is explicitly not a resolution: Accepted, Deferred, and documented declines stay in full, because they are still true of the product and the declines exist to stop a dangerous change being attempted twice.
1,066 lines to 873, with no entry lost that describes present behaviour.
The plan froze behavior at 8fe2342, but nine commits landed after it and several changed behavior deliberately: a directly imported image is intercepted and disclosed rather than failing the build, assets are module contributions, the file total carries an asset_breakdown, fileCostQuality gained an imprecise axis, and a measured-but-unfingerprinted module is unverifiable. Refactoring against the old freeze would have preserved behavior the product no longer has. Seven additions to the frozen list record what must now survive, and the baselines are corrected: the cluster is 4,088 lines rather than 3,680, of which 2,841 are implementation. The plan also permits deleting tests, which means a green suite cannot prove preservation — a deleted test passes too. The 646 Rust test names at the freeze point are captured, and every phase must account for each one that disappears by naming the row that replaces it.
Nearly every asset test repeated the same six lines: make a temp dir, write each file with its own expect, then remove the dir at the end. That trailing cleanup only ran when the test PASSED, so a failing assertion leaked the directory — proven by injecting a failure and watching it survive, where the fixture now removes it on drop. One Fixture absorbs all of it, including the nested-path cases that needed their own create_dir_all. 42 scaffolding expects are gone and nine doc comments are compressed to the defect each one records rather than the essay around it. Two tests are removed and both are accounted for. bundle_css_is_an_err_on_a_broken_stylesheet asserted only that a dangling @import errors; process_assets_falls_back_to_raw_disclosure feeds the same input through the caller seam and asserts strictly more, including that contributions are empty, which is reachable only if the bundle really errored. Its load-bearing sentence moved there. no_more_jobs_execute_at_once_than_there_are_permits was mine and was the weaker of a pair: asset_processing_never_exceeds_two_concurrent_jobs asserts an exact peak of two AND that all six queued jobs complete, so the nested-rayon work moved into it and the duplicate went. Verified by multiset diff of every assertion rather than by reading: 43 removed, of which 42 are scaffolding expects and one belonged to the deleted test; 2 added, both the fixture's own. No surviving predicate, value, or message changed. AssetKind::ALL and as_str are also deleted, having had no callers.
The provider had four constructors — bounded and unbounded, each with and without a synthetic entry — and the ledger was an Option threaded through a dozen signatures. The unbounded pair existed so processor tests could skip the ledger, which meant the tests measured through code production never runs, and made production safety something a caller could simply omit. There is now one constructor and a mandatory context. bundle_css, bundle_css_set and the unbounded process_assets are test-only and build a REAL ledger with production limits; the freshness integration test moved onto process_assets_bounded, the actual production entry. Roughly a dozen if-let-Some guards collapse to direct calls, and the dead no-context fingerprint fallback goes with them. One test had to be told which limit it is exercising, and the reason is worth recording. The per-sheet degradation fixture reads ~280 stylesheets, so the union breaches the 256-file per-attempt bound and the retry reads all of them again — 560 against a build-wide ledger of 512. Union(N) + retry(N) = 2N, and the path needs N > 256 while 2N <= 512, so any N that triggers the degradation also exhausts the ledger. That is real production behaviour, and the test had been passing only because it bypassed the ledger entirely. It now runs with the build-wide limit lifted and the per-attempt bound intact, which is the thing it was written to check. Test inventory is unchanged from the freeze apart from the two removals already accounted for.
The asset feature reached ~9,000 lines before anyone counted, and no single change was unreasonable — the growth came from adding beside instead of replacing. These rules name that specific failure rather than restating a preference for small code, and each has a trigger that can be checked in review. The shapes are the ones that actually produced the bloat here: a second mechanism for a concern the codebase already has (four provider constructors, two read ledgers, an Option that made production safety bypassable), speculative surface with no callers, a test-only path that doubled the code and hid a real production limit while doing it, and correlated state that three functions each re-read. Two carve-outs keep the rules from being turned against the product. Performance is measured, not asserted — the brotli assumption was wrong by 35x and measuring took ten minutes. And correctness outranks size: never trade a disclosed-correct number, a freshness guarantee, or a gate for fewer lines, because shorter code that is harder to verify has moved the review cost rather than removed it.
Two number-moving changes that share ONE analyzer revision, so they ship as one commit. Splitting them left a point in history where the daemon compressed at quality 9 while still stamping revision +11 — q9 and q4 results sharing a cache key, which is exactly the incomparability the revision exists to prevent. Adversarial verification caught that; the commits are regrouped rather than the messages patched. BROTLI. Every reported brotli figure was high, because the daemon compressed at quality 4 while the web serves 11. Measured on the real 139 kB minified bundle: q4 39,671 B in 27 ms, q9 36,775 B in 60 ms, q11 34,196 B in 955 ms. Quality 11 is unavailable to a compressor that runs per keystroke at 35x the time; quality 9 halves the overstatement, 16.0% down to 7.5%, for +33 ms. The oracle confirms it — refractor was the documented worst at 15.0% and now reads 7.8%, the CSS benchmark went 24.8% to 8.8%. That retires the fixture-specific 35% tolerance, which existed only because our compressor was weaker than the oracle's and gated nothing: measured by varying the fold count, the fixture stayed green with asset counting deleted outright. MISSING ASSETS. A stylesheet whose @import target does not exist recorded a failed read, stamping the request-local asset_io stage on the whole asset result, so a deterministic package fact was refused by every durable store and re-measured on every keystroke. Read failures now carry their ErrorKind in one typed collection rather than two parallel vectors, and asset_io is derived from transient failures only. Freshness needed the second half or the fix would have changed nothing: a missing path took the unverifiable sentinel, which can never be fresh. It now takes an ABSENT fingerprint, fresh for exactly as long as the file stays missing and stale the moment it appears. Verified under a default-suspect burden: an absent sentinel returns Fresh only while metadata is NotFound, any real fingerprint for that same file returns Gone in that state, and Gone dominates — so a run that saw one path in two states is refused on read regardless of which marker recorded it. ANALYZER_REVISION moves to rolldown-1.1.x+12 for both.
D10 and D21 are fixed, so their entries are deleted and collapse to one row each in the Resolved index, per the policy this file now states. D7 moves from Deferred to Accepted, which is what it always was. Deferred says worth doing, not now; this is not queued work. Its own recorded honest fix — ask the sideEffects DECLARATION rather than the build — is the one thing FR-021 and the engine boundary contract exist to forbid, since both say that reading decides a badge and never a byte. Closing it means amending a Critical requirement, not writing code. Re-verified today: rolldown's load hook carries no importer at all and resolve_id discards the one it has, and the sideEffects patterns are collapsed to a bool deliberately, with the code saying in prose that keeping them invited exactly the second reading D7 would need. D6 is re-scoped to ENGINE. Its asset leaf is closed; what remains is a .node addon and a deep-path require that cannot resolve, which are resolver concerns that happen to share the entry because a shipped asset used to be a third trigger for the same symptom. The triage rule in CLAUDE.md is scoped so it governs findings the agent surfaces and has no authority over work I asked for. Parking a request in a document is not a decision and does not count as completing it.
The brotli quality change and the missing-asset classification both move the daemon binary, so the integrity hash the extension verifies before spawning it moves with them. Generated by pnpm package:win32-x64.
The asset work left five documents behind — a design, a plan, an audit, a review and a refactor plan — describing a feature that has now shipped. A plan kept past its landing stops being a plan and becomes a claim about the present that nobody rechecks, so they are deleted and git keeps the history. What was worth keeping is merged into bundler-architecture.md as a section on the non-JavaScript files a package ships, sitting beside the one on how each kind of import is handled — because that is what it always was: part of how the bundler measures an import, not a separate subject. It carries only the reasoning that cannot be read off the code: why stubbing at the load boundary is what makes the JavaScript number exact rather than an optimisation, why the unmeasured-kind list is an allowlist, why each artifact is compressed on its own, and why any disclosure costs the result its High confidence. known-issues carries the bare-import reasoning inline now rather than citing a plan that no longer exists, and the SRS's references to the deleted bundler-redesign spec point at the construct matrix that still gates every version bump.
ehsan18t
force-pushed
the
bundler-b2-asset-counting
branch
from
July 18, 2026 18:27
86a3c4f to
0ad279c
Compare
ehsan18t
changed the title
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.
Summary
Related issues
Type of change
fix)feat)perf)refactor/style)docs)build/ci/chore)!to the commit type)Checklist
type(scope)!: subjectplus a descriptive body).pnpm testpasses locally (build + TypeScript + scripts + Rust).cargo fmt, Clippy,tsc,cargo deny) — the lefthook pre-commit hook covers these.