Skip to content

Commit 8d3e6dc

Browse files
committed
Correct Assets MVP source rules for Data, Sprite, Vector, and references - PR_26163_073-assets-source-rules-followup
1 parent 2b1f438 commit 8d3e6dc

7 files changed

Lines changed: 187 additions & 20 deletions

File tree

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
# PR_26163_073-assets-source-rules-followup
2+
3+
## Branch Validation
4+
- PASS: current branch is `main`.
5+
- Expected branch: `main`.
6+
7+
## Impacted Lanes
8+
- Toolbox Assets runtime lane.
9+
- Assets mock DB repository lane.
10+
- Targeted Playwright lane: `tests/playwright/tools/AssetToolMockRepository.spec.mjs`.
11+
- Workspace contract lane via legacy command `npm run test:workspace-v2`.
12+
13+
## Requirement Checklist
14+
- PASS: `docs_build/dev/PROJECT_INSTRUCTIONS.md` was read before implementation.
15+
- PASS: current branch was verified as `main` before changes.
16+
- PASS: Data assets support Source options `Upload` and `Reference`.
17+
- PASS: Data Upload file picker accepts `.json`, `.csv`, and `.txt`.
18+
- PASS: Data Upload uses the selected filename in the File detail and saved row.
19+
- PASS: Data Upload save without a selected file fails visibly with `Choose an upload file before saving.`
20+
- PASS: Sprite add/edit rows are Reference-only for MVP and do not show upload file selection.
21+
- PASS: Vector add/edit rows are Reference-only for MVP and do not show upload file selection.
22+
- PASS: Palette References remain Reference-only.
23+
- PASS: Image, Audio, and Font upload filename behavior remains covered and passing.
24+
- PASS: user-facing source help text appears per asset type in add/edit rows.
25+
- PASS: no fake filename generator exists; `catalogFileNameForName` is absent.
26+
- PASS: Source modes change only through explicit Source dropdown selection.
27+
- PASS: Projectile remains absent from usage values.
28+
- PASS: shared Tags type-ahead behavior remains covered and passing.
29+
- PASS: owner-scoped asset records remain covered and passing.
30+
- PASS: scope stayed limited to Assets source rules, targeted tests, and required reports.
31+
32+
## Changed Files
33+
- `toolbox/assets/assets.js`
34+
- `src/dev-runtime/persistence/tool-repositories/assets-mock-repository.js`
35+
- `tests/playwright/tools/AssetToolMockRepository.spec.mjs`
36+
- `docs_build/dev/reports/playwright_v8_coverage_report.txt`
37+
- `docs_build/dev/reports/PR_26163_073-assets-source-rules-followup.md`
38+
- `docs_build/dev/reports/codex_review.diff`
39+
- `docs_build/dev/reports/codex_changed_files.txt`
40+
41+
## Validation Performed
42+
- PASS: `node --check toolbox/assets/assets.js`
43+
- PASS: `node --check src/dev-runtime/persistence/tool-repositories/assets-mock-repository.js`
44+
- PASS: `node --check tests/playwright/tools/AssetToolMockRepository.spec.mjs`
45+
- PASS: `npx playwright test tests/playwright/tools/AssetToolMockRepository.spec.mjs --reporter=line --workers=1`
46+
- Result: 3 passed.
47+
- PASS: `npm run test:workspace-v2`
48+
- Result: 5 passed.
49+
- PASS: `git diff --check`
50+
51+
## Playwright Result
52+
- PASS: targeted Assets Playwright validates Data Upload/Reference, Data `.json`/`.csv`/`.txt` filename capture, Data upload missing-file failure, Sprite/Vector/Palette Reference-only source rules, Image/Audio/Font upload filename behavior, Projectile absence, shared Tags type-ahead, and owner scope.
53+
54+
## V8 Coverage
55+
- PASS/WARN: `toolbox/assets/assets.js` browser V8 coverage collected at 96%.
56+
- WARN: `src/dev-runtime/persistence/tool-repositories/assets-mock-repository.js` is exercised through Node-side repository assertions and API-backed browser behavior, but is not browser-collected V8 runtime code.
57+
- Details: `docs_build/dev/reports/playwright_v8_coverage_report.txt`.
58+
59+
## Search Evidence
60+
- PASS: `toolbox/assets/assets.js` contains `const REFERENCE_ONLY_ASSET_TYPES = new Set(["Sprites", "Vectors", "Palette References"]);`.
61+
- PASS: `toolbox/assets/assets.js` contains `const ALWAYS_MIXED_SOURCE_ASSET_TYPES = new Set(["Data"]);`.
62+
- PASS: `toolbox/assets/assets.js` contains `Data can upload .json, .csv, or .txt files`.
63+
- PASS: `toolbox/assets/assets.js` contains `Sprites are Reference-only for MVP.`
64+
- PASS: `toolbox/assets/assets.js` contains `Vectors are Reference-only for MVP.`
65+
- PASS: `toolbox/assets/assets.js` contains `Palette References must reference a palette swatch.`
66+
- PASS: `rg "catalogFileNameForName|fake filename"` returned no active implementation matches.
67+
- PASS: Projectile appears only in negative Playwright assertions confirming it is absent.
68+
69+
## Skipped Lanes
70+
- Samples validation: SKIP. Samples are explicitly out of scope; this PR only changes the Assets tool source-rule behavior and its mock repository contract.
71+
- Full samples smoke: SKIP. The request explicitly says not to run full samples smoke.
72+
- Broad unrelated toolbox lanes: SKIP. No shared navigation, tool registry, auth, sample JSON, or production runtime behavior changed.
73+
74+
## Manual Validation Steps
75+
1. Open `toolbox/assets/index.html`.
76+
2. Reset Asset Library.
77+
3. Add Data and confirm Source offers Upload and Reference.
78+
4. Save Data Upload without selecting a file and confirm visible failure.
79+
5. Select `.json`, `.csv`, then `.txt` files and confirm the File detail updates to the selected filename.
80+
6. Save Data Upload, then add another Data row and choose Reference to confirm the reference selector appears.
81+
7. Add Sprites and Vectors and confirm only Reference is available and no Upload File picker appears.
82+
8. Add Palette References and confirm it remains Reference-only.
83+
9. Add Images, Audio, and Fonts and confirm Upload filename display still works.
84+
85+
## Samples Decision
86+
- SKIP: no sample JSON alignment, sample runtime, or sample smoke behavior is in scope for PR_26163_073.

docs_build/dev/reports/codex_changed_files.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
docs_build/dev/reports/PR_26163_072-assets-source-upload-reference-details.md
1+
docs_build/dev/reports/PR_26163_073-assets-source-rules-followup.md
22
docs_build/dev/reports/codex_review.diff
33
docs_build/dev/reports/playwright_v8_coverage_report.txt
44
src/dev-runtime/persistence/tool-repositories/assets-mock-repository.js
-64.9 KB
Binary file not shown.
Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
Playwright V8 Coverage Report
22

3-
PR: PR_26163_072-assets-source-upload-reference-details
3+
PR: PR_26163_073-assets-source-rules-followup
44
Coverage source: targeted Playwright/Chromium V8 coverage from tests/playwright/tools/AssetToolMockRepository.spec.mjs.
55
Dependencies: no new npm packages.
66
Thresholds: none enforced; coverage is advisory.
7-
Result: WARN only for dev-runtime code that is exercised by tests but not collectible by browser V8 runtime coverage.
7+
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.
88

99
Changed runtime JavaScript files:
10-
(94%) toolbox/assets/assets.js - covered by targeted Assets Playwright; V8 reported executed lines 704/704 and executed functions 67/71.
10+
(96%) toolbox/assets/assets.js - covered by targeted Assets Playwright; V8 reported executed lines 725/725 and executed functions 69/72.
1111
(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.
1212

1313
Changed non-runtime validation files:
14-
(0%) tests/playwright/tools/AssetToolMockRepository.spec.mjs - Playwright spec updated for PR072 behavior; test files are not browser runtime coverage targets.
14+
(0%) tests/playwright/tools/AssetToolMockRepository.spec.mjs - Playwright spec updated for PR073 behavior; test files are not browser runtime coverage targets.

src/dev-runtime/persistence/tool-repositories/assets-mock-repository.js

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,11 @@ export const ASSET_ROLE_DEFINITIONS = Object.freeze([
4848
extensions: [".json", ".csv", ".txt"],
4949
mimeTypes: ["application/json", "text/csv", "text/plain"],
5050
previewBehavior: "Data/Table Tool managed preview",
51-
uploadEnabled: false,
52-
inputMode: "managed-tool",
51+
uploadEnabled: true,
52+
inputMode: "file",
5353
maxSizeBytes: 5242880,
5454
usageRoles: ["config", "table"],
55-
validationNeeds: ["Data/Table Tool required", "Structured data must declare format"]
55+
validationNeeds: ["Data files must use .json, .csv, or .txt", "Structured data must declare format"]
5656
},
5757
{
5858
id: "font",
@@ -228,7 +228,7 @@ function normalizeCatalogUsage(value) {
228228
}
229229

230230
function isReferenceCatalogType(assetType) {
231-
return assetType === "Palette References" || assetType === "Data";
231+
return assetType === "Sprites" || assetType === "Vectors" || assetType === "Palette References";
232232
}
233233

234234
function normalizeCatalogSourceMode(value) {
@@ -277,9 +277,6 @@ export function pickerDiagnosticForRole(role, paletteSnapshot = null) {
277277
if (!role) {
278278
return "Choose an approved asset role.";
279279
}
280-
if (role.id === "data") {
281-
return "Data/Table Tool required.";
282-
}
283280
return `${role.label} file upload ready.`;
284281
}
285282

@@ -1077,6 +1074,16 @@ export function createAssetToolMockRepository(options = {}) {
10771074
label: "File"
10781075
});
10791076
}
1077+
if (assetType && source === UPLOAD_SOURCE_MODE && fileName) {
1078+
const role = roleDefinitionForId(catalogAssetRoleForType(assetType));
1079+
if (role && !extensionMatchesRole(role, fileName)) {
1080+
findings.push({
1081+
action: `Choose an approved ${assetType} file: ${role.extensions.join(", ")}.`,
1082+
field: "fileName",
1083+
label: "File"
1084+
});
1085+
}
1086+
}
10801087
if (!usage) {
10811088
findings.push({
10821089
action: `Choose a usage: ${ASSET_USAGE_OPTIONS.join(", ")}.`,

tests/playwright/tools/AssetToolMockRepository.spec.mjs

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ const USAGE_VALUES = [
2929
];
3030
const UPLOAD_COLUMNS = ["Source", "File", "Usage", "Tags", "Preview", "Actions"];
3131
const REFERENCE_COLUMNS = ["Source", "Reference", "Usage", "Tags", "Preview", "Actions"];
32-
const REFERENCE_ASSET_TYPES = new Set(["Palette References", "Data"]);
32+
const REFERENCE_ASSET_TYPES = new Set(["Sprites", "Vectors", "Palette References"]);
3333

3434
test.beforeEach(async ({ page }) => {
3535
await installPlaywrightStorageIsolation(page, {
@@ -218,6 +218,39 @@ test("Assets source controls require real upload filenames and valid references"
218218
await editRow.getByRole("button", { name: "Save" }).click();
219219
await expect(page.locator("[data-asset-tool-row]").filter({ hasText: "Reference" })).toBeVisible();
220220

221+
await page.getByRole("button", { name: "Add Data" }).click();
222+
editRow = page.locator("[data-asset-tool-editing-row='__new__:Data']");
223+
await expect(editRow.getByLabel("Source").locator("option")).toHaveText(["Upload", "Reference"]);
224+
await expect(editRow.locator("[data-asset-tool-source-help]")).toContainText(".json, .csv, or .txt");
225+
await expect(editRow.getByLabel("Upload File")).toBeVisible();
226+
await expect(editRow.getByLabel("Upload File")).toHaveAttribute("accept", /\.json/);
227+
await expect(editRow.getByLabel("Upload File")).toHaveAttribute("accept", /\.csv/);
228+
await expect(editRow.getByLabel("Upload File")).toHaveAttribute("accept", /\.txt/);
229+
await editRow.getByRole("button", { name: "Save" }).click();
230+
await expect(page.locator("[data-asset-tool-log]")).toHaveText("Choose an upload file before saving.");
231+
await expect(page.locator("[data-asset-tool-count]")).toHaveText("2");
232+
for (const fileName of ["source-data.json", "source-data.csv", "source-data.txt"]) {
233+
await editRow.getByLabel("Upload File").setInputFiles({
234+
buffer: Buffer.from("mock data"),
235+
mimeType: "text/plain",
236+
name: fileName
237+
});
238+
await expect(editRow.locator("[data-asset-tool-selected-file]")).toHaveText(fileName);
239+
}
240+
await editRow.getByLabel("Usage").selectOption("Theme");
241+
await editRow.getByRole("button", { name: "Save" }).click();
242+
await expect(page.locator("[data-asset-tool-row]").filter({ hasText: "source-data.txt" })).toBeVisible();
243+
244+
await page.getByRole("button", { name: "Add Data" }).click();
245+
editRow = page.locator("[data-asset-tool-editing-row='__new__:Data']");
246+
await editRow.getByLabel("Source").selectOption("Reference");
247+
await expect(editRow.getByLabel("Reference")).toBeVisible();
248+
await expect(editRow.getByLabel("Upload File")).toHaveCount(0);
249+
await editRow.getByLabel("Reference").selectOption({ label: "source-data.txt" });
250+
await editRow.getByLabel("Usage").selectOption("Interface");
251+
await editRow.getByRole("button", { name: "Save" }).click();
252+
await expect(page.locator("[data-asset-tool-row]").filter({ hasText: "Reference" }).filter({ hasText: "Interface" })).toBeVisible();
253+
221254
await page.getByRole("button", { name: "Add Audio" }).click();
222255
editRow = page.locator("[data-asset-tool-editing-row='__new__:Audio']");
223256
await expect(editRow.getByLabel("Source")).toHaveValue("Upload");
@@ -242,9 +275,28 @@ test("Assets source controls require real upload filenames and valid references"
242275
await expect(editRow.locator("[data-asset-tool-selected-file]")).toHaveText("source-font.woff2");
243276
await editRow.getByRole("button", { name: "Cancel" }).click();
244277

278+
await page.getByRole("button", { name: "Add Sprites" }).click();
279+
editRow = page.locator("[data-asset-tool-editing-row='__new__:Sprites']");
280+
await expect(editRow.getByLabel("Source")).toHaveValue("Reference");
281+
await expect(editRow.getByLabel("Source").locator("option")).toHaveText(["Reference"]);
282+
await expect(editRow.getByLabel("Reference")).toBeVisible();
283+
await expect(editRow.getByLabel("Upload File")).toHaveCount(0);
284+
await expect(editRow.locator("[data-asset-tool-source-help]")).toContainText("Reference-only");
285+
await editRow.getByRole("button", { name: "Cancel" }).click();
286+
287+
await page.getByRole("button", { name: "Add Vectors" }).click();
288+
editRow = page.locator("[data-asset-tool-editing-row='__new__:Vectors']");
289+
await expect(editRow.getByLabel("Source")).toHaveValue("Reference");
290+
await expect(editRow.getByLabel("Source").locator("option")).toHaveText(["Reference"]);
291+
await expect(editRow.getByLabel("Reference")).toBeVisible();
292+
await expect(editRow.getByLabel("Upload File")).toHaveCount(0);
293+
await expect(editRow.locator("[data-asset-tool-source-help]")).toContainText("Reference-only");
294+
await editRow.getByRole("button", { name: "Cancel" }).click();
295+
245296
await page.getByRole("button", { name: "Add Palette References" }).click();
246297
editRow = page.locator("[data-asset-tool-editing-row='__new__:Palette References']");
247298
await expect(editRow.getByLabel("Source")).toHaveValue("Reference");
299+
await expect(editRow.getByLabel("Source").locator("option")).toHaveText(["Reference"]);
248300
await expect(editRow.getByLabel("Reference")).toBeVisible();
249301
await expect(editRow.getByLabel("Upload File")).toHaveCount(0);
250302
await expect(editRow).toContainText("No valid reference source exists.");

toolbox/assets/assets.js

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,17 +39,26 @@ let editingAssetType = "";
3939
let selectedAssetId = "";
4040
const draftAssetValues = new Map();
4141
const draftTagKeys = new Map();
42-
const REFERENCE_ONLY_ASSET_TYPES = new Set(["Palette References", "Data"]);
42+
const REFERENCE_ONLY_ASSET_TYPES = new Set(["Sprites", "Vectors", "Palette References"]);
43+
const ALWAYS_MIXED_SOURCE_ASSET_TYPES = new Set(["Data"]);
4344
const UPLOAD_COLUMNS = Object.freeze(["Source", "File", "Usage", "Tags", "Preview", "Actions"]);
4445
const REFERENCE_COLUMNS = Object.freeze(["Source", "Reference", "Usage", "Tags", "Preview", "Actions"]);
4546
const UPLOAD_SOURCE = "Upload";
4647
const REFERENCE_SOURCE = "Reference";
4748
const UPLOAD_ACCEPT_BY_ASSET_TYPE = Object.freeze({
4849
Audio: "audio/*,.mp3,.wav,.ogg,.m4a",
50+
Data: ".json,.csv,.txt,application/json,text/csv,text/plain",
4951
Fonts: ".ttf,.otf,.woff,.woff2",
50-
Images: "image/*,.png,.jpg,.jpeg,.webp,.gif,.svg",
51-
Sprites: "image/*,.png,.jpg,.jpeg,.webp,.gif",
52-
Vectors: ".svg,image/svg+xml"
52+
Images: "image/*,.png,.jpg,.jpeg,.webp,.gif,.svg"
53+
});
54+
const SOURCE_HELP_BY_ASSET_TYPE = Object.freeze({
55+
Audio: "Audio can upload a sound file or reference an existing audio source.",
56+
Data: "Data can upload .json, .csv, or .txt files, or reference an existing data source.",
57+
Fonts: "Fonts upload font files; Reference appears when a font source exists.",
58+
Images: "Images can upload an image file or reference an existing image source.",
59+
"Palette References": "Palette References must reference a palette swatch.",
60+
Sprites: "Sprites are Reference-only for MVP.",
61+
Vectors: "Vectors are Reference-only for MVP."
5362
});
5463

5564
function normalizeText(value) {
@@ -220,6 +229,9 @@ function sourceOptionsForAssetType(assetType, referenceOptions, selectedSource)
220229
if (isReferenceAssetType(assetType)) {
221230
return [REFERENCE_SOURCE];
222231
}
232+
if (ALWAYS_MIXED_SOURCE_ASSET_TYPES.has(assetType)) {
233+
return [UPLOAD_SOURCE, REFERENCE_SOURCE];
234+
}
223235
if (referenceOptions.length > 0 || selectedSource === REFERENCE_SOURCE) {
224236
return [UPLOAD_SOURCE, REFERENCE_SOURCE];
225237
}
@@ -231,6 +243,16 @@ function createSourceSelect(assetType, selectedSource, referenceOptions) {
231243
return createSelect("Source", "assetToolSourceInput", options, selectedSource);
232244
}
233245

246+
function createSourceControl(assetType, selectedSource, referenceOptions) {
247+
const wrapper = document.createElement("div");
248+
wrapper.className = "content-stack content-stack--compact";
249+
const help = document.createElement("span");
250+
help.dataset.assetToolSourceHelp = "true";
251+
help.textContent = SOURCE_HELP_BY_ASSET_TYPE[assetType] || "Choose how this asset source is created.";
252+
wrapper.append(createSourceSelect(assetType, selectedSource, referenceOptions), help);
253+
return wrapper;
254+
}
255+
234256
function createFileUploadControl(assetType, selectedFileName) {
235257
const wrapper = document.createElement("div");
236258
wrapper.className = "content-stack content-stack--compact";
@@ -373,7 +395,7 @@ function createEditRow(assetType, asset = null, snapshot = {}) {
373395
}
374396

375397
const sourceCell = document.createElement("td");
376-
sourceCell.append(createSourceSelect(assetType, selectedSource, referenceOptions));
398+
sourceCell.append(createSourceControl(assetType, selectedSource, referenceOptions));
377399

378400
const detailCell = document.createElement("td");
379401
if (selectedSource === REFERENCE_SOURCE) {
@@ -603,8 +625,8 @@ function renderMetadata(snapshot) {
603625
`Tags: ${assetTags(asset).map((tagKey) => tagNameForKey(snapshot.tags || [], tagKey)).join(", ") || "No tags"}`,
604626
`Stored path: ${asset.storedPath || asset.path}`,
605627
];
606-
if (isReferenceAssetType(asset.assetType || asset.type)) {
607-
metadataLines.splice(1, 0, `Reference: ${assetReference(asset)}`);
628+
if (assetSource(asset) === REFERENCE_SOURCE) {
629+
metadataLines.splice(1, 0, `Source: ${REFERENCE_SOURCE}`, `Reference: ${assetReference(asset)}`);
608630
} else {
609631
metadataLines.splice(1, 0, `Source: ${assetSource(asset)}`, `File: ${assetFile(asset)}`);
610632
}

0 commit comments

Comments
 (0)