Add product visual design memory#21
Conversation
Entire-Checkpoint: 32fbf2c327b4
Entire-Checkpoint: 32fbf2c327b4
Entire-Checkpoint: 32fbf2c327b4
Entire-Checkpoint: 32fbf2c327b4
![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: 192e39c363
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da9a2ce1e6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| boolean(value.required, `${path}.required`); | ||
| member(value.status, ["passed", "failed", "blocked", "skipped"], `${path}.status`); | ||
| boundedUniqueStrings(value.reasons, `${path}.reasons`, { minimum: 0, maximum: 128 }); | ||
| if (value.evidence === null) return; |
There was a problem hiding this comment.
For a v0.3 result supplied by another producer or restored into the ledger, a required non-static validator can set status: "passed" and evidence: null; this early return accepts it, and a recomputed integrity digest and decision then let latestValidationResult() expose the whole result as passed. Since non-static validators require trustworthy evidence, only blocked/skipped results should be allowed to omit it.
Useful? React with 👍 / 👎.
| uniqueStringArray(surface.viewports, `${label}.viewports`, 1, 20); | ||
| uniqueStringArray(surface.themes, `${label}.themes`, 1, 3); | ||
| uniqueStringArray(surface.states, `${label}.states`, 1, 50); |
There was a problem hiding this comment.
These per-dimension limits allow profiles whose Cartesian capture matrix exceeds the baseline manifest's 500-capture maximum—for example, one surface with 4 viewports, 3 themes, and 50 states requires 600 captures. Such a profile passes validateProductDesignProfile(), but finalizeVisualCaptures() necessarily emits more than 500 entries and then fails validateVisualBaselineManifest(), so no valid baseline can ever be finalized; validate the aggregate matrix size up front.
Useful? React with 👍 / 👎.
| const commandResult = await runValidationCommand(definition, workspace, home); | ||
| commands.push(commandResult); | ||
| const gateStatus = typed | ||
| ? await appendValidatorResult(validators, definition, commandResult, workspace) |
There was a problem hiding this comment.
When the declared evidence path is already tracked in the validated commit, a validator command that merely exits zero can reuse that committed report because the runner never removes or marks it before execution. This lets stale semantic, visual, or operational metrics pass every later exact-head validation without the adapter emitting fresh evidence, contrary to the typed-validator contract; clear the declared output before launching the command or otherwise prove it was produced by this run.
Useful? React with 👍 / 👎.
| theme, | ||
| state, | ||
| artifact: { | ||
| uri: new URL(basename(filename), `${baseUri.toString().replace(/\/?$/, "/")}`).toString(), |
There was a problem hiding this comment.
Profile IDs and states permit URL metacharacters, so a valid capture such as home--desktop--light--signed#in.png is found and hashed but new URL() interprets #in.png as a fragment; ? similarly becomes a query. The emitted durable URI therefore points to a different artifact path even though its digest and byte count describe the local file, so encode the filename as a path segment or reject unsafe matrix identifiers.
Useful? React with 👍 / 👎.
| push: | ||
| branches: [main] |
There was a problem hiding this comment.
On a push to main, checkout makes both HEAD and origin/main resolve to the newly pushed commit, while the committed manifest has requireEntireCheckpoint: true. The runner therefore examines an empty origin/main..HEAD range and throws before executing any validator, so this workflow's main-push path cannot pass; use the push event's previous commit as the base or restrict this workflow to pull requests.
Useful? React with 👍 / 👎.
| const actual = sha256(bytes); | ||
| if (actual !== source.digest) blockers.push(`source digest mismatch: ${source.path}`); | ||
| } | ||
| for (const decision of profileValue.sources.decisions) await readFile(containedPath(root, decision, "decision")); |
There was a problem hiding this comment.
When a referenced decision document changes, this loop only confirms that the path remains readable, unlike token and component sources whose bytes are checked against committed digests. A decision-only change therefore leaves the profile digest and approved baseline valid and inspectDesignMemory() still reports design_memory_ready, even though part of the claimed design authority has drifted; record and verify decision digests as well.
Useful? React with 👍 / 👎.
1 participant
Summary
Validation
da9a2ce1e6dcd5b7ced065d7386ca4fbf65f240asha256:2ec4bd24dfa60b8b2f64331de3e7704f914c960d0fcc5025053d73380c318bdeRollout boundary
Merge authorized. Deployment remains separately gated.