Skip to content

Release review fixes for 0.9.0 - #58

Merged
johanrd merged 3 commits into
mainfrom
release-0.9-fixes
Aug 29, 2026
Merged

Release review fixes for 0.9.0#58
johanrd merged 3 commits into
mainfrom
release-0.9-fixes

Conversation

@johanrd

@johanrd johanrd commented Aug 29, 2026

Copy link
Copy Markdown
Owner

Fixes from a review of everything since v0.8.0 (#51, #52, #54#57), before the 0.9.0 release. Correctness for consumers first.

Stale-cache paths closed

  • --no-glint still reads imported component templates (tag substitution), so the import closure is now part of the transform and report keys with Glint off too.
  • A .ts/.js module's co-located .hbs template and /// <reference path> targets are part of the closure. Without a tsconfig, relative imports resolve.
  • The lockfile is found at or above the tsconfig directory, so a workspace with one lockfile at the repository root invalidates on package changes.
  • The tsconfig sha covers the whole extends chain (paths, baseUrl, types in a base config) and is re-validated against the file system.
  • backendKindFor (transform/report keys) makes the same decision as selectBackend: a TypeScript 7 package that resolves but does not load falls back to ts6 in both.
  • A result computed after a Glint extraction threw is not cached; the next run retries. A missing backend is a stable state the key already covers and stays cached.
  • Long-lived hosts: the project-wide input list re-validates on the mtimes of the directories walked (a new .d.ts or augmentation is seen); an index probe watches the directory it lives in; trailing commas are removed outside strings only.

Performance

  • The CLI computes report keys before the Glint preload and preloads the misses only. A fully cached run no longer builds a TypeScript program: bench warm run −52 %.
  • dependencySha is memoised per file content under a static file system; file records keep import lists, not content; a nested package directory is not walked.
  • "One cached file" +17 ms: backendKindFor loads the TypeScript 7 module so that the key matches the backend that ran.

Release notes for 0.9.0 (behaviour a 0.8.0 user notices)

  • Three on-disk caches under node_modules/.cache/html-validate-ember/: Glint extraction (glint/), transform output (transform/), and for validate-gts the report of each file (report/). HVE_NO_CACHE=1 bypasses all three.
  • A cache entry is keyed on the file's content, the content of every project file it imports (transitively), the project's ambient .d.ts files and module augmentations, the lockfile, the tsconfig chain, the html-validate configuration and version, and the plugin. Editing an imported component invalidates its consumers. A .hbs template is keyed on everything the registry reaches.
  • The first run after upgrading is fully cold (new key fields).
  • On a report-cache hit validate-gts replays the stored report; stderr diagnostics from the transform (parse failures) are printed on the run that computed them, not on replays.
  • .htmlvalidate.json is part of the key by content; files it references by path (elements, extends presets) are not.
  • Faster: parse memo (Parse each template once #54), transform cache (Cache the transform output per file #55), report cache (CLI: replay the report of unchanged files #56); on the console app a warm validate-gts run went from 10.8 s to under 1 s.

Both lanes 326 passed + 1 expected fail; typecheck:tests clean.

Cowritten by Claude

- The closure is part of the transform and report keys with Glint off
  too: the resolver reads an imported component's template to
  substitute its tag.
- A `.ts`/`.js` module's co-located `.hbs` template (next to it or under
  `templates/components/`) and `/// <reference path>` targets are part
  of the closure. Without a tsconfig, relative imports still resolve.
- The lockfile is found at or above the tsconfig directory (workspace
  root). A nested directory with its own package.json is not walked for
  project-wide inputs.
- The tsconfig sha covers the whole `extends` chain and is re-validated
  against the file system; `backendKindFor` makes the same decision as
  `selectBackend` (a TypeScript 7 package that does not load falls back
  to ts6 in both).
- A result computed after a Glint extraction threw is not cached at any
  level; the next run retries.
- Long-lived hosts: the project-wide input list re-validates on the
  mtimes of the directories walked; an `index` probe watches the
  directory it lives in; trailing commas are removed outside strings
  only; file records keep import lists, not content.
- The CLI computes report keys before the Glint preload and preloads
  the misses only; `dependencySha` is memoised per content under a
  static file system. `--help` names all three caches.

Cowritten by Claude
@johanrd johanrd added bug Something isn't working run-bench Run the benchmark comparison on this pull request labels Aug 29, 2026
@github-actions

github-actions Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

🏎️ Benchmark Comparison

Benchmark Control (p50) Experiment (p50) Δ
extract small template 708.09 µs 712.32 µs +0.6%
extract medium template 2.72 ms 2.74 ms +0.8%
extract cross-file resolution 2.22 ms 2.20 ms -0.6%
extract large template 550.51 µs 551.67 µs +0.2%
cold run (cache off) 6298.48 ms 6314.02 ms +0.2%
🟢 warm run (all cached) 781.03 ms 291.06 ms -62.7%
one cached file 288.33 ms 294.65 ms +2.2%
🔴 no glint 4539.67 ms 5122.62 ms +12.8%

🟢 faster · 🔴 slower · 🟡 5–10 %, possibly noise · ⚪ within 5 %

Full output
clk: ~3.36 GHz
cpu: Intel(R) Xeon(R) Platinum 8370C CPU @ 2.80GHz
runtime: node 24.19.0 (x64-linux)

benchmark                    avg (min … max) p75 / p99    (min … top 1%)
-------------------------------------------- -------------------------------
extract small template          1.02 ms/iter 980.19 µs ▄█                   
                       (559.16 µs … 5.88 ms)   3.75 ms ██                   
                     (  6.37 kb …   3.84 mb) 301.64 kb ███▅▃▂▃▂▂▂▂▂▁▁▁▂▂▂▂▁▂

extract medium template         3.31 ms/iter   3.48 ms ▆█                   
                        (2.40 ms … 11.62 ms)   8.90 ms ██▃                  
                     (161.63 kb …   4.27 mb) 761.34 kb ███▅▅▃▂▃▃▄▂▂▁▁▁▁▁▁▁▂▁

extract cross-file resolution   2.58 ms/iter   2.64 ms  █                   
                         (1.94 ms … 6.54 ms)   5.47 ms  █▃                  
                     ( 71.72 kb …   1.47 mb) 430.93 kb ████▄▃▄▃▃▂▂▂▁▂▂▁▂▂▁▂▁

extract large template        585.92 µs/iter 582.34 µs  █                   
                       (489.16 µs … 3.07 ms)   1.56 ms  █▄                  
                     ( 11.86 kb …   1.13 mb)  49.20 kb ███▃▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁

whole process (min / p50 of 3 runs, 21 files)
  cold run (cache off)           5798 ms /   6298 ms
  warm run (all cached)           769 ms /    781 ms
  one cached file                 285 ms /    288 ms
  no glint                       4530 ms /   4540 ms
clk: ~3.37 GHz
cpu: Intel(R) Xeon(R) Platinum 8370C CPU @ 2.80GHz
runtime: node 24.19.0 (x64-linux)

benchmark                    avg (min … max) p75 / p99    (min … top 1%)
-------------------------------------------- -------------------------------
extract small template          1.03 ms/iter   1.00 ms ▄█                   
                       (557.56 µs … 4.51 ms)   3.90 ms ██                   
                     (  6.60 kb …   3.85 mb) 300.26 kb ███▅▃▃▂▂▂▂▁▁▂▁▂▁▁▁▂▂▁

extract medium template         3.33 ms/iter   3.46 ms  █                   
                         (2.37 ms … 9.40 ms)   8.66 ms ▄█▂                  
                     ( 63.18 kb …   4.14 mb) 758.05 kb ███▄▃▄▂▂▁▄▂▂▁▂▂▁▁▂▁▁▁

extract cross-file resolution   2.63 ms/iter   2.67 ms  █                   
                        (1.93 ms … 10.29 ms)   7.05 ms  █                   
                     ( 58.40 kb …   1.50 mb) 422.85 kb ██▇▄▃▃▃▂▂▂▃▂▂▁▁▁▁▁▁▁▁

extract large template        585.96 µs/iter 582.42 µs  █                   
                       (494.77 µs … 2.82 ms)   1.56 ms ▄█▅                  
                     (384.00  b …   2.05 mb)  49.83 kb ███▃▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁

whole process (min / p50 of 3 runs, 21 files)
  cold run (cache off)           5820 ms /   6314 ms
  warm run (all cached)           289 ms /    291 ms
  one cached file                 293 ms /    295 ms
  no glint                       5096 ms /   5123 ms

@johanrd
johanrd requested a balanced review from Copilot August 29, 2026 11:28
@johanrd johanrd added the run-ecosystem-ci Run the (expensive) ecosystem CI on this PR label Aug 29, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The changes rewrite cross-file cache-invalidation logic that affects correctness for all consumers, and include a confirmed JSONC parsing regression, so a human should review before approval.

Pull request overview

This PR bundles correctness and performance fixes ahead of the 0.9.0 release, focused on the on-disk cache keys (Glint/transform/report) so that stale-cache paths introduced since v0.8.0 are closed. It makes the import closure part of the cache key even with Glint off, tightens dependency tracking (tsconfig extends chain, lockfile discovery, co-located/classic .hbs peers, /// <reference path> targets), and reorders the CLI so report keys are computed before the Glint preload (preloading only cache misses).

Changes:

  • lib/deps.ts: broadened dependency scanning/resolution (reference-path directives, .hbs peers, no-tsconfig relative resolution, workspace lockfile lookup), added tsconfigChainSha, reworked JSONC comma handling, and revised memoization/re-validation.
  • run.ts + transform.ts: precompute report keys, preload only misses, and track files whose Glint extraction threw (__glintUnavailable) so their results are not cached.
  • lib/cache.ts + lib/backend/index.ts: tsconfig sha now covers the whole extends chain; backendKindFor uses loadTsgo to match selectBackend's decision; cache-key dependency short-circuits only when caching is disabled.
File summaries
File Description
lib/deps.ts Core dependency-closure/resolution rewrite: reference-path scanning, .hbs peers, tsconfig chain sha, lockfile discovery, JSONC comma handling, memo re-validation.
lib/cache.ts getTsconfigSha delegates to tsconfigChainSha; key dependency computed unless cache disabled.
lib/backend/index.ts backendKindFor loads tsgo (matching selectBackend) instead of only resolving the package.
transform.ts Adds __glintUnavailable tracking to avoid caching results after a Glint extraction throws.
run.ts Precomputes report keys, preloads only Glint misses, guards report writes on __glintUnavailable.
test/deps.test.ts New tests for peers, reference paths, workspace lockfile, no-tsconfig resolution, chain sha.
test/cache.test.ts Adds assertion that editing the tsconfig changes the report key.
Review details
  • Files reviewed: 7/7 changed files
  • Comments generated: 2
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/deps.ts
Comment thread lib/deps.ts Outdated
…ndows separators

Copilot review on #58: the string-safe JSONC scanner only skipped whitespace when
looking for the closing bracket, so `{ "a": 1, // note\n }` kept the comma and
JSON.parse failed after comment removal. The templates/components peer regex was
POSIX-only, dropping the classic-layout peer from the closure on Windows.
The cache-key component loaded typescript-7's sync and ast modules
(~30 ms) on every process, including replays that never need a backend.
Whether the package loads is decided by its version and the Node version,
so both are in the key and the package is only resolved.
@johanrd
johanrd merged commit 2fdc5b6 into main Aug 29, 2026
4 checks passed
@github-actions github-actions Bot mentioned this pull request Aug 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working run-bench Run the benchmark comparison on this pull request run-ecosystem-ci Run the (expensive) ecosystem CI on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants