Conversation
![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: c3f2225a73
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const dataset = node?.dataset ?? {}; | ||
|
|
||
| return { | ||
| id: | ||
| String(dataset.contractTarget ?? dataset.contractInteraction ?? "").trim() || |
There was a problem hiding this comment.
observeRemotePage() only serializes a remote target's synthesized id here, so by the time mapRemoteObservationTargets() reaches evaluateSurfaceCompliance(), the validator no longer has interactionId, componentId, viewportId, or contextId. That breaks both resolveTargetAcquisitionOverride() and resolveTargetAcquisitionPolicy(): any surface that relies on a per-interaction exception or a viewport/context-specific budget will be validated against the base surface budget during --remote-url, producing false target-* violations for contracts that are actually compliant.
Useful? React with 👍 / 👎.
| const missingMetrics: string[] = []; | ||
| const requiredMetrics = ["boundingBox", "edgeInsetPx"]; | ||
|
|
There was a problem hiding this comment.
This unobservable check only requires boundingBox and edgeInsetPx, but both spacing rules below depend on nearestNeighborGapPx. In the non-remote path buildInteractiveTargetDescriptor() leaves nearestNeighborGapPx as null unless authors add an explicit data-contract-target-gap, so multi-control surfaces can silently skip target-gap-too-tight and destructive-target-too-close without any target-unobservable finding. That means a large part of the new target-acquisition policy is not enforced in ordinary static validation.
Useful? React with 👍 / 👎.
| if (state.id === context.id) { | ||
| return true; | ||
| } | ||
| return state.kind === context.kind; |
There was a problem hiding this comment.
findMatchingFeedbackContexts() falls back to state.kind === context.kind even after a state has an explicit id/context that does not match this context. Because the schema only requires context ids to be unique, a valid contract can declare multiple error or loading contexts; in that case one observed state will be validated against all same-kind contexts and can emit spurious feedback-* violations for requirements that belong to a different context.
Useful? React with 👍 / 👎.
1 participant
Summary
Testing