Web build pipeline fixes for #148

[beaucollins]

Fixes found while integrating #148's web build into a downstream consumer. Based on #148.

Fixes

• Web output-flavor declaration matches the compiler's web_output_target (valdi_compiled.bzl): a module emits/consumes web/release only when both the android and ios output targets are release and the build isn't forced to debug; otherwise web/debug. Keying off output_flavor alone mis-declared web/release outputs for debug-target modules (e.g. jasmine) that the compiler only ever writes as web/debug.
• Register transitive module res directories (CollapseWebPaths.cpp): recursively collect every module res directory and break out of the per-directory image scan so transitively-staged res dirs are still registered. Previously only immediate children of src/ were scanned, so transitive-dependency images were bundled but never registered (blank icons at runtime).
• Skip non-web sources in the native collapse pass (valdi_collapse_web_paths.bzl): source files with no web/ segment fall back to a raw path that would escape the -o output directory and fail under a strict sandbox; skip them (they are placed by the main collapse pass) and preserve real module names for compiler-generated web/<flavor>/res.
• Preserve leading .. in the path shim (web_path_browserify_shim.js.tpl): relative-path normalization no longer drops leading parent-directory segments.
• Custom bytes loader instead of the non-existent asset/bytes (web_webpack.config.js.tpl + new web_bytes_loader.js.tpl + wiring in valdi_web_application.bzl / app_templates/BUILD.bazel): webpack has no built-in byte-array module type, so .bin/.protodecl resources are emitted as Uint8Array via a custom loader.
• Synchronize hot-reload lifecycle writes (HotReloadLifecycleReporter.swift): events are emitted from multiple queues; an NSLock serializes writes so concurrent lines are not interleaved.
• Docs: custom-view one-mechanism guidance (native-customviews.md, custom-view skill): a <custom-view> binds its native view via exactly one mechanism — a viewFactory or a platform class name — never both.

[github-actions]

Sensitive Files Detected

🔧 Build rules — Affects build rules for all Valdi consumers.

_{This is an automated notice. A maintainer will review after import.}

[github-actions]

📊 PR Size: size/M

Total changes: 235 lines (12 files)

Top files changed:

• valdi/compiler/toolbox/src/valdi/compiler_toolbox/CollapseWebPaths.cpp: +58 -15
• bzl/valdi/valdi_compiled.bzl: +30 -22
• bzl/valdi/valdi_collapse_web_paths.bzl: +17 -30
• bzl/valdi/app_templates/web_bytes_loader.js.tpl: +13 -0
• docs/docs/native-customviews.md: +5 -6
• compiler/compiler/Compiler/Sources/Reloader/HotReloadLifecycleReporter.swift: +9 -1
• bzl/valdi/valdi_web_application.bzl: +9 -0
• valdi/compiler/toolbox/test/CollapseWebPaths_tests.cpp: +5 -2
• bzl/valdi/app_templates/web_path_browserify_shim.js.tpl: +5 -1
• bzl/valdi/app_templates/web_webpack.config.js.tpl: +3 -1

...and 2 more files

_{Size calculated as additions + deletions. Labels: XS (<10), S (<50), M (<250), L (<1000), XL (1000+)}

[github-actions]

⚠️ Bazel & CI Test Results

Test Suite	Result
Valdi Smoke Tests	✅ success
API Surface Check	✅ success
valdi_web Integration Test	❌ failure
Linux: C++ Tests	❌ failure
Snapshot Tests	✅ success
Linux: Build & Export	✅ success
Linux: Build Compiler	✅ success
Test Coverage Delta	❌ failure
macOS: C++ & Platform Tests	❌ failure
Linux: Hotreload Smoke	✅ success
Linux: Registry Validation	✅ success
Linux: Module Tests	✅ success

Some tests failed. Please check the workflow logs for details.

🚀 Bazel remote cache is now enabled - future builds will be faster!

_{Workflow: Valdi CI}

[beaucollins]

CI failure triage

None of the current failures are introduced by this PR. Comparing against the base branch's own most recent bzl-changes run:

Job	Base branch	This PR	Attribution
valdi_web Integration Test (experiments/helloworld_playground:integration_test)	failing	failing	Pre-existing on base
macOS C++ & Platform Tests (//valdi:valdi_ios_swift_test → AsyncValdiRuntimeTests.testWarmGetRuntimeWithoutInlineDeliveryStaysAsynchronous)	failing	failing	Pre-existing on base
Test Coverage Delta	failing	failing	Pre-existing on base
Linux: C++ Tests	passing	failing	Flaky timeout, not from this diff (see below)

Linux: C++ Tests is the only job that differs from base. It's a test-target timeout — the runtime unit-test binary hung on RuntimeTests/RuntimeFixture.AsyncStrictModeSyncCallAssertsOnMainThread/QuickJS (-- Test timed out --), an async / main-thread-assertion runtime test.

This PR's diff touches nothing linked into that binary:

• CollapseWebPaths.cpp builds the valdi_compiler_toolbox build tool ([for tool]), not the runtime.
• HotReloadLifecycleReporter.swift is the reloader (Swift), not a Linux C++ gtest.
• DiskUtils.* / PathUtils.* are the only runtime C++ files in range and they are byte-identical to the base branch (no diff).

So that binary is identical to base (which passed) — a timing-flaky timeout on this run, not a regression. A re-run of Linux: C++ Tests should go green.

The three pre-existing failures are unrelated to these build-pipeline fixes and also fail on the base branch.

[scorsin-oai]

Thanks for tracking these down. Several changes address real gaps in #148, but there are a few parts I would revise before merging. I reviewed all 12 changed files at df12566d against 745e8f38 and ran focused before/after path checks; I have not built this complete branch.

Please preserve the external-repository path normalization

Removing _repository_relative_short_path() reverses 4684e767, which fixed external-repository packaging. For paths that reach the fallback in _dest(), this reintroduces destinations outside the package directory. A concrete before/after check:

Input:       ../icons+/assets/license.txt
Base:        assets/license.txt
This PR:     ../icons+/assets/license.txt

copyManifest() appends that destination to the output directory and normalizes it, so the proposed result escapes the output directory and can fail under the sandbox or omit the file from the package.

The declaration-condition reversal also changes an in-repository src/valdi_modules/src/valdi/valdi_core/src/Asset.d.ts from the canonical src/valdi_core/src/Asset.d.ts back to its full source-tree path in this pass. The separate declaration-copy pass may supply the canonical copy, but the mapping regression should still be avoided.

Skipping declarations that don't belong in the native-copy pass can be handled separately. Please retain repository normalization in the package/native mapping and registration paths.

asset/bytes exists; the Webpack versions differ

Webpack supports asset/bytes, exporting a Uint8Array. The Bazel lockfile in #148 (bzl/valdi/npm/pnpm-lock.yaml) resolves Webpack 5.104.1, whose implementation includes the bytes parser and generator. However, compiler/companion/package-lock.json still resolves 5.90.3, which lacks that support.

This explains why our Bazel builds can work while an older consumer bundler fails. Could you confirm the Webpack version selected by the failing downstream build? I would prefer aligning the supported dependency versions and documenting the minimum requirement while retaining direct .bin imports. The proposed loader preserves the bytes API, but we should not replace built-in support based on the claim that it doesn't exist.

Keep the custom-view cleanup documentation

The clarification that viewFactory and platform class attributes cannot coexist is correct; JSXProcessor explicitly rejects that combination.

The separate removal of destroy() and the | void factory return type is not correct. WebViewClassRegistry.ts supports both, and CustomViewElementClass.ts registers the optional destroy() callback for cleanup. Please keep those existing parts of the documentation.

Other fixes and the testing gaps

• Web debug output selection: This looks appropriate. At the current base, release-only gating omits Web outputs for debug-only modules such as Jasmine; matching declarations and exported providers to the compiler's selected flavor addresses that. Our snapshot application uses release-capable modules, and the renderer unit suite does not test packaging Jasmine into a browser application.
• Compiled image-resource placement: The _dest() correction is valid. A path ending in web/release/res/icons/res/icon.svg currently maps to src/release/res/icons/res/icon.svg; this change correctly maps it to src/icons/res/icon.svg. Our integration module uses inline_assets = True, which provides original resources at the canonical location. Its separate web_no_inline_images = True setting tests Webpack's final image emission, not this compiler-output layout. Please add coverage for a transitive module using compiler-emitted resources.
• Recursive image discovery: This appears to compensate for the malformed layout fixed by the path change above. Once placement is corrected, is there still a supported case requiring the recursive scan? It would be useful to demonstrate that case before keeping the extra traversal and duplicate-selection logic.
• Leading .. in the browser path shim: Valid fix. I reproduced normalize('../assets/icon.svg') incorrectly becoming assets/icon.svg; the proposed behavior matches Node for the cases checked.
• Declaration import('…') rewriting: Valid addition. The existing fixture covered ordinary imports only; the new typeof import(...) assertion covers the missing form.
• Hot-reload output locking: Reasonable given the callbacks from different queues. Our existing reporter test emits events sequentially, so it does not cover concurrent writes.

These expose gaps in packaging/configuration coverage that passing renderer snapshots would not catch. The targeted fixes are useful; please preserve the existing path and cleanup behavior and resolve the Webpack-version question before merging.

[beaucollins]

Thanks for the careful review — all four points addressed in a9f172c.

External-repository path normalization — restored _repository_relative_short_path() and the is_valdi_source_path flag, so the package/native mapping and registration passes normalize external paths again (no fallback destinations escaping the package dir, and in-repo .d.ts keep their canonical src/<module>/src/... mapping). The two behavioral fixes now run over the normalized path: the _dest() skip for compiler-generated web/<flavor>/res dirs, and skipping non-web/ sources in the native pass.

asset/bytes — you're right, and confirmed: the Bazel web build resolves webpack 5.104.1, whose bytes parser/generator is present, so asset/bytes works. Reverted the .bin/.protodecl rule back to type: 'asset/bytes' and removed the custom loader. Note the companion package-lock.json still resolves 5.90.3, but that bundler is only on the hot-reload path, not this one — happy to align/pin it and document a minimum separately if you'd like.

Custom-view docs — kept the one-mechanism (viewFactory xor platform-class) clarification, and restored the destroy?() callback and the AttributeHandler | void factory return type, since WebViewClassRegistry / CustomViewElementClass support both.

Recursive image discovery — demonstrated it's unnecessary once placement is corrected, and reverted it. With the _dest() fix, every module's res (direct and transitive) stages at the canonical src/<module>/res — a direct child of src/, which the original non-recursive scan already finds. Built a downstream web app whose graph includes transitive image-bearing dependencies and diffed the generated _image_registry.js: byte-identical with the non-recursive scan. So the recursive traversal + duplicate-selection logic was dead code and is gone. Kept the declaration import('…') rewrite and its test — those are unrelated and correct.

Verified the downstream web targets still build and both browser integration tests pass with --define enable_web=true (local-compiler build).

[scorsin-oai]

I hereby approve of this PR