Skip to content

Commit 090c6dc

Browse files
ToolboxAidToolboxAid
committed
Clean Asset Manager V2 code review findings and remove stale scope churn - PR_26126_107-asset-manager-v2-code-review-cleanup
1 parent ef51abc commit 090c6dc

16 files changed

Lines changed: 14611 additions & 6890 deletions

‎docs/dev/reports/PR_26126_093_asset_manager_v2_manual_validation_notes.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
- `node --check tools/asset-manager-v2/js/AssetManagerV2App.js`
55
- `node --check tools/asset-manager-v2/js/controls/AssetFormControl.js`
66
- `node --check tools/asset-manager-v2/js/controls/AssetCatalogControl.js`
7-
- `node --check tools/asset-manager-v2/js/services/TemporaryUatWorkspace.js`
7+
- `node --check tools/asset-manager-v2/js/services/TemporaryUatSamplePalette.js`
88
- `node --check tools/asset-manager-v2/js/services/WorkspaceBridge.js`
99
- `node --check src/shared/assets/assetPreviewHelpers.js`
1010
- `node --check tests/playwright/PreviewGeneratorV2Baseline.spec.mjs`
@@ -19,7 +19,7 @@
1919
## Manual Checks Covered By Tests
2020
- Disabled and grayed ID textbox.
2121
- Color ID generation from normalized swatch names.
22-
- Temporary `?workspace=UAT` UAT palette loading.
22+
- Temporary `?palette=sample` UAT palette loading.
2323
- Color picker replacing the file picker and Status-only empty palette messages.
2424
- Audio, font, and image preview path resolution from Workspace V2 game assets.
2525
- Selected Asset Detail accordion and Stretch Override fieldset styling.

‎docs/dev/reports/PR_26126_093_asset_manager_v2_palette_sample_notes.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
# PR_26126_093 Asset Manager V2 Palette Sample Notes
22

33
## Temporary UAT Loader
4-
- Added `tools/asset-manager-v2/js/services/TemporaryUatWorkspace.js`.
4+
- Added `tools/asset-manager-v2/js/services/TemporaryUatSamplePalette.js`.
55
- The loader is explicitly marked temporary UAT-only and isolated for later removal.
6-
- It is activated only by `?workspace=UAT`.
6+
- It is activated only by `?palette=sample`.
77
- Normal Workspace V2 palette loading remains separate.
88

99
## Palette Format
@@ -19,7 +19,7 @@
1919
- Swatches include only schema-valid `symbol`, `hex`, `name`, `source`, and `tags` fields.
2020

2121
## Behavior
22-
- `?workspace=UAT` loads the palette into memory and populates the Asset Manager V2 Color picker.
22+
- `?palette=sample` loads the palette into memory and populates the Asset Manager V2 Color picker.
2323
- The file picker stays hidden when Type is Color.
2424
- Color asset IDs are generated from swatch names, for example `Signal Violet!` becomes `assets.color.hud.signal-violet`.
2525

‎docs/dev/reports/PR_26126_096_asset_manager_v2_manual_validation_notes.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
## Commands
44
- `node --check tools/asset-manager-v2/js/assetPreviewHelpers.js`
55
- `node --check tools/asset-manager-v2/js/services/WorkspaceBridge.js`
6-
- `node --check tools/asset-manager-v2/js/services/TemporaryUatWorkspace.js`
6+
- `node --check tools/asset-manager-v2/js/services/TemporaryUatSamplePalette.js`
77
- `node --check tools/asset-manager-v2/js/AssetManagerV2App.js`
88
- `node --check tests/playwright/PreviewGeneratorV2Baseline.spec.mjs`
99
- `npx playwright test tests/playwright/PreviewGeneratorV2Baseline.spec.mjs --project=playwright --reporter=list -g "Asset Manager V2"`
@@ -20,6 +20,6 @@
2020
- Selected Asset Detail height is increased to at least 220px.
2121
- Asset preview remains inside Selected Asset Detail.
2222
- Workspace preview context is games-only.
23-
- `?workspace=UAT` temporary UAT workspace root uses `games/Asteroids/` for preview/path testing only.
24-
- `?workspace=UAT` image previews resolve under `/games/Asteroids/assets/...`.
23+
- `?palette=sample` temporary UAT root uses `games/Asteroids/` for preview/path testing only.
24+
- `?palette=sample` image previews resolve under `/games/Asteroids/assets/...`.
2525
- Sample JSON files were not modified.

‎docs/dev/reports/PR_26126_096_asset_manager_v2_temporary_uat_session_root_notes.md‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
11
# PR_26126_096 Asset Manager V2 Temporary UAT Session Root Notes
22

33
## Temporary Behavior
4-
- `?workspace=UAT` remains isolated in `TemporaryUatWorkspace.js`.
5-
- For UAT preview/path testing only, `?workspace=UAT` now provides the temporary game root `games/Asteroids/`.
4+
- `?palette=sample` remains isolated in `TemporaryUatSamplePalette.js`.
5+
- For UAT preview/path testing only, `?palette=sample` now provides the temporary game root `games/Asteroids/`.
66
- The temporary root is used only by Asset Manager V2 preview options.
77
- Asset records still store normalized asset paths such as `assets/images/uat-preview.png`; the game root is not persisted into asset entries.
88
- The code and Status log both mark this behavior as temporary UAT-only.
99

1010
## Preview Result
11-
- During `?workspace=UAT`, file-backed previews for `assets/...` paths resolve through `/games/Asteroids/assets/...`.
11+
- During `?palette=sample`, file-backed previews for `assets/...` paths resolve through `/games/Asteroids/assets/...`.
1212
- The temporary preview path does not use Sample or Tool roots.
1313
- Palette color behavior remains unchanged and still uses the temporary sample palette swatches only.
1414

1515
## Validation
1616
- Playwright validates the temporary Status message for `games/Asteroids/`.
17-
- Playwright validates a `?workspace=UAT` image asset preview renders with `/games/Asteroids/assets/images/uat-preview.png`.
17+
- Playwright validates a `?palette=sample` image asset preview renders with `/games/Asteroids/assets/images/uat-preview.png`.
1818
- Playwright validates the UAT preview path excludes `/samples/` and `/tools/`.

‎docs/dev/reports/PR_26126_097_asset_manager_v2_font_preview_notes.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
## Path Resolution
1010
- Font preview URLs continue to resolve through the active Workspace game root.
11-
- During temporary `?workspace=UAT` UAT testing, `assets/fonts/vector-battle.ttf` resolves to `/games/Asteroids/assets/fonts/vector-battle.ttf`.
11+
- During temporary `?palette=sample` UAT testing, `assets/fonts/vector-battle.ttf` resolves to `/games/Asteroids/assets/fonts/vector-battle.ttf`.
1212
- Existing Workspace V2 font previews continue to resolve under `/games/Asteroids/assets/...` when `gameId=Asteroids` is active.
1313

1414
## Failure Reporting

‎docs/dev/reports/PR_26126_105_asset_manager_v2_launch_guard_notes.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
## Guarded Launches
44
- Direct Asset Manager V2 launch without Workspace Manager context now shows the launch guard overlay before controls are mounted.
55
- Workspace Manager launch without an active palette now shows the same guard overlay.
6-
- The temporary UAT-only `?workspace=UAT` context remains allowed and supplies the sample Asteroids game root for preview/path testing.
6+
- The temporary UAT-only `?palette=sample` context remains allowed and supplies the sample Asteroids game root for preview/path testing.
77

88
## Overlay Message
99
- The guard message states: `Asset Manager V2 is only available through Workspace Manager with a game workspace and palette.`

‎docs/dev/reports/PR_26126_105_asset_manager_v2_manual_validation_notes.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
- `npm run test:workspace-v2` passed 13/13 Playwright tests.
1414
- Playwright validates direct Asset Manager V2 launch shows the launch guard overlay.
1515
- Playwright validates Workspace launch without an active palette shows the launch guard overlay.
16-
- Playwright validates `?workspace=UAT` remains a valid temporary UAT context.
16+
- Playwright validates `?palette=sample` remains a valid temporary UAT context.
1717
- Playwright validates Path is disabled and visually matches the disabled ID field.
1818
- Playwright validates click/tap asset selection continues to update selected tile styling, Selected Asset Detail, and preview.
1919
- The keyboard cleanup `rg` search returned no matches in Asset Manager V2 source or the touched Playwright spec.

‎docs/dev/reports/PR_26126_107_asset_manager_v2_code_review_cleanup_notes.md‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# PR_26126_107 Code Review Cleanup Notes
2+
3+
## Dead Method Removal
4+
- Removed unused `AssetManagerV2App.exportAssets`.
5+
- Removed unused `AssetManagerV2App.exportToolState`.
6+
- Removed unused `AssetManagerV2App.copyAssetsJson`.
7+
- Verified those method names no longer appear in Asset Manager V2 source or tests.
8+
9+
## Report Churn Cleanup
10+
- Restored the old PR_26126_093, PR_26126_096, PR_26126_097, and PR_26126_105 report files to their pre-PR_26126_106 contents.
11+
- Kept current PR_26126_107 reports and shared review/coverage reports as the only report updates beyond the explicit historical-report revert.
12+
13+
## Scope Guard
14+
- `package.json` was not changed in this cleanup pass.
15+
- `tests/playwright/PreviewGeneratorV2Baseline.spec.mjs` was not changed in this cleanup pass.
16+
- Asset Manager V2 tests remain separated in `tests/playwright/AssetManagerV2.spec.mjs`.
17+
- Temporary UAT launch now accepts `?workspace=uat` as well as `?workspace=UAT`.
18+
- No sample JSON was modified.

‎docs/dev/reports/PR_26126_107_asset_manager_v2_manual_validation_notes.md‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
# PR_26126_107 Manual Validation Notes
2+
3+
## Commands
4+
- `node --check tools/asset-manager-v2/js/AssetManagerV2App.js`
5+
- `rg -n "exportAssets\\(|exportToolState\\(|copyAssetsJson\\(" tools\asset-manager-v2 tests\playwright\AssetManagerV2.spec.mjs tests\playwright\PreviewGeneratorV2Baseline.spec.mjs`
6+
- `rg -n "KeyD|KeyA|KeyS|KeyW|ArrowLeft|ArrowRight|ArrowUp|ArrowDown|Home|End|PageUp|PageDown|Enter|keydown|keyup" tools\asset-manager-v2 tests\playwright\AssetManagerV2.spec.mjs`
7+
- `git diff -- package.json tests\playwright\PreviewGeneratorV2Baseline.spec.mjs`
8+
- `git diff --check`
9+
- `npm run test:workspace-v2`
10+
11+
## Results
12+
- `npm run test:workspace-v2` passed 14/14 Playwright tests.
13+
- Removed dead Asset Manager V2 methods are absent from source and tests.
14+
- Active Asset Manager V2 source/tests contain no listed keyboard-navigation key handling or assertions.
15+
- Asset Manager V2 temporary UAT launch is validated with lowercase `?workspace=uat`.
16+
- `package.json` and `PreviewGeneratorV2Baseline.spec.mjs` have no PR_26126_107 diff.
17+
- Asset Manager V2 tests remain in `tests/playwright/AssetManagerV2.spec.mjs`.
18+
- Historical PR report churn from PR_26126_106 was reverted for the requested report files.
19+
- No sample JSON was modified.

‎docs/dev/reports/codex_changed_files.txt‎

Lines changed: 24 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,29 @@
11
# git status --short
2-
M docs/dev/reports/PR_26126_093_asset_manager_v2_manual_validation_notes.md
3-
M docs/dev/reports/PR_26126_093_asset_manager_v2_palette_sample_notes.md
4-
M docs/dev/reports/PR_26126_096_asset_manager_v2_manual_validation_notes.md
5-
M docs/dev/reports/PR_26126_096_asset_manager_v2_temporary_uat_session_root_notes.md
6-
M docs/dev/reports/PR_26126_097_asset_manager_v2_font_preview_notes.md
7-
M docs/dev/reports/PR_26126_105_asset_manager_v2_launch_guard_notes.md
8-
M docs/dev/reports/PR_26126_105_asset_manager_v2_manual_validation_notes.md
9-
M docs/dev/reports/codex_changed_files.txt
10-
M docs/dev/reports/codex_review.diff
11-
M docs/dev/reports/coverage_changed_js_guardrail.txt
12-
M docs/dev/reports/playwright_v8_coverage_report.txt
13-
M package.json
14-
M tests/playwright/PreviewGeneratorV2Baseline.spec.mjs
15-
M tools/asset-manager-v2/js/AssetManagerV2App.js
16-
D tools/asset-manager-v2/js/services/TemporaryUatSamplePalette.js
17-
?? docs/dev/reports/PR_26126_106_asset_manager_v2_keyboard_cleanup_review_notes.md
18-
?? docs/dev/reports/PR_26126_106_asset_manager_v2_test_separation_notes.md
19-
?? docs/dev/reports/PR_26126_106_asset_manager_v2_workspace_uat_notes.md
20-
?? tests/helpers/workspaceV2CoverageReporter.mjs
21-
?? tests/playwright/AssetManagerV2.spec.mjs
22-
?? tools/asset-manager-v2/js/services/TemporaryUatWorkspace.js
2+
M docs/dev/reports/PR_26126_093_asset_manager_v2_manual_validation_notes.md
3+
M docs/dev/reports/PR_26126_093_asset_manager_v2_palette_sample_notes.md
4+
M docs/dev/reports/PR_26126_096_asset_manager_v2_manual_validation_notes.md
5+
M docs/dev/reports/PR_26126_096_asset_manager_v2_temporary_uat_session_root_notes.md
6+
M docs/dev/reports/PR_26126_097_asset_manager_v2_font_preview_notes.md
7+
M docs/dev/reports/PR_26126_105_asset_manager_v2_launch_guard_notes.md
8+
M docs/dev/reports/PR_26126_105_asset_manager_v2_manual_validation_notes.md
9+
AM docs/dev/reports/PR_26126_107_asset_manager_v2_code_review_cleanup_notes.md
10+
AM docs/dev/reports/PR_26126_107_asset_manager_v2_manual_validation_notes.md
11+
M docs/dev/reports/codex_changed_files.txt
12+
M docs/dev/reports/codex_review.diff
13+
MM docs/dev/reports/coverage_changed_js_guardrail.txt
14+
MM docs/dev/reports/playwright_v8_coverage_report.txt
15+
M tests/playwright/AssetManagerV2.spec.mjs
16+
M tools/asset-manager-v2/js/AssetManagerV2App.js
17+
M tools/asset-manager-v2/js/services/TemporaryUatWorkspace.js
2318

2419
# git ls-files --others --exclude-standard
25-
docs/dev/reports/PR_26126_106_asset_manager_v2_keyboard_cleanup_review_notes.md
26-
docs/dev/reports/PR_26126_106_asset_manager_v2_test_separation_notes.md
27-
docs/dev/reports/PR_26126_106_asset_manager_v2_workspace_uat_notes.md
28-
tests/helpers/workspaceV2CoverageReporter.mjs
29-
tests/playwright/AssetManagerV2.spec.mjs
30-
tools/asset-manager-v2/js/services/TemporaryUatWorkspace.js
20+
(no output)
3121

3222
# git diff --stat
33-
...093_asset_manager_v2_manual_validation_notes.md | 4 +-
34-
...26_093_asset_manager_v2_palette_sample_notes.md | 6 +-
35-
...096_asset_manager_v2_manual_validation_notes.md | 6 +-
36-
..._manager_v2_temporary_uat_session_root_notes.md | 8 +-
37-
...6126_097_asset_manager_v2_font_preview_notes.md | 2 +-
38-
...6126_105_asset_manager_v2_launch_guard_notes.md | 2 +-
39-
...105_asset_manager_v2_manual_validation_notes.md | 2 +-
40-
docs/dev/reports/codex_changed_files.txt | 56 +-
41-
docs/dev/reports/codex_review.diff | 3584 +++++++++++++++++---
42-
docs/dev/reports/coverage_changed_js_guardrail.txt | 4 +-
43-
docs/dev/reports/playwright_v8_coverage_report.txt | 12 +-
44-
package.json | 2 +-
45-
.../playwright/PreviewGeneratorV2Baseline.spec.mjs | 1346 +-------
46-
tools/asset-manager-v2/js/AssetManagerV2App.js | 30 +-
47-
.../js/services/TemporaryUatSamplePalette.js | 36 -
48-
15 files changed, 3236 insertions(+), 1864 deletions(-)
23+
.../PR_26126_107_asset_manager_v2_code_review_cleanup_notes.md | 1 +
24+
.../reports/PR_26126_107_asset_manager_v2_manual_validation_notes.md | 1 +
25+
docs/dev/reports/coverage_changed_js_guardrail.txt | 2 +-
26+
docs/dev/reports/playwright_v8_coverage_report.txt | 4 ++--
27+
tests/playwright/AssetManagerV2.spec.mjs | 2 +-
28+
tools/asset-manager-v2/js/services/TemporaryUatWorkspace.js | 3 ++-
29+
6 files changed, 8 insertions(+), 5 deletions(-)

0 commit comments

Comments
 (0)