Add Assets multi-file upload to project storage and fix review diff encoding - PR_26163_074-assets-multi-file-upload-project-write & Add Assets multi-file upload to project storage with progress diagnostics - PR_26163_074-assets-multi-file-upload-project-write

Author: ToolboxAid

docs_build/dev/reports/PR_26163_074-assets-multi-file-upload-project-write.md

@@ -0,0 +1,148 @@
+# PR_26163_074-assets-multi-file-upload-project-write
+
+## Branch Validation
+
+- Current branch: `main`
+- Expected branch: `main`
+- Branch validation: PASS
+
+## Summary
+
+Assets now supports multi-file upload for upload-capable asset types: Images, Audio, Fonts, and Data. Each valid selected file creates a separate owner-scoped asset record, the File column uses the actual uploaded filename, and each stored path is generated under the current project path:
+
+- `projects/<projectId>/image/<filename>`
+- `projects/<projectId>/audio/<filename>`
+- `projects/<projectId>/font/<filename>`
+- `projects/<projectId>/data/<filename>`
+
+Multi-file uploads now open an in-page upload diagnostics dialog that updates during the batch. It shows the current file, file count progress, bytes uploaded, total bytes, BPS, human-readable speed, ETA, elapsed time, per-file OK/WARN/FAIL/SKIP rows, and written/failed/skipped/warnings summary counters.
+
+Sprite, Vector, and Palette References remain Reference-only for MVP. Projectile remains absent from usage values. Shared Tags type-ahead behavior is unchanged.
+
+## Requirement Checklist
+
+| Requirement | Status | Evidence |
+| --- | --- | --- |
+| Read `docs_build/dev/PROJECT_INSTRUCTIONS.md` first | PASS | Read before implementation. |
+| Verify branch is `main` before changes | PASS | `git branch --show-current` returned `main`. |
+| Fix `codex_review.diff` output encoding as readable UTF-8 | PASS | Regenerated via `node scripts/write-codex-review-artifacts.mjs`; Playwright test `codex review diff artifact is readable UTF-8 text` passed. |
+| Multi-file upload for Images | PASS | Playwright selected two Image files and verified two rows plus `projects/.../image/...` path. |
+| Multi-file upload for Audio | PASS | Playwright selected two Audio files and verified two rows plus `projects/.../audio/...` path. |
+| Multi-file upload for Fonts | PASS | Playwright selected two Font files and verified two rows plus `projects/.../font/...` path. |
+| Multi-file upload for Data | PASS | Playwright selected two Data files and verified two rows plus `projects/.../data/...` path. |
+| File input supports multiple files where upload is allowed | PASS | Playwright asserts `Upload File` has `multiple = true` for upload rows. |
+| Each valid selected file creates one asset record row | PASS | Playwright validates one row per valid selected file for Images, Audio, Fonts, and Data. |
+| Uploaded file paths stay under `/projects/` with current project id and asset type | PASS | Playwright validates stored paths matching `projects/<ULID>/image/`, `audio/`, `font/`, and `data/`. |
+| Do not write files outside `/projects/` | PASS | Catalog upload stored paths are generated by `catalogUploadStoredPath()` using only `projects/<projectId>/<asset-role-folder>/<filename>`. |
+| Do not generate fake filenames | PASS | UI and repository use the selected `File.name`; tests verify actual filenames render in rows and metadata. |
+| Asset record stores generated project-relative path | PASS | Asset metadata and batch log show `storedPath` values under `projects/...`. |
+| Batch upload logs OK/WARN/FAIL/SKIP | PASS | Playwright diagnostic batch verifies all four statuses render in `data-asset-tool-batch-log`. |
+| Upload dialog opens for multi-file upload | PASS | Playwright verifies `data-asset-tool-upload-dialog` becomes visible after Save. |
+| Progress updates while upload is running | PASS | Playwright verifies the dialog phase reaches `Uploading` before the final complete state. |
+| Current file and file count progress render | PASS | Playwright verifies current file text and `/ 2` file count progress during upload. |
+| Bytes uploaded and total bytes render | PASS | Playwright verifies total bytes and bytes uploaded are not `0 B` during the clean batch. |
+| BPS, speed, ETA, and elapsed render | PASS | Playwright verifies BPS is nonzero, speed contains `/s`, and ETA/elapsed show seconds. |
+| Per-file OK/WARN/FAIL/SKIP rows render in dialog | PASS | Playwright verifies dialog rows for `OK`, `WARN`, `FAIL`, and `SKIP`. |
+| Batch summary renders written/failed/skipped/warnings | PASS | Playwright verifies dialog counters for written, failed, skipped, and warnings. |
+| One failed file does not stop remaining uploads unless global | PASS | Playwright diagnostic batch includes a bad extension FAIL followed by a later OK row. |
+| No silent fallback when progress cannot be calculated | PASS | Zero-byte upload produces a visible dialog WARN row: progress could not be calculated. |
+| Missing project id fails visibly | PASS | Playwright `?handoff=missing` run shows FAIL and SKIP log rows and leaves asset count at `0`. |
+| Sprite, Vector, Palette References remain Reference-only | PASS | Playwright verifies each has `Source = Reference` and no Upload File control. |
+| Projectile remains removed from Usage values | PASS | Existing Assets coverage still asserts usage options equal the approved list and exclude Projectile. |
+| Shared Tags type-ahead preserved | PASS | Existing Assets coverage still adds a shared Tag and uses it through the Assets tag datalist. |
+| Records remain owner-scoped | PASS | Existing Assets coverage still verifies user 2 cannot see user 1's asset row. |
+
+## Batch Upload Evidence
+
+- Clean Image batch: `2 written, 0 failed, 0 skipped, 0 warnings`.
+- Diagnostic Image batch: `3 written, 1 failed, 1 skipped, 1 warnings`.
+- Upload dialog clean-batch final counters:
+  - Written: `2`
+  - Failed: `0`
+  - Skipped: `0`
+  - Warnings: `0`
+- Upload dialog diagnostic-batch final counters:
+  - Written: `3`
+  - Failed: `1`
+  - Skipped: `1`
+  - Warnings: `1`
+- Upload progress fields verified:
+  - Current File
+  - File Count
+  - Bytes Uploaded
+  - Total Bytes
+  - BPS
+  - Speed
+  - ETA
+  - Elapsed
+- Logged per-file statuses:
+  - `OK: batch-status-ok.png`
+  - `WARN: batch-status-warning.png`
+  - `FAIL: batch-status-bad.exe`
+  - `SKIP: batch-status-ok.png`
+- Missing project id batch: `0 written, 1 failed, 1 skipped, 0 warnings`.
+
+## Project Path Evidence
+
+Playwright verified selected asset metadata with these path patterns:
+
+- `projects/<ULID>/image/batch-image-a.png`
+- `projects/<ULID>/audio/batch-audio-a.wav`
+- `projects/<ULID>/font/batch-font-a.woff2`
+- `projects/<ULID>/data/batch-data-a.json`
+
+## Changed Files
+
+- `toolbox/assets/index.html`
+- `toolbox/assets/assets.js`
+- `src/dev-runtime/persistence/tool-repositories/assets-mock-repository.js`
+- `tests/playwright/tools/AssetToolMockRepository.spec.mjs`
+- `docs_build/dev/reports/playwright_v8_coverage_report.txt`
+- `docs_build/dev/reports/codex_review.diff`
+- `docs_build/dev/reports/codex_changed_files.txt`
+- `docs_build/dev/reports/PR_26163_074-assets-multi-file-upload-project-write.md`
+
+## Impacted Lanes
+
+- runtime: Assets browser tool behavior.
+- integration: Assets DB/mock adapter path generation and owner-scoped records.
+- Playwright: Yes, targeted Assets coverage updated.
+
+## Testing Performed
+
+- PASS: `node --check toolbox/assets/assets.js`
+- PASS: `node --check src/dev-runtime/persistence/tool-repositories/assets-mock-repository.js`
+- PASS: `node --check tests/playwright/tools/AssetToolMockRepository.spec.mjs`
+- PASS: `git diff --check -- toolbox/assets/index.html toolbox/assets/assets.js src/dev-runtime/persistence/tool-repositories/assets-mock-repository.js tests/playwright/tools/AssetToolMockRepository.spec.mjs docs_build/dev/reports/playwright_v8_coverage_report.txt`
+- PASS: `npx playwright test tests/playwright/tools/AssetToolMockRepository.spec.mjs --workers=1 --reporter=list` (6 passed; final run includes upload dialog/progress diagnostics)
+- PASS: `npm run test:workspace-v2` (workspace-contract lane, 5 passed)
+
+Initial targeted Playwright intentionally failed before artifact regeneration because the pre-existing `codex_review.diff` was not readable UTF-8. After regenerating with the Node artifact writer, the same lane passed.
+
+## V8 Coverage
+
+- PASS: `docs_build/dev/reports/playwright_v8_coverage_report.txt` generated by the final targeted Assets Playwright run.
+- PASS: `toolbox/assets/assets.js` covered at 96% function coverage.
+- WARN: `src/dev-runtime/persistence/tool-repositories/assets-mock-repository.js` is server-side mock adapter code and is not collected directly by browser V8 coverage; it is exercised through the API-backed Playwright flow.
+
+## Skipped Lanes
+
+- Full samples smoke: SKIPPED by explicit request. Safe to skip because this PR only changes the Assets tool UI, the Assets mock adapter path generation, targeted Assets Playwright tests, and reports. No sample loader/runtime game sample behavior changed.
+
+## Manual Validation Steps
+
+1. Open `toolbox/assets/index.html`.
+2. Click `Reset Asset Library`.
+3. Open `Images`, click `Add Images`, choose multiple image files, select Usage, and Save.
+4. Confirm one row appears per valid selected file and the Status Log shows a batch summary.
+5. Confirm the Upload Diagnostics dialog shows current file, file count, bytes uploaded, total bytes, BPS, speed, ETA, elapsed time, per-file statuses, and written/failed/skipped/warnings counters.
+6. Click `View` for an uploaded asset and confirm `Stored path` starts with `projects/<projectId>/image/`.
+7. Repeat for Audio, Fonts, and Data and confirm the matching `audio`, `font`, and `data` path folders.
+8. Open `Sprites`, `Vectors`, and `Palette References` and confirm they remain Reference-only.
+9. Open `toolbox/assets/index.html?handoff=missing`, attempt a multi-file Image upload, and confirm the visible FAIL/SKIP batch log with zero saved rows.
+
+## Review Artifacts
+
+- `docs_build/dev/reports/codex_review.diff`
+- `docs_build/dev/reports/codex_changed_files.txt`
+- `tmp/PR_26163_074-assets-multi-file-upload-project-write_delta.zip`

docs_build/dev/reports/codex_changed_files.txt

@@ -1,7 +1,22 @@
-docs_build/dev/reports/PR_26163_073-assets-source-rules-followup.md
-docs_build/dev/reports/codex_review.diff
-docs_build/dev/reports/playwright_v8_coverage_report.txt
-src/dev-runtime/persistence/tool-repositories/assets-mock-repository.js
-tests/playwright/tools/AssetToolMockRepository.spec.mjs
-toolbox/assets/assets.js
-docs_build/dev/reports/codex_changed_files.txt
+# git status --short
+M docs_build/dev/reports/codex_changed_files.txt
+ M docs_build/dev/reports/codex_review.diff
+ M docs_build/dev/reports/playwright_v8_coverage_report.txt
+ M src/dev-runtime/persistence/tool-repositories/assets-mock-repository.js
+ M tests/playwright/tools/AssetToolMockRepository.spec.mjs
+ M toolbox/assets/assets.js
+ M toolbox/assets/index.html
+?? docs_build/dev/reports/PR_26163_074-assets-multi-file-upload-project-write.md
+
+# git ls-files --others --exclude-standard
+docs_build/dev/reports/PR_26163_074-assets-multi-file-upload-project-write.md
+
+# git diff --stat
+docs_build/dev/reports/codex_changed_files.txt     |  26 +-
+ docs_build/dev/reports/codex_review.diff           | Bin 39852 -> 53126 bytes
+ .../dev/reports/playwright_v8_coverage_report.txt  |  43 ++-
+ .../tool-repositories/assets-mock-repository.js    |  22 +-
+ .../tools/AssetToolMockRepository.spec.mjs         | 186 ++++++++++++
+ toolbox/assets/assets.js                           | 330 ++++++++++++++++++++-
+ toolbox/assets/index.html                          |  42 +++
+ 7 files changed, 620 insertions(+), 29 deletions(-)

docs_build/dev/reports/codex_review.diff

@@ -1,14 +1,39 @@
 Playwright V8 Coverage Report
 
-PR: PR_26163_073-assets-source-rules-followup
-Coverage source: targeted Playwright/Chromium V8 coverage from tests/playwright/tools/AssetToolMockRepository.spec.mjs.
+PR: generated by current Playwright run.
+Coverage source: Playwright/Chromium built-in V8 coverage.
+Coverage scope: all repo-relative browser JavaScript collected by Playwright/Chromium V8 coverage.
 Dependencies: no new npm packages.
-Thresholds: none enforced; coverage is advisory.
-Result: PASS/WARN. Browser runtime coverage was collected for the changed Assets UI module; dev-runtime repository code is covered by assertions and API behavior but is not browser-collected V8 code.
+Thresholds: none enforced.
+Note: coverage is an advisory baseline only for this PR.
+Note: missing changed runtime JS coverage is reported as WARN, not FAIL.
+Note: line counts are V8 range-based and advisory; function counts show partial module exercise where available.
+Note: entry percentages use function coverage when available, otherwise line coverage.
+Note: coverage entries are aggregated across every page/tool where coverageReporter.start(page) and coverageReporter.stop(page) ran.
 
-Changed runtime JavaScript files:
-(96%) toolbox/assets/assets.js - covered by targeted Assets Playwright; V8 reported executed lines 725/725 and executed functions 69/72.
-(0%) src/dev-runtime/persistence/tool-repositories/assets-mock-repository.js - WARN: dev-runtime repository code is exercised by Node-side repository assertions and browser behavior through the API adapter, but it is not browser runtime JavaScript collected by Playwright V8 coverage.
+Exercised tool entry points detected:
+(92%) Toolbox Index - exercised 5 runtime JS files
+(0%) Tool Template V2 - not exercised by this Playwright run
+(57%) Theme V2 Shared JS - exercised 2 runtime JS files
 
-Changed non-runtime validation files:
-(0%) tests/playwright/tools/AssetToolMockRepository.spec.mjs - Playwright spec updated for PR073 behavior; test files are not browser runtime coverage targets.
+Changed runtime JS files covered:
+(0%) src/dev-runtime/persistence/tool-repositories/assets-mock-repository.js - WARNING: changed runtime JS file was not collected by Playwright V8 coverage; advisory only
+(96%) toolbox/assets/assets.js - executed lines 1018/1018; executed functions 94/98
+
+Files with executed line/function counts where available:
+(53%) src/engine/api/server-api-client.js - executed lines 159/159; executed functions 10/19
+(56%) assets/theme-v2/js/gamefoundry-partials.js - executed lines 650/650; executed functions 30/54
+(64%) assets/theme-v2/js/tool-display-mode.js - executed lines 209/209; executed functions 9/14
+(67%) toolbox/assets/assets-api-client.js - executed lines 19/19; executed functions 2/3
+(75%) toolbox/tool-registry-api-client.js - executed lines 152/152; executed functions 21/28
+(96%) toolbox/assets/assets.js - executed lines 1018/1018; executed functions 94/98
+(100%) toolbox/tags/tags-api-client.js - executed lines 10/10; executed functions 2/2
+(100%) toolbox/tags/tags.js - executed lines 242/242; executed functions 19/19
+
+Uncovered or low-coverage changed JS files:
+(0%) src/dev-runtime/persistence/tool-repositories/assets-mock-repository.js - WARNING: uncovered changed runtime JS file; advisory only
+
+Changed JS files considered:
+(0%) src/dev-runtime/persistence/tool-repositories/assets-mock-repository.js - changed JS file not collected as browser runtime coverage
+(0%) tests/playwright/tools/AssetToolMockRepository.spec.mjs - changed JS file not collected as browser runtime coverage
+(96%) toolbox/assets/assets.js - changed JS file with browser V8 coverage

docs_build/dev/reports/playwright_v8_coverage_report.txt

@@ -39,7 +39,7 @@ export const ASSET_ROLE_DEFINITIONS = Object.freeze([
     inputMode: "file",
     maxSizeBytes: 52428800,
     usageRoles: ["sound", "music"],
-    validationNeeds: ["MIME must be audio/* or approved audio MIME", "File size must be greater than zero", "Project storage path must be generated under projects/<projectId>/audio/<usage>/"]
+    validationNeeds: ["MIME must be audio/* or approved audio MIME", "File size must be greater than zero", "Project storage path must be generated under projects/<projectId>/audio/"]
   },
   {
     id: "data",
@@ -52,7 +52,7 @@ export const ASSET_ROLE_DEFINITIONS = Object.freeze([
     inputMode: "file",
     maxSizeBytes: 5242880,
     usageRoles: ["config", "table"],
-    validationNeeds: ["Data files must use .json, .csv, or .txt", "Structured data must declare format"]
+    validationNeeds: ["Data files must use .json, .csv, or .txt", "Project storage path must be generated under projects/<projectId>/data/"]
   },
   {
     id: "font",
@@ -65,7 +65,7 @@ export const ASSET_ROLE_DEFINITIONS = Object.freeze([
     inputMode: "file",
     maxSizeBytes: 10485760,
     usageRoles: ["ui", "display"],
-    validationNeeds: ["Font files must use approved font formats", "Project storage path must be generated under projects/<projectId>/font/<usage>/"]
+    validationNeeds: ["Font files must use approved font formats", "Project storage path must be generated under projects/<projectId>/font/"]
   },
   {
     id: "image",
@@ -78,7 +78,7 @@ export const ASSET_ROLE_DEFINITIONS = Object.freeze([
     inputMode: "file",
     maxSizeBytes: 10485760,
     usageRoles: ["sprite", "background", "bezel", "preview", "ui"],
-    validationNeeds: ["MIME must be image/* or approved image MIME", "File size must be greater than zero", "Project storage path must be generated under projects/<projectId>/image/<usage>/"]
+    validationNeeds: ["MIME must be image/* or approved image MIME", "File size must be greater than zero", "Project storage path must be generated under projects/<projectId>/image/"]
   },
 ]);
 
@@ -369,14 +369,20 @@ function storagePathForProjectAsset(projectId, assetRole, usage, fileName) {
   return `${PROJECT_ASSET_STORAGE_ROOT}/${normalizedProjectId}/${role.storageFolder}/${normalizedUsage}/${sanitizeFileName(fileName)}`;
 }
 
+function catalogUploadStoredPath(projectId, assetRole, fileName) {
+  const role = roleDefinitionForId(assetRole);
+  const normalizedProjectId = normalizeProjectId(projectId);
+  if (!normalizedProjectId || !role || !fileName) {
+    return "";
+  }
+  return `${PROJECT_ASSET_STORAGE_ROOT}/${normalizedProjectId}/${role.storageFolder}/${sanitizeFileName(fileName)}`;
+}
+
 function catalogStoredPath({ assetRole, assetType, fileName, projectId, reference, source, usage }) {
   if (source === REFERENCE_SOURCE_MODE) {
     return `${PROJECT_ASSET_STORAGE_ROOT}/${projectId}/${slugify(assetType)}/${slugify(usage)}/reference/${slugify(reference)}`;
   }
-  const roleUsage = normalizeUsage(usage.toLowerCase().replaceAll(" ", "-"), assetRole);
-  return roleUsage
-    ? storagePathForProjectAsset(projectId, assetRole, roleUsage, fileName)
-    : `${PROJECT_ASSET_STORAGE_ROOT}/${projectId}/${slugify(assetType)}/${slugify(usage)}/${fileName}`;
+  return catalogUploadStoredPath(projectId, assetRole, fileName);
 }
 
 function clonePaletteSwatch(swatch) {