0.6.0: make-it-loud round 2 + dogfood fixes (PR 1 of 2)#94
Conversation
edge.htmlAttributes is a pre-serialized string with no clean Astro spread, so
consumers hand-write the root `data-czap-*` attrs and silently miss any new
CapAxis. Add the structured, spreadable form:
- `EdgeTier.tierDataAttributesMap(result)` → `Record<`data-czap-${CapAxis}`,
string>`, built by iterating the canonical CAP_AXES registry so a new axis
appears automatically. `tierDataAttributes` (string) is now serialized FROM
the map, so the two forms can never drift.
- `EdgeHostResolution.htmlAttributesMap` on the host-adapter result (mirrored in
_spine/edge.d.ts). Astro: `<html {...resolution.htmlAttributesMap}>`.
Gated: edge-tier.test.ts pins map == one key per CAP_AXES axis AND that the
string form serializes EXACTLY the map (add an axis to one without the other and
it reds).
NOTE: new public export → the api-surface semver gate requires a minor bump
(@czap/edge 0.5.0 → 0.6.0). Version bump + snapshot/docs regen intentionally
deferred to the release decision.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KxU3Y8XueHqfteVGA4KdEh
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 Walkthrough📝 Walkthrough🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
This comment has been minimized.
This comment has been minimized.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66080ddc51
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| markBound(element, name); | ||
| init(load, opts, element); |
There was a problem hiding this comment.
When the scanner boots an implicit plain-element attribute before Astro invokes the same custom directive entry (for example after a view-transition swap or any delayed island hydration), this entry point still initializes the runtime even though the shared data-czap-directive-bound guard is already set. The stream/LLM/worker/GPU initializers are not idempotent, so that ordering can open duplicate EventSource, worker, or shader sessions for one element; the Astro entry path should return when the element is already bound for name before calling init.
Useful? React with 👍 / 👎.
| // Accept CSS-authored vector component lists (`1 2`, `1, 2`) and WGSL-style | ||
| // constructors (`vec2f(1, 2)`) without treating the constructor's `2/3/4` as a | ||
| // component. GLSL stays scalar-only above. | ||
| const componentSource = trimmed.replace(/\bvec[234][fiu]?\s*\(/gi, '('); |
There was a problem hiding this comment.
When an @wgsl value is authored with valid WGSL constructor syntax such as vec2<f32>(1, 2), this regex does not strip the generic type prefix, so the numeric scan also captures the 2 from vec2 and 32 from f32 and turns the value into [2, 32, 1, 2]. That makes the compiler infer a vec4f field and emit incorrect state bindings, causing shaders that expect the intended vec2<f32> uniform to fail validation or sample the wrong data.
Useful? React with 👍 / 👎.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
packages/astro/src/runtime/directive-boot.ts (1)
98-114: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
bootDirectiveEntryhas no unmark-on-failure symmetry withscanAndBootDirectives.
markBoundruns beforeinit, but unlike the activation loop below (which unmarks on failure specifically "so a later re-scan (astro:after-swap) can retry after a transient chunk-load error"), this shared helper has no equivalent safety net. Any future directive entry whoseinitcallback lets an error propagate (sync throw or unhandled rejection) will leave the element permanently marked bound with no retry path.Consider wrapping
initso failures unmark the element, mirroring the existing pattern:♻️ Proposed fix
export function bootDirectiveEntry( name: DirectiveName, load: () => Promise<unknown>, opts: Record<string, unknown>, element: HTMLElement, init: (load: () => Promise<unknown>, opts: Record<string, unknown>, element: HTMLElement) => void, ): void { markBound(element, name); - init(load, opts, element); + try { + init(load, opts, element); + } catch (error) { + unmarkBound(element, name); + throw error; + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/astro/src/runtime/directive-boot.ts` around lines 98 - 114, `bootDirectiveEntry` marks the element as bound before calling `init`, but it does not restore the unbound state if initialization fails, unlike `scanAndBootDirectives` which unmarks on error. Update `bootDirectiveEntry` to wrap the `init` call in failure handling so any sync throw or rejected async initialization removes the bound mark for the given `name` on `element`, preserving retry behavior for later scans. Keep the fix localized to the `bootDirectiveEntry`/`markDirectiveBound` flow and mirror the existing unmark-on-failure pattern used by the directive scanner.tests/unit/edge/kv-cache.test.ts (1)
302-327: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding a negative-path test.
This test only covers valid scalar/vec2/vec3/vec4 shapes.
asWGSLUniformValueinkv-cache.tsalso rejects malformed vectors (wrong length, NaN, mixed types) by dropping the field — a test exercising that path would guard the validation logic against regressions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/edge/kv-cache.test.ts` around lines 302 - 327, Add a negative-path unit test around the KV cache WGSL rehydrate flow to cover malformed vector bindings, not just valid vec2/vec3/vec4 values. Use the existing test area in kv-cache.test.ts and exercise the validation path in asWGSLUniformValue from kv-cache.ts with cases like wrong-length arrays, NaN, or mixed-type vectors. Assert that invalid fields are dropped/omitted after putCompiledOutputs and getCompiledOutputs, so the rehydration behavior stays protected against regressions.packages/cli/src/commands/audit.ts (1)
86-100: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReceipt line duplicates the entire findings array already streamed to stdout.
When
opts.findingsis set,payload.findingsis included inreceipt(via...payload), soJSON.stringify(receipt)written to stderr embeds the full findings array a second time, in addition to the NDJSON lines on stdout. This doubles output size for large audits and undermines the summary/stream separation the feature is meant to provide.♻️ Proposed fix: strip findings from the stderr summary line
if (opts.findings) { - process.stderr.write(JSON.stringify(receipt) + '\n'); - for (const finding of payload.findings ?? []) { + const { findings: streamedFindings, ...summaryReceipt } = receipt as AuditReceipt & { + findings?: readonly AuditFinding[]; + }; + process.stderr.write(JSON.stringify(summaryReceipt) + '\n'); + for (const finding of streamedFindings ?? []) { process.stdout.write(JSON.stringify(finding) + '\n'); } } else { emit(receipt); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/commands/audit.ts` around lines 86 - 100, The audit summary written in the audit command duplicates the full findings array because `receipt` is built from `...payload` and then serialized when `opts.findings` is enabled. Update the `audit` command flow to exclude `findings` from the stderr `receipt` object (or otherwise strip it before `JSON.stringify(receipt)`) while still streaming each finding individually to stdout in the `payload.findings` loop.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Around line 29-34: The changelog bullet in the update note misstates the
registry mapping for data-czap-boundary by describing it as a worker/GPU
payload, while the actual directive mapping is different. Update the wording in
the changelog entry that references DIRECTIVE_ATTRIBUTE_REGISTRY and the
scannable attribute behavior so data-czap-boundary is described only as
satellite/worker, and keep GPU associated with its own data-czap-shader-src
attribute.
In `@docs/api/edge/src/variables/EdgeTier.md`:
- Around line 67-99: The docs for tierDataAttributesMap currently describe a
mutable Record, but the exported API is readonly, so update the generated
markdown in EdgeTier.md to match the actual contract. In the
tierDataAttributesMap section, adjust the return type and any prose/examples
tied to mutability so they reflect Readonly<Record<string, string>> (or the
equivalent readonly mapped shape used by EdgeTier.tierDataAttributesMap),
keeping the docs consistent with the declaration.
In `@packages/astro/src/runtime/wasm.ts`:
- Around line 89-96: The wasmDirective callback is invoking runtimeLoad()
without handling its returned promise, which can lead to an unhandled rejection
if the directive load fails. Update the wasmDirective/bootDirectiveEntry
callback to explicitly handle the Promise from runtimeLoad, matching the
existing fire-and-forget pattern used for loadWasmRuntime(runtimeEl) (for
example, by discarding it intentionally and ensuring any rejection is caught).
Keep the fix localized to wasmDirective and its runtimeLoad call site.
In `@packages/vite/src/boundary-manifest.ts`:
- Around line 247-268: parseWgslCastValue and wgslVector currently accept
malformed `@wgsl` vector literals silently, which can narrow or drop authored
values without any diagnostic. Update the WGSL cast parsing path in
boundary-manifest to validate the parsed component count against the authored
vec2/vec3/vec4 constructor shape, and route any mismatch or unparseable vector
value through Diagnostics.warnOnce instead of returning a misleading
scalar/vector or undefined. Use parseWgslCastValue, wgslVector, and the existing
Diagnostics warning patterns in this file to keep malformed WGSL uniform values
from degrading silently.
In `@packages/vite/src/plugin.ts`:
- Around line 26-31: Both ensureBoundaryDefinitions() and
ensureBoundaryManifest() are independently calling scanProject(projectRoot),
which duplicates the same filesystem walk when a project uses both boundary
caches. Update the boundary caching flow in plugin.ts so the scan result from
scanProject is shared between ensureBoundaryDefinitions() and
ensureBoundaryManifest(), ideally by caching the scanned project data once and
reusing it in collectBoundaryDefinitions and collectBoundaryManifest.
---
Nitpick comments:
In `@packages/astro/src/runtime/directive-boot.ts`:
- Around line 98-114: `bootDirectiveEntry` marks the element as bound before
calling `init`, but it does not restore the unbound state if initialization
fails, unlike `scanAndBootDirectives` which unmarks on error. Update
`bootDirectiveEntry` to wrap the `init` call in failure handling so any sync
throw or rejected async initialization removes the bound mark for the given
`name` on `element`, preserving retry behavior for later scans. Keep the fix
localized to the `bootDirectiveEntry`/`markDirectiveBound` flow and mirror the
existing unmark-on-failure pattern used by the directive scanner.
In `@packages/cli/src/commands/audit.ts`:
- Around line 86-100: The audit summary written in the audit command duplicates
the full findings array because `receipt` is built from `...payload` and then
serialized when `opts.findings` is enabled. Update the `audit` command flow to
exclude `findings` from the stderr `receipt` object (or otherwise strip it
before `JSON.stringify(receipt)`) while still streaming each finding
individually to stdout in the `payload.findings` loop.
In `@tests/unit/edge/kv-cache.test.ts`:
- Around line 302-327: Add a negative-path unit test around the KV cache WGSL
rehydrate flow to cover malformed vector bindings, not just valid vec2/vec3/vec4
values. Use the existing test area in kv-cache.test.ts and exercise the
validation path in asWGSLUniformValue from kv-cache.ts with cases like
wrong-length arrays, NaN, or mixed-type vectors. Assert that invalid fields are
dropped/omitted after putCompiledOutputs and getCompiledOutputs, so the
rehydration behavior stays protected against regressions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5eca8fbc-8048-4e26-b853-0401184f04f6
📒 Files selected for processing (120)
ASTRO-RUNTIME-MODEL.mdCHANGELOG.mdGETTING-STARTED.mddocs/adr/0028-plain-element-directive-scanner.mddocs/adr/0029-wgsl-uniform-buffer-layout.mddocs/adr/README.mddocs/api/astro/src/functions/resolveInitialStateFallback.mddocs/api/astro/src/functions/satelliteAttrs.mddocs/api/astro/src/interfaces/SatelliteProps.mddocs/api/audit/src/functions/runAuditPasses.mddocs/api/cloudflare/src/functions/cloudflareMiddleware.mddocs/api/compiler/src/README.mddocs/api/compiler/src/interfaces/WGSLBinding.mddocs/api/compiler/src/interfaces/WGSLCompileResult.mddocs/api/compiler/src/interfaces/WGSLStruct.mddocs/api/compiler/src/type-aliases/CompilerDef.mddocs/api/compiler/src/type-aliases/WGSLStates.mddocs/api/compiler/src/type-aliases/WGSLUniformValue.mddocs/api/compiler/src/type-aliases/WGSLUniformVector.mddocs/api/compiler/src/variables/WGSLCompiler.mddocs/api/edge/src/functions/createBoundaryCache.mddocs/api/edge/src/functions/createEdgeHostAdapter.mddocs/api/edge/src/interfaces/BoundaryCache.mddocs/api/edge/src/interfaces/EdgeHostAdapter.mddocs/api/edge/src/interfaces/EdgeHostResolution.mddocs/api/edge/src/namespaces/EdgeHostAdapter/type-aliases/BoundaryConfig.mddocs/api/edge/src/namespaces/EdgeHostAdapter/type-aliases/BoundaryResolution.mddocs/api/edge/src/namespaces/EdgeHostAdapter/type-aliases/CacheStatus.mddocs/api/edge/src/namespaces/EdgeHostAdapter/type-aliases/CacheTags.mddocs/api/edge/src/namespaces/EdgeHostAdapter/type-aliases/CompileContext.mddocs/api/edge/src/namespaces/EdgeHostAdapter/type-aliases/Config.mddocs/api/edge/src/namespaces/EdgeHostAdapter/type-aliases/Context.mddocs/api/edge/src/namespaces/EdgeHostAdapter/type-aliases/Resolution.mddocs/api/edge/src/namespaces/EdgeTier/type-aliases/Result.mddocs/api/edge/src/variables/EdgeHostAdapter.mddocs/api/edge/src/variables/EdgeTier.mddocs/api/edge/src/variables/KVCache.mddocs/api/vite/src/functions/collectBoundaryManifest.mddocs/api/vite/src/functions/plugin.mddocs/api/vite/src/functions/serializeBoundaryOutput.mddocs/api/vite/src/interfaces/CollectBoundaryManifestOptions.mddocs/api/vite/src/interfaces/PluginConfig.mdpackage.jsonpackages/_spine/compiler.d.tspackages/_spine/edge.d.tspackages/_spine/package.jsonpackages/assets/package.jsonpackages/astro/package.jsonpackages/astro/src/Satellite.astropackages/astro/src/Satellite.tspackages/astro/src/client-directives/gpu.tspackages/astro/src/client-directives/graph.tspackages/astro/src/client-directives/llm.tspackages/astro/src/client-directives/satellite.tspackages/astro/src/client-directives/stream.tspackages/astro/src/client-directives/svg.tspackages/astro/src/client-directives/wasm.tspackages/astro/src/client-directives/worker.tspackages/astro/src/runtime/boundary.tspackages/astro/src/runtime/directive-boot.tspackages/astro/src/runtime/gpu.tspackages/astro/src/runtime/graph-directive.tspackages/astro/src/runtime/inspector-panels.tspackages/astro/src/runtime/llm.tspackages/astro/src/runtime/satellite.tspackages/astro/src/runtime/slots.tspackages/astro/src/runtime/stream.tspackages/astro/src/runtime/svg.tspackages/astro/src/runtime/wasm.tspackages/astro/src/runtime/wgpu.tspackages/astro/src/runtime/worker.tspackages/audit/package.jsonpackages/audit/src/index.tspackages/canonical/package.jsonpackages/cli/package.jsonpackages/cli/src/commands/audit.tspackages/cloudflare/package.jsonpackages/cloudflare/src/middleware.tspackages/command/package.jsonpackages/compiler/package.jsonpackages/compiler/src/dispatch.tspackages/compiler/src/index.tspackages/compiler/src/wgsl.tspackages/core/package.jsonpackages/create-liteship/package.jsonpackages/create-liteship/templates/default/package.jsonpackages/detect/package.jsonpackages/edge/package.jsonpackages/edge/src/edge-tier.tspackages/edge/src/host-adapter.tspackages/edge/src/kv-cache.tspackages/error/package.jsonpackages/gauntlet/package.jsonpackages/genui/package.jsonpackages/liteship/package.jsonpackages/mcp-server/package.jsonpackages/quantizer/package.jsonpackages/remotion/package.jsonpackages/scene/package.jsonpackages/stage/package.jsonpackages/vite/package.jsonpackages/vite/src/boundary-manifest.tspackages/vite/src/plugin.tspackages/vite/src/transform-css.tspackages/web/package.jsonpackages/worker/package.jsontests/fixtures/api-surface-snapshot.jsontests/integration/wgsl-cast.test.tstests/property/quantizer-output-changes.prop.test.tstests/unit/astro/astro-directive-branches.test.tstests/unit/astro/directive-boot-scanner.test.tstests/unit/astro/wgpu-runtime.test.tstests/unit/cloudflare/middleware.test.tstests/unit/compiler/wgsl-compiler.test.tstests/unit/devops/audit-command.test.tstests/unit/devops/audit-consumer-mode.test.tstests/unit/edge/edge-tier.test.tstests/unit/edge/kv-cache.test.tstests/unit/vite/boundary-manifest.test.tstests/unit/vite/vite-runtime.test.ts
| - **`@czap/astro` — directives boot on plain elements.** `client:stream` / | ||
| `client:llm` / `client:gpu` / `client:wasm` / `client:graph` now activate on | ||
| plain elements, not only framework islands, via a scanner whose selectors derive | ||
| from `DIRECTIVE_ATTRIBUTE_REGISTRY` (a new directive is scannable by construction). | ||
| A bare `data-czap-boundary` — also a worker/GPU payload — stays explicit and | ||
| warns once when found bare instead of silently doing nothing. See ADR-0028. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Changelog claim doesn't match the registry it describes.
This bullet states data-czap-boundary is "also a worker/GPU payload," but DIRECTIVE_ATTRIBUTE_REGISTRY in packages/astro/src/runtime/slots.ts maps data-czap-boundary only to satellite and worker (both implicitBoot: false); gpu uses a distinct data-czap-shader-src attribute (implicitBoot: true). The "GPU" reference here is inaccurate and could mislead consumers about which directive owns which attribute.
✏️ Suggested wording fix
- A bare `data-czap-boundary` — also a worker/GPU payload — stays explicit and
- warns once when found bare instead of silently doing nothing. See ADR-0028.
+ A bare `data-czap-boundary` — also a worker payload — stays explicit and
+ warns once when found bare instead of silently doing nothing. See ADR-0028.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - **`@czap/astro` — directives boot on plain elements.** `client:stream` / | |
| `client:llm` / `client:gpu` / `client:wasm` / `client:graph` now activate on | |
| plain elements, not only framework islands, via a scanner whose selectors derive | |
| from `DIRECTIVE_ATTRIBUTE_REGISTRY` (a new directive is scannable by construction). | |
| A bare `data-czap-boundary` — also a worker/GPU payload — stays explicit and | |
| warns once when found bare instead of silently doing nothing. See ADR-0028. | |
| - **`@czap/astro` — directives boot on plain elements.** `client:stream` / | |
| `client:llm` / `client:gpu` / `client:wasm` / `client:graph` now activate on | |
| plain elements, not only framework islands, via a scanner whose selectors derive | |
| from `DIRECTIVE_ATTRIBUTE_REGISTRY` (a new directive is scannable by construction). | |
| A bare `data-czap-boundary` — also a worker payload — stays explicit and | |
| warns once when found bare instead of silently doing nothing. See ADR-0028. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CHANGELOG.md` around lines 29 - 34, The changelog bullet in the update note
misstates the registry mapping for data-czap-boundary by describing it as a
worker/GPU payload, while the actual directive mapping is different. Update the
wording in the changelog entry that references DIRECTIVE_ATTRIBUTE_REGISTRY and
the scannable attribute behavior so data-czap-boundary is described only as
satellite/worker, and keep GPU associated with its own data-czap-shader-src
attribute.
| ### tierDataAttributesMap | ||
|
|
||
| > **tierDataAttributesMap**: (`result`) => `Record`\<`` `data-czap-${CapAxis}` ``, `string`\> | ||
|
|
||
| Structured, spreadable `data-czap-*` map for the root HTML element (auto-includes every CAP_AXES axis). | ||
|
|
||
| Structured `data-czap-*` attribute map for the root `<html>` element — the | ||
| spreadable form of [tierDataAttributes](#tierdataattributes). | ||
|
|
||
| Keyed by the FULL attribute name (`data-czap-<axis>`), built by iterating the | ||
| canonical CAP_AXES registry, so a newly-added capability axis appears | ||
| automatically. A consumer that spreads this map (`<html {...map}>`) can never | ||
| silently MISS an axis the way a hand-written attribute list does — the whole | ||
| point of exposing it alongside the pre-serialized string. | ||
|
|
||
| #### Parameters | ||
|
|
||
| ##### result | ||
|
|
||
| [`EdgeTierResult`](../interfaces/EdgeTierResult.md) | ||
|
|
||
| #### Returns | ||
|
|
||
| `Record`\<`` `data-czap-${CapAxis}` ``, `string`\> | ||
|
|
||
| #### Example | ||
|
|
||
| ```ts | ||
| // Astro: <html {...EdgeTier.tierDataAttributesMap(result)}> | ||
| tierDataAttributesMap(result) | ||
| // => { 'data-czap-tier': 'reactive', 'data-czap-motion': 'animations', 'data-czap-design': 'enhanced' } | ||
| ``` | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep the new map docs aligned with the readonly contract.
tierDataAttributesMap is still rendered here as a mutable Record, but the exported declaration is Readonly<Record<string, string>>. That mismatch advertises mutability that callers don't actually get.
🧰 Tools
🪛 LanguageTool
[grammar] ~67-~67: Ensure spelling is correct
Context: ..." data-czap-design="enhanced"' ``` ### tierDataAttributesMap > tierDataAttributesMap: (result) => `...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~78-~78: Consider using “who” when you are referring to a person instead of an object.
Context: ... axis appears automatically. A consumer that spreads this map (<html {...map}>) ca...
(THAT_WHO)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/api/edge/src/variables/EdgeTier.md` around lines 67 - 99, The docs for
tierDataAttributesMap currently describe a mutable Record, but the exported API
is readonly, so update the generated markdown in EdgeTier.md to match the actual
contract. In the tierDataAttributesMap section, adjust the return type and any
prose/examples tied to mutability so they reflect Readonly<Record<string,
string>> (or the equivalent readonly mapped shape used by
EdgeTier.tierDataAttributesMap), keeping the docs consistent with the
declaration.
|
|
||
| /** Astro client directive entry that marks the host before starting the WASM runtime. */ | ||
| export const wasmDirective = (load: () => Promise<unknown>, opts: Record<string, unknown>, el: HTMLElement): void => { | ||
| bootDirectiveEntry('wasm', load, opts, el, (runtimeLoad, _runtimeOpts, runtimeEl) => { | ||
| void loadWasmRuntime(runtimeEl); | ||
| runtimeLoad(); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Unhandled promise from runtimeLoad().
loadWasmRuntime(runtimeEl) is explicitly marked void (it self-catches internally), but runtimeLoad() — typed () => Promise<unknown> — is called bare with no void/await/catch. If the real directive load implementation ever rejects (chunk-load failure, etc.), this becomes an unhandled rejection.
🩹 Proposed fix
bootDirectiveEntry('wasm', load, opts, el, (runtimeLoad, _runtimeOpts, runtimeEl) => {
void loadWasmRuntime(runtimeEl);
- runtimeLoad();
+ void runtimeLoad();
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** Astro client directive entry that marks the host before starting the WASM runtime. */ | |
| export const wasmDirective = (load: () => Promise<unknown>, opts: Record<string, unknown>, el: HTMLElement): void => { | |
| bootDirectiveEntry('wasm', load, opts, el, (runtimeLoad, _runtimeOpts, runtimeEl) => { | |
| void loadWasmRuntime(runtimeEl); | |
| runtimeLoad(); | |
| }); | |
| }; | |
| /** Astro client directive entry that marks the host before starting the WASM runtime. */ | |
| export const wasmDirective = (load: () => Promise<unknown>, opts: Record<string, unknown>, el: HTMLElement): void => { | |
| bootDirectiveEntry('wasm', load, opts, el, (runtimeLoad, _runtimeOpts, runtimeEl) => { | |
| void loadWasmRuntime(runtimeEl); | |
| void runtimeLoad(); | |
| }); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/astro/src/runtime/wasm.ts` around lines 89 - 96, The wasmDirective
callback is invoking runtimeLoad() without handling its returned promise, which
can lead to an unhandled rejection if the directive load fails. Update the
wasmDirective/bootDirectiveEntry callback to explicitly handle the Promise from
runtimeLoad, matching the existing fire-and-forget pattern used for
loadWasmRuntime(runtimeEl) (for example, by discarding it intentionally and
ensuring any rejection is caught). Keep the fix localized to wasmDirective and
its runtimeLoad call site.
| import { | ||
| collectBoundaryDefinitions, | ||
| collectBoundaryManifest, | ||
| serializeBoundaryOutput, | ||
| type BoundaryDefinitionMap, | ||
| } from './boundary-manifest.js'; |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -B3 -A15 'export async function collectBoundaryManifest' packages/vite/src/boundary-manifest.ts
rg -n -B3 -A15 'export async function collectBoundaryDefinitions' packages/vite/src/boundary-manifest.ts
rg -n 'scanProject\(' packages/vite/src/boundary-manifest.tsRepository: freebatteryfactory/LiteShip
Length of output: 2130
🏁 Script executed:
#!/bin/bash
sed -n '180,240p' packages/vite/src/plugin.ts
sed -n '240,320p' packages/vite/src/plugin.ts
sed -n '480,540p' packages/vite/src/boundary-manifest.tsRepository: freebatteryfactory/LiteShip
Length of output: 8084
Share the boundary scan between these caches. In packages/vite/src/plugin.ts, ensureBoundaryDefinitions() and ensureBoundaryManifest() each call scanProject(projectRoot), so a project that hits both the CSS @quantize path and virtual:czap/boundaries will walk the tree twice. Reusing one scan result would remove the duplicate filesystem pass.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/vite/src/plugin.ts` around lines 26 - 31, Both
ensureBoundaryDefinitions() and ensureBoundaryManifest() are independently
calling scanProject(projectRoot), which duplicates the same filesystem walk when
a project uses both boundary caches. Update the boundary caching flow in
plugin.ts so the scan result from scanProject is shared between
ensureBoundaryDefinitions() and ensureBoundaryManifest(), ideally by caching the
scanned project data once and reusing it in collectBoundaryDefinitions and
collectBoundaryManifest.
… single-chunk Commit 6 routed each client-directive runtime through `bootDirectiveEntry` from `directive-boot.ts`, whose `LOADERS` dynamic-import all 8 directives. That dragged code-splitting into `runtime/stream.ts`'s graph, so the e2e stream lib-bundle split into 22 chunks; the single-file e2e server served only the entry chunk, its static imports of the sibling chunks 404'd in the browser, the module never executed, and `window.__streamPromise` stayed undefined -- failing `stream.e2e` on truth-linux and all three browser-e2e jobs, invisible to the local (non-browser) run. Extract the bound-marker primitives (`boundNames` / `markBound` / `unmarkBound` / `markDirectiveBound` / `bootDirectiveEntry`) into a dependency-free leaf `directive-bound.ts`. The 8 runtime directives import from the leaf; `directive-boot.ts` (the scanner) keeps `LOADERS` and re-exports the primitives for back-compat. Behavior is unchanged. Also make the e2e server fail LOUD if a lib bundle ever code-splits again -- naming the entry, the chunk count, and the fix -- instead of silently serving a broken chunk. Verified locally: stream-bundle rebuilt 22 -> 1 chunk; typecheck/lint/format green; tests/unit/astro 504 passed; build green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014wkz4TzwswnSCMVWQAngSd
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90cb9543c6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| function warnEmptyManifestOutputs(boundary: string, entry: BoundaryManifestEntry): void { | ||
| if (entry.outputs.length > 0 || Object.keys(entry.outputsByTier).length > 0) return; |
There was a problem hiding this comment.
When a boundary has an @quantize block that only emits non-CSS casts (for example @wgsl/@glsl/@aria) or otherwise compiles to no CSS, collectBoundaryManifest still produces a non-empty outputs pool and outputsByTier entries whose CSS fields serialize to ''. This early return suppresses the new diagnostic, and resolveBoundaryOutputs treats that precompiled object as a hit, so Cloudflare serves an empty stylesheet with no warning or compile fallback. The guard needs to check whether any selected output actually has servable CSS, not just whether the pool/indexes exist.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/astro/src/runtime/directive-bound.ts (1)
48-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the redundant
markDirectiveBoundwrapper
packages/astro/src/runtime/directive-bound.tsonly forwards tomarkBound, and the repo has no internal call sites beyond a re-export frompackages/astro/src/runtime/directive-boot.ts. If the alias isn’t needed for the public API, drop the wrapper and exportmarkBounddirectly to keep the leaf API minimal.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/astro/src/runtime/directive-bound.ts` around lines 48 - 51, The markDirectiveBound wrapper is redundant because it only forwards to markBound and has no internal call sites beyond the directive-boot re-export. Remove the markDirectiveBound function from directive-bound.ts and update the runtime export surface to expose markBound directly, keeping the existing DirectiveName/HTMLElement binding behavior intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/astro/src/runtime/directive-bound.ts`:
- Around line 48-51: The markDirectiveBound wrapper is redundant because it only
forwards to markBound and has no internal call sites beyond the directive-boot
re-export. Remove the markDirectiveBound function from directive-bound.ts and
update the runtime export surface to expose markBound directly, keeping the
existing DirectiveName/HTMLElement binding behavior intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c420d8ef-39e3-44e1-a8e2-e69f0c47de5b
📒 Files selected for processing (11)
packages/astro/src/runtime/directive-boot.tspackages/astro/src/runtime/directive-bound.tspackages/astro/src/runtime/gpu.tspackages/astro/src/runtime/graph-directive.tspackages/astro/src/runtime/llm.tspackages/astro/src/runtime/satellite.tspackages/astro/src/runtime/stream.tspackages/astro/src/runtime/svg.tspackages/astro/src/runtime/wasm.tspackages/astro/src/runtime/worker.tstests/e2e/server.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/astro/src/runtime/wasm.ts
- packages/astro/src/runtime/gpu.ts
- packages/astro/src/runtime/satellite.ts
- packages/astro/src/runtime/stream.ts
- packages/astro/src/runtime/svg.ts
- packages/astro/src/runtime/llm.ts
- packages/astro/src/runtime/graph-directive.ts
- packages/astro/src/runtime/worker.ts
- packages/astro/src/runtime/directive-boot.ts
…t, empty-CSS warn, +3) Batch-fix the real findings from the CodeRabbit / Greptile / Codex review of PR #94: - vite (Major): parse the generic `vec2<f32>(...)` WGSL constructor correctly -- strip the whole `<...>` before the numeric scan (it was capturing `f32`'s `32`, turning `vec2<f32>(1,2)` into a `vec4f`) and validate the component count against the declared vecN, warning + dropping a malformed vector instead of shipping a silently-wrong offset. Upholds ADR-0029's own guarantee. - astro (P2): make `bootDirectiveEntry` idempotent + collision-aware. The plain-element scanner and Astro's island hydration both route through it; without the guard, an island that also carries the directive's implicit attribute was booted twice (duplicate EventSource / worker / shader). Collision detection moves here from the scanner so it fires for both boot paths; the scanner drops its now-redundant pre-mark. - cloudflare (P2): warn when a manifest boundary's pool serializes to no SERVABLE CSS (an @quantize block emitting only @wgsl/@glsl/@aria), not just when the pool is empty. - cli (P2): default `audit --pretty` off under `--findings`, so the per-finding lines don't duplicate the receipt JSON / NDJSON stream on a TTY. - edge: `tierDataAttributesMap` returns `Readonly<...>` to match the spine contract. - astro: `void` the bare `runtimeLoad()` in `wasmDirective` (unhandled-rejection guard). - docs: correct "worker/GPU" -> "satellite/worker" for `data-czap-boundary` in the CHANGELOG + ADR-0028 (the registry maps it to satellite + worker); regenerate docs/api. Deferred to its own change: sharing `scanProject` between the two boundary caches -- an HMR-sensitive signature/cache refactor for a marginal (one-scan-at-startup) win, not a safe pre-merge bang-out. Regression guards: WGSL generic/malformed parse test; `bootDirectiveEntry` idempotence + collision unit tests; the gpu shader-fetch test now uses a fresh host (re-booting the same element is correctly a no-op now). Verified: typecheck x3 / lint / format / build / check-invariants green; vite / cloudflare / edge / astro / cli / meta slices green; docs:check green; the e2e stream lib-bundle stays a single chunk. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014wkz4TzwswnSCMVWQAngSd
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 096a08f79d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (hasDirectiveMarker(element) || hasImplicitDirectiveAttribute(element)) { | ||
| continue; |
There was a problem hiding this comment.
When an element has data-czap-boundary plus an implicit directive attribute such as data-czap-shader-src, but no data-czap-directive="satellite"/"worker", this guard suppresses the explicit-only boundary diagnostic because hasImplicitDirectiveAttribute is true. The scanner then boots only the GPU directive; no satellite/worker evaluator runs to call applyBoundaryState, so boundary state and @wgsl/@glsl uniform updates remain inert without the warning expected for explicit-only boundary payloads.
Useful? React with 👍 / 👎.
… not in bootDirectiveEntry The previous commit moved collision detection into `bootDirectiveEntry`, but a directive whose own tier/capability gate no-ops (e.g. `client:gpu` at a low tier) never reaches `bootDirectiveEntry` -- so a "two directives on one element" collision went undetected. The browser test `astro-directive-boot > ...warn about the collision (client:gpu + satellite trap)` regressed (truth-linux `coverage:browser` + all three browser jobs). Detect collision at the scanner instead, MARKER-based: if an element carries a marker for two enabled directives, warn -- independent of whether either directive actually boots. `bootDirectiveEntry` keeps ONLY the idempotence guard (the double-boot fix); the scanner no longer pre-marks (the entry owns marking), and its unmark-on-failure still enables retry. Verified: typecheck x3 / lint / format / build green; tests/unit/astro 506 + meta 611 green; new scanner-collision (marker-based, gate-independent) + bootDirectiveEntry idempotence unit tests; the e2e stream lib-bundle stays a single chunk. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014wkz4TzwswnSCMVWQAngSd
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2074b9cdda
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| element: HTMLElement, | ||
| init: (load: () => Promise<unknown>, opts: Record<string, unknown>, element: HTMLElement) => void, | ||
| ): void { | ||
| if (boundNames(element).has(name)) return; |
There was a problem hiding this comment.
When the plain-element scanner sees data-czap-shader-src before Astro hydrates the same island, it calls this entry with empty opts and marks gpu bound; on static/styled provisional tiers initGPUDirective only registers the detect-ready retry. If Astro then invokes client:gpu={{ force: true }}, this guard returns before gpuDirective can read opts.value, so the documented force escape hatch is ignored exactly on low/headless tiers where it is needed. Consider allowing a forced GPU call to upgrade a prior scanner no-op rather than treating every prior bind as final.
Useful? React with 👍 / 👎.
| if (ctx.boundaryDefinitions && !discovered) { | ||
| throw ValidationError( |
There was a problem hiding this comment.
When a transformed CSS file lives outside the project scan (for example a linked package or node_modules, which scanProject skips) and keeps its boundaries.ts beside the stylesheet, ctx.boundaryDefinitions is still present but lacks that local export. This throws before the existing resolvePrimitive(..., id, ...) fallback below can search from the CSS file's directory, so package-authored @quantize blocks that previously resolved locally now hard-fail unless the app re-exports every boundary at the project root.
Useful? React with 👍 / 👎.
| const componentSource = ctor ? ctor[2]! : trimmed; | ||
| const parts = [...componentSource.matchAll(/[+-]?(?:\d+\.?\d*|\.\d+)(?:e[+-]?\d+)?/gi)] |
There was a problem hiding this comment.
When the value is neither a finite scalar nor a recognized vecN constructor, this falls back to scanning the entire raw string for numbers. That makes invalid/non-numeric WGSL values with digits such as 10px, var(--scale-2), or calc(100% - 1px) compile as scalar/vector uniforms instead of being dropped or warned, which can add unintended fields and change the inferred struct layout; only scan a bare numeric component list, not arbitrary text.
Useful? React with 👍 / 👎.
…review)
Two real findings on this PR's own new code, from the bot re-review:
- vite: `parseWgslCastValue` now requires a pure numeric-list grammar on the
non-constructor path, so arbitrary text with stray digits (`10px`,
`calc(100% - 1px)`, `var(--scale-2)`) is dropped-and-warned instead of scanning
its digits into a false scalar/vector uniform (and silently changing the struct
layout).
- astro: the directive idempotence guard was swallowing the documented
`client:gpu={{ force: true }}` escape hatch when the plain-element scanner had
already bound gpu at a provisional tier (a scanner no-op that only defers to
detect-ready). A forced boot now clears that prior bind and re-enters;
`initGPUDirective` is re-entry-safe (its detect-ready upgrade path re-runs it).
Regression guards: `@wgsl 10px` is dropped + warned; gpu force boots after a prior
scanner bind (both non-vacuous). Verified: typecheck x3 / lint / format green;
tests/unit/astro 507 + vite boundary-manifest green; docs/api regenerated,
docs:check clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014wkz4TzwswnSCMVWQAngSd
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c25de8ff69
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (forced) { | ||
| unmarkBound(el, 'gpu'); | ||
| } |
There was a problem hiding this comment.
When the scanner reaches this element first on a provisional static/styled tier, initGPUDirective has already registered an onDetectReady retry before returning. If Astro then hydrates the same host with client:gpu={{ force: true }} (or data-czap-gpu-force) before detection settles, this unmarks and starts the shader immediately, but the earlier detect-ready callback is still armed and will call initGPUDirective(... force: true) again once the tier upgrades, creating a second canvas/render loop for the same element. The forced path needs to cancel or make the pending retry observe that a real GPU runtime has already started instead of just clearing the bound marker.
Useful? React with 👍 / 👎.
…dy retry fires Follow-up to the force-hatch fix. When the scanner bails at a provisional tier it arms an onDetectReady retry; a subsequent forced boot starts the shader immediately, but the armed retry would append a SECOND canvas / render loop once the tier upgrades. Track started shaders OFF the DOM in a WeakSet and skip the retry when the host already started. Regression test extended: a tier upgrade after a forced boot yields exactly one canvas. Verified: typecheck x3 / lint / format green; tests/unit/astro 507; docs:check clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014wkz4TzwswnSCMVWQAngSd
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 207b202535
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // generic (`vec2<f32>(...)`) form, capturing the DECLARED arity and the argument | ||
| // list. The generic `<...>` must be stripped whole, or its digits (`f32` -> 32) | ||
| // leak into the component scan and mis-shape the vector. | ||
| const ctor = /^vec([234])(?:[fiu]|<[^>]*>)?\s*\(([^)]*)\)$/i.exec(trimmed); |
There was a problem hiding this comment.
When an @wgsl value uses valid integer/unsigned constructor syntax such as vec2i(1, 2) or vec2<u32>(1, 2), this regex accepts it but only keeps the numeric components; the compiler later infers every array-valued uniform as vecNf, losing the declared scalar kind. That makes the generated preamble expose a float vector field for an integer vector authoring form, so shaders that use the uniform in integer contexts can fail WGSL validation or read the wrong type. Restrict this parser to the f32 forms it can actually represent, or carry the vector element type through compilation.
Useful? React with 👍 / 👎.
…ing implicit peer warnExplicitOnlyDirectiveAttributes suppressed the data-czap-boundary marker warning whenever ANY implicit directive attribute was present -- but an implicit peer like data-czap-shader-src (gpu) does NOT evaluate the boundary (only satellite/worker do). So a bare boundary sitting next to a gpu shader booted the shader, left the boundary inert, and stayed silent. Suppress only on an explicit directive marker. Verified: typecheck x3 / lint / format green; tests/unit/astro 508; docs:check clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014wkz4TzwswnSCMVWQAngSd
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a9df11bd1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| process.stderr.write(JSON.stringify(receipt) + '\n'); | ||
| for (const finding of payload.findings ?? []) { | ||
| process.stdout.write(JSON.stringify(finding) + '\n'); |
There was a problem hiding this comment.
When czap audit --findings is used by CI or a pipe like jq, this branch moves the documented command receipt to stderr and leaves stdout as zero-or-more individual finding objects (empty stdout on a clean run). packages/cli/README.md and receipts.ts both define the CLI contract as one JSON receipt on stdout, with --findings adding detail to that receipt, so machine consumers will stop seeing the summary/status line exactly when they opt into findings.
Useful? React with 👍 / 👎.
…t, empty-CSS warn, +3) Batch-fix the real findings from the CodeRabbit / Greptile / Codex review of PR #94: - vite (Major): parse the generic `vec2<f32>(...)` WGSL constructor correctly -- strip the whole `<...>` before the numeric scan (it was capturing `f32`'s `32`, turning `vec2<f32>(1,2)` into a `vec4f`) and validate the component count against the declared vecN, warning + dropping a malformed vector instead of shipping a silently-wrong offset. Upholds ADR-0029's own guarantee. - astro (P2): make `bootDirectiveEntry` idempotent + collision-aware. The plain-element scanner and Astro's island hydration both route through it; without the guard, an island that also carries the directive's implicit attribute was booted twice (duplicate EventSource / worker / shader). Collision detection moves here from the scanner so it fires for both boot paths; the scanner drops its now-redundant pre-mark. - cloudflare (P2): warn when a manifest boundary's pool serializes to no SERVABLE CSS (an @quantize block emitting only @wgsl/@glsl/@aria), not just when the pool is empty. - cli (P2): default `audit --pretty` off under `--findings`, so the per-finding lines don't duplicate the receipt JSON / NDJSON stream on a TTY. - edge: `tierDataAttributesMap` returns `Readonly<...>` to match the spine contract. - astro: `void` the bare `runtimeLoad()` in `wasmDirective` (unhandled-rejection guard). - docs: correct "worker/GPU" -> "satellite/worker" for `data-czap-boundary` in the CHANGELOG + ADR-0028 (the registry maps it to satellite + worker); regenerate docs/api. Deferred to its own change: sharing `scanProject` between the two boundary caches -- an HMR-sensitive signature/cache refactor for a marginal (one-scan-at-startup) win, not a safe pre-merge bang-out. Regression guards: WGSL generic/malformed parse test; `bootDirectiveEntry` idempotence + collision unit tests; the gpu shader-fetch test now uses a fresh host (re-booting the same element is correctly a no-op now). Verified: typecheck x3 / lint / format / build / check-invariants green; vite / cloudflare / edge / astro / cli / meta slices green; docs:check green; the e2e stream lib-bundle stays a single chunk. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014wkz4TzwswnSCMVWQAngSd
…review)
Two real findings on this PR's own new code, from the bot re-review:
- vite: `parseWgslCastValue` now requires a pure numeric-list grammar on the
non-constructor path, so arbitrary text with stray digits (`10px`,
`calc(100% - 1px)`, `var(--scale-2)`) is dropped-and-warned instead of scanning
its digits into a false scalar/vector uniform (and silently changing the struct
layout).
- astro: the directive idempotence guard was swallowing the documented
`client:gpu={{ force: true }}` escape hatch when the plain-element scanner had
already bound gpu at a provisional tier (a scanner no-op that only defers to
detect-ready). A forced boot now clears that prior bind and re-enters;
`initGPUDirective` is re-entry-safe (its detect-ready upgrade path re-runs it).
Regression guards: `@wgsl 10px` is dropped + warned; gpu force boots after a prior
scanner bind (both non-vacuous). Verified: typecheck x3 / lint / format green;
tests/unit/astro 507 + vite boundary-manifest green; docs/api regenerated,
docs:check clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014wkz4TzwswnSCMVWQAngSd
1 participant
0.6.0 — "make-it-loud, round 2" · PR 1 of 2 (the Front-Door Cut follows as PR 2, before the tag/cut).
A second dogfooding pass: every fix turns a silent degradation into a loud diagnostic through the existing
Diagnostics.warnOncechannel, unifies the Vite boundary-discovery seam, and adds vector WGSL uniforms so the WebGPU cast reaches GLSL parity. No engine-shape breaks; the one type widening is additive-shaped.Findings closed (all dogfood-sourced)
@czap/vite: the@quantizetransform resolves boundary names via the same module-graph discovery as the manifest; an unknown@quantize <name>is now a hard error (not a lightningcss "Unknown at rule"); a manifest-covered-but-empty boundary warns (manifest-boundary-empty-outputs) instead of silently serving empty CSS.@czap/audit:czap audit --consumerskips the internal structure pass entirely (was ~437 info findings on a consumer app);czap audit --findingsstreams parseable NDJSON to stdout.@czap/astro: COOP/COEP are consumer-overridable andcoepis configurable (landed onmainpre-branch,b5379bb2; included here for the ledger).@czap/quantizer:outputChangessurfacesglsl/wgsloutput tables alongsidecss.@czap/astro: client directives (client:stream/llm/gpu/wasm/graph) boot on plain elements, not only framework islands, via aDIRECTIVE_ATTRIBUTE_REGISTRY-derived scanner; a baredata-czap-boundarywarns instead of silently doing nothing. See ADR-0028.@czap/compiler+@czap/astro: vector WGSL uniforms (vec2/vec3/vec4), a declaration-derived uniform-buffer layout, and awgsl-uniform-buffer-fulloverflow warning.u_resolutionis a realvec2<f32>; closes the GLSL/WGSL parity gap. See ADR-0029.@czap/astro:driveUniformFromSignalcontinuous-signal tracking on theSharedArrayBufferworker path is pinned by a non-vacuous gate.@czap/edge:EdgeTier.tierDataAttributesMap/EdgeHostResolution.htmlAttributesMap— the spreadabledata-czap-*map form, built from theCAP_AXESregistry.Docs
[0.6.0]entry + a retroactive[0.4.0]shader-SRI Breaking/migration note (B2 — a breaking change that shipped in 0.4.0 undocumented).Release mechanics
0.5.0 → 0.6.0+create-liteshiptemplate refs.Test plan
Local-safe suite green:
build,typecheck×3,lint,check-invariants, full Vitest 7064 passed / 7 skipped. The 7 skips are the wasm-absent-local parity tests — CI builds the wasm and runs them.build:wasmis intentionally not run locally (it's the pre-publish step); CI / the gauntlet exercises it.🤖 Generated with Claude Code
https://claude.ai/code/session_014wkz4TzwswnSCMVWQAngSd
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Greptile Summary
This PR ("make-it-loud round 2") delivers a second dogfooding pass: silent runtime degradations are replaced with
Diagnostics.warnOncediagnostics, the Vite boundary-discovery seam is unified, and WGSL uniforms gainvec2/vec3/vec4scalar/vector support to close the GLSL parity gap. All 25 packages are bumped0.5.0 → 0.6.0.directive-bound.tsis extracted as a dependency-free leaf so each client-directive runtime can callbootDirectiveEntrywithout dragging the scanner's code-splitLOADERSgraph into its bundle; aDIRECTIVE_ATTRIBUTE_REGISTRY-derived scanner adds plain-element implicit boot anddata-czap-boundary-without-marker warnings.WGSLUniformValue = number | WGSLUniformVectorpropagates through the compiler, boundary payload, wgpu runtime, and worker change-tracker;parseWgslCastValueinboundary-manifest.tshandlesvecNf(...)/vec<N><f32>(...)CSS-authored constructor syntax and drops malformed values loudly.@quantizeboundaries throwValidationError; consumer audit skips the 437-finding structure pass;--findingsstreams NDJSON to stdout;EdgeTier.tierDataAttributesMapexposes a spreadabledata-czap-*attribute map.Confidence Score: 5/5
Safe to merge. All changed paths are covered by new tests; the 7064-pass suite is green with only pre-existing wasm-absent-local skips.
Every changed subsystem has dedicated unit, integration, or property tests: directive boot idempotence, vec2/vec3/vec4 round-trip through the compiler and wgpu buffer, malformed @WGSL vector rejection, consumer audit mode, NDJSON findings output, and edge attribute map. The WGSL buffer widening (32→64 bytes) is validated by the updated overflow test. The directive-bound.ts extraction is a pure dependency restructuring with no semantic change to the bound-marker protocol. No removal of existing guards; only additive changes and loud-on-error replacements of previously-silent paths.
No files require special attention. The parseWgslCastValue regex path in packages/vite/src/boundary-manifest.ts handles the most novel parsing logic and is covered by the @WGSL parses the generic vec2(...) form integration test.
Important Files Changed
Comments Outside Diff (8)
packages/cli/src/commands/audit.ts, line 93-113 (link)--findings+ TTYWhen
opts.findingsistrueandprocess.stderr.isTTYis truthy (interactive terminal, no explicit--pretty=false),wantPrettyresolves totrue. The code then writes the full receipt JSON to stderr at line 94 (which embeds every finding as JSON), and a second time as formatted text lines at 108–113. A user runningczap audit --findingsin their terminal will see each finding appear twice on stderr — once buried in the receipt blob, once in the human-readable lines — with the machine-readable stdout being the only clean channel.Consider defaulting
wantPrettytofalsewhenopts.findingsis set (or guarding the line 94 receipt write behind!wantPretty) so the two output modes don't overlap.Prompt To Fix With AI
packages/astro/src/runtime/directive-boot.ts, line 84-93 (link)The message for
directive-attribute-requires-markerhardcodes"satellite" or "worker"as the fix suggestion. This comes from the registry-derived logic correctly coveringdata-czap-boundaryfor both directives, but if a third directive ever adoptsdata-czap-boundarywithimplicitBoot: false, the message would still say "satellite or worker" while the fix might also include the new directive name. Generating the suggestion text from the registry entry — e.g. by joining the directive keys whosedata-czap-boundaryentry hasimplicitBoot: false— would keep the message self-maintaining.Prompt To Fix With AI
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
General comment
@quantizetransform resolves boundary names through the same discovery as the manifest. In the head run, a boundary exported fromsrc/lib/boundaries.tsshould be discovered by the project scan and compiled, but the transform hard-errors:boundary "layout" referenced in @quantize not found. This leaves valid manifest-covered boundaries unusable in CSS transforms.packages/vite/src/transform-css.ts:298-305treats absence fromctx.boundaryDefinitionsas a hard error. The suppliedboundaryDefinitionsare produced bycollectBoundaryDefinitions, which only collects files matchingboundaries.ts/*.boundaries.ts; the transform therefore still rejects a boundary that the intended manifest/module-graph discovery contract should accept.boundaryDefinitionssource exactly match manifest discovery for supported boundary modules, including nested project boundary modules, or fall back to the manifest entry before throwing. Add a regression test that exportslayoutfrom a nested boundary module, verifiescollectBoundaryManifestcontains it, and verifiesplugin.transformcompiles@quantize layoutto container CSS.General comment
Diagnostics.warnOncecodemanifest-boundary-empty-outputs. In the head run,collectBoundaryManifestreturned the expected empty entry (outputs: [],outputsByTier: {}), but the captured diagnostics array was empty.packages/vite/src/boundary-manifest.ts:614-621constructs empty entries with{ outputs: [], outputsByTier: {} }when no states exist, but does not emit the claimedDiagnostics.warnOncediagnostic for that condition.Diagnostics.warnOncewith sourceczap/vite.boundary-manifest(or the existing source constant), codemanifest-boundary-empty-outputs, and a message identifying the boundary/export and explaining that no@quantizeoutput was generated. Ensure warn-once keys include the boundary/source so duplicate loads do not spam.General comment
czap audit --findingsstreams parseable NDJSON to stdout. Runtime evidence on head shows stdout contains one audit receipt JSON object with metadata and a nestedfindingsarray. This is not an NDJSON findings stream: consumers expecting one finding object per line cannot process it incrementally and must parse the whole receipt.packages/cli/src/commands/audit.ts, the command always buildsreceiptand callsemit(receipt)at line 93. Theopts.findingsflag only causes findings to be included in the payload earlier; it does not switch stdout emission to per-finding NDJSON records.opts.findingsis true, write eachpayload.findingsentry asJSON.stringify(finding) + ' 'to stdout instead of emitting the aggregate receipt, and keep any human summary on stderr.General comment
offset: f32,normal: f32,color: f32, andu_resolution: f32. This violates the claimed contract that vector WGSL uniforms are supported and that u_resolution is emitted as a real vec2/vec2f uniform.Record<string, number>, stores fieldValues asnumber[], and passes potentially array-valued entries into scalar-onlyinferStableWGSLType. The scalar inference at lines 100-102 treats arrays as non-integer values and returnsf32, so all vector-valued fields collapse to scalar f32 in the struct declaration.WGSLUniformValue[], and updateinferStableWGSLTypeto detect array lengths and return vec2f/vec3f/vec4f before scalar promotion. Ensure dispatch/index exports use the same scalar/vector contract.General comment
General comment
collectBoundaryManifestwithoutputs: []is silently skipped during build asset generation. The runtime artifact showsBUILD_START warnings=[] emitted=[]even though the manifest entry is covered and has no outputs. The claimed contract says this should warn once withmanifest-boundary-empty-outputs.packages/vite/src/plugin.ts,ensureBoundaryAssetsstill usesif (entry.outputs.length === 0) continue;without emitting any diagnostic before skipping the entry.manifest-boundary-empty-outputsdiagnostic code/message, and ensure it is only emitted once per boundary/build as intended.Reviews (7): Last reviewed commit: "fix(astro): warn for a bare boundary pay..." | Re-trigger Greptile