Reagan/make shopping experience awesome#442
Merged
Merged
Conversation
The Instacart cart confirmation exposed a compact set of page facts, but the HTML guidance mostly named broad artifact categories. Add receipt-style patterns to the interaction skill and pin the prompt nudge so future agents treat shopping and cart confirmations as scannable HTML candidates. Constraint: Existing renderer accepts static sandboxed HTML from fenced blocks Rejected: Add renderer heuristics for prose summaries | model-side guidance is narrower and avoids changing transcript rendering Confidence: high Scope-risk: narrow Directive: Keep HTML triggers tied to dense structured facts; short conversational replies should remain markdown Tested: npm run test -- tests/unit/hl/skillIndexPrompt.test.ts Tested: npm run typecheck Tested: npm run lint Not-tested: Live Claude Code shopping run after prompt update Co-authored-by: OmX <omx@oh-my-codex.dev>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 10 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
…d site, no Chose: prefix
The dark-mode HTML guidance allowed gold accents to become large frame shadows, which made comparison artifacts feel too loud. Constrain large offset shadows to the palette shadow color and keep accents as small highlights. Constraint: Dark theme already defines cream as the palette shadow color Rejected: Remove hard shadows entirely | the house style depends on hard offset shadows Confidence: high Scope-risk: narrow Directive: Do not use accent colors for large structural shadows in dark-mode HTML artifacts Tested: npm run test -- tests/unit/hl/skillIndexPrompt.test.ts Tested: npm run typecheck Tested: npm run lint Tested: git diff --check Not-tested: Live agent-generated dark-mode HTML artifact after prompt update Co-authored-by: OmX <omx@oh-my-codex.dev>
The disabled rule `isConfirmed && !multiSelect` conflated 'currently selected' with 'already submitted', so clicking a card to select it then clicking Choose was impossible — the button disabled itself the moment the card became selected. The parent-level `disabled=locked` already handles post-submit lockdown.
Mirrors how Codex's rollout/state split works: the conversation log is
the source of truth, and the UI reconstructs derived state from it
instead of maintaining a parallel cache.
ChatTranscript now forwards each turn's next-turn user reply as
nextUserText down through ChatTurn into OptionList and AskForm. Each
component reverses its outbound message format ('Selected from options:'
and 'Answered:') to rebuild the selected ids without consulting any
client-side persistence layer. Tab-switch optimism still routes through
the in-memory submission cache, which is intentionally not persisted.
There was a problem hiding this comment.
2 issues found across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
AskForm: 'Other: <text>' answers can contain commas, so splitting on /,\s+/ corrupted them. Locate the Other segment, peel the rest of the line off as its text, and only split predefined labels. OptionList: in multi-section pickers, drop Other replies that lack a matching label prefix instead of defaulting to section 0 — silent mis-attribution risked showing the wrong section as chosen.
There was a problem hiding this comment.
2 issues found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/renderer/hub/chat-v2/AskForm.tsx">
<violation number="1" location="app/src/renderer/hub/chat-v2/AskForm.tsx:418">
P1: The new `Other` parsing assumes `Other` is the final value, so when `Other` appears first (a valid serialized order), later options are lost during transcript reconstruction.</violation>
</file>
<file name="app/src/renderer/hub/chat-v2/OptionList.tsx">
<violation number="1" location="app/src/renderer/hub/chat-v2/OptionList.tsx:849">
P2: Multi-section `Other:` selections are dropped when section labels are missing, so valid historical selections may not round-trip from transcript.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| let valuesPart = valuesStr; | ||
| let otherTail: string | null = null; | ||
| const otherIdx = (() => { | ||
| const leading = valuesPart.match(/^Other(?::|$)/); |
There was a problem hiding this comment.
P1: The new Other parsing assumes Other is the final value, so when Other appears first (a valid serialized order), later options are lost during transcript reconstruction.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/renderer/hub/chat-v2/AskForm.tsx, line 418:
<comment>The new `Other` parsing assumes `Other` is the final value, so when `Other` appears first (a valid serialized order), later options are lost during transcript reconstruction.</comment>
<file context>
@@ -406,19 +406,33 @@ export function deriveAskSubmission(
+ let valuesPart = valuesStr;
+ let otherTail: string | null = null;
+ const otherIdx = (() => {
+ const leading = valuesPart.match(/^Other(?::|$)/);
+ if (leading) return 0;
+ const m = valuesPart.match(/,\s+Other(?::|$)/);
</file context>
Comment on lines
+849
to
+850
| if (idx < 0) continue; | ||
| target = idx; |
There was a problem hiding this comment.
P2: Multi-section Other: selections are dropped when section labels are missing, so valid historical selections may not round-trip from transcript.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/renderer/hub/chat-v2/OptionList.tsx, line 849:
<comment>Multi-section `Other:` selections are dropped when section labels are missing, so valid historical selections may not round-trip from transcript.</comment>
<file context>
@@ -831,18 +831,23 @@ export function deriveSubmissionFromTranscript(
const label = labelMatch?.[1].trim();
const idx = label ? sections.findIndex((s) => s.label === label) : -1;
- target = idx >= 0 ? idx : 0;
+ if (idx < 0) continue;
+ target = idx;
}
</file context>
Suggested change
| if (idx < 0) continue; | |
| target = idx; | |
| if (idx < 0) { | |
| const unlabeled = sections.filter((s) => !s.label); | |
| if (unlabeled.length !== 1) continue; | |
| target = sections.findIndex((s) => !s.label); | |
| } else { | |
| target = idx; | |
| } |
Remove the per-question card background/border/padding so the form sits in the chat flow instead of inside a boxed surface. Drop the monospace tracked-uppercase header chip on questions and the same treatment on post-answer labels — they now read as plain sans like the rest of the chat. Custom radio/checkbox replace the native controls (single filled circle on check, no doubled-up ring), and the option row's gap is split so descriptions tuck under the title instead of floating 10px below.
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/renderer/hub/chat-v2/AskForm.tsx">
<violation number="1" location="app/src/renderer/hub/chat-v2/AskForm.tsx:418">
P1: The new `Other` parsing assumes `Other` is the final value, so when `Other` appears first (a valid serialized order), later options are lost during transcript reconstruction.</violation>
</file>
<file name="app/src/renderer/hub/chat-v2/OptionList.tsx">
<violation number="1" location="app/src/renderer/hub/chat-v2/OptionList.tsx:849">
P2: Multi-section `Other:` selections are dropped when section labels are missing, so valid historical selections may not round-trip from transcript.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
The previous SVG bg-image had a hard-coded #111 stroke, which vanished in light mode where --color-fg-primary (the checked fill) is also near-black. Switching to a mask + var(--color-bg-base) makes the stroke always contrast with the fill in both themes.
Collaborator
Author
|
@cubic review |
There was a problem hiding this comment.
2 issues found across 17 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/renderer/hub/chat-v2/AskForm.tsx">
<violation number="1" location="app/src/renderer/hub/chat-v2/AskForm.tsx:418">
P1: The new `Other` parsing assumes `Other` is the final value, so when `Other` appears first (a valid serialized order), later options are lost during transcript reconstruction.</violation>
</file>
<file name="app/src/renderer/hub/chat-v2/OptionList.tsx">
<violation number="1" location="app/src/renderer/hub/chat-v2/OptionList.tsx:849">
P2: Multi-section `Other:` selections are dropped when section labels are missing, so valid historical selections may not round-trip from transcript.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Renderer hands url to shell.openExternal, which rejects non-http(s), and image goes into <img src> — relative or javascript:/data URLs from a model-emitted block shouldn't be navigated to or loaded. Validate at parse time so bad options drop with the rest of the fail-shut checks instead of failing at click time.
useState(() => …) runs once. During session replay the AskForm/OptionList turn lands before the user-reply turn, so the initial nextUserText is null and the picker mounts as unsubmitted — even though the answer is already on the transcript and arrives a few re-renders later. Add a useEffect that hydrates state when transcriptSubmission becomes non-null and the user hasn't already submitted locally.
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/renderer/hub/chat-v2/AskForm.tsx">
<violation number="1" location="app/src/renderer/hub/chat-v2/AskForm.tsx:418">
P1: The new `Other` parsing assumes `Other` is the final value, so when `Other` appears first (a valid serialized order), later options are lost during transcript reconstruction.</violation>
</file>
<file name="app/src/renderer/hub/chat-v2/OptionList.tsx">
<violation number="1" location="app/src/renderer/hub/chat-v2/OptionList.tsx:849">
P2: Multi-section `Other:` selections are dropped when section labels are missing, so valid historical selections may not round-trip from transcript.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
The late-hydration effect was guarded on `submitted`, but bootstrap from the in-memory cache also sets submitted=true — so a stale cache hit would permanently block transcript hydration even though the transcript is the durable source of truth. Switch to a localSubmitRef that flips only on a real user submit in this mount, so cache-derived state still yields to a later-arriving transcript.
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/renderer/hub/chat-v2/AskForm.tsx">
<violation number="1" location="app/src/renderer/hub/chat-v2/AskForm.tsx:418">
P1: The new `Other` parsing assumes `Other` is the final value, so when `Other` appears first (a valid serialized order), later options are lost during transcript reconstruction.</violation>
</file>
<file name="app/src/renderer/hub/chat-v2/OptionList.tsx">
<violation number="1" location="app/src/renderer/hub/chat-v2/OptionList.tsx:849">
P2: Multi-section `Other:` selections are dropped when section labels are missing, so valid historical selections may not round-trip from transcript.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
localSubmitRef was a ref, so clearing it on submit failure didn't trigger a re-render or re-run the hydration effect. If the transcript had already arrived before the submit attempt, it would never be applied. Promote it to state so the effect's dependency list reacts to the flip back to false.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by cubic
Rebuilt the shopping/options picker for faster, clearer choices with one‑click picks, verified sources, and a cleaner receipt. Picker/ask state now derives from the transcript (including late replies), re‑hydrates after failed local submits, and overrides any cache to avoid stale views.
New Features
shell:open-external.id,image,title,url,site;urlandimagemust be absolute http(s). Decorative Airbnb badge art is dropped; missing fields are omitted (no filler “—”). Non‑http(s) values are rejected at parse time.Migration
url(absolute http(s)) and humansitefor every option (e.g., “Amazon”, “Airbnb”; no TLD).allowOther: trueonly when you really want it; default is nowfalse.image(absolute http(s)); skip badges/favicons and AirbnbAirbnbPlatformAssets/GuestFavorite/orthographic-images.Written for commit d658fb1. Summary will update on new commits. Review in cubic