Prove a guessed promo code tells the customer nothing - #2018
Conversation
A code that matches nothing must buy nothing *and* give nothing away, but the story only checked that no negative figure appeared. A zero-value row named after the guess, or a "we don't know that code" message, would have passed. The quote step now keeps only the order summary table, so a message where a table should be fails loudly, and a new step insists the summary never names the code the customer guessed at. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 45 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe change adds a reusable answer-preservation wrapper, updates discount-code quote support, and verifies that invalid codes do not appear in summaries or alter no-code pricing. The page-down journey also uses the shared wrapper. ChangesDiscount-code quote verification
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Customer
participant DiscountCodeSteps
participant quoteFor
participant askForTotal
Customer->>DiscountCodeSteps: Request quote with promo code
DiscountCodeSteps->>quoteFor: Call quoteFor
quoteFor->>askForTotal: Submit promo code
askForTotal-->>quoteFor: Return booking-page response
quoteFor-->>DiscountCodeSteps: Return quote response
DiscountCodeSteps->>quoteFor: Request no-code quote
quoteFor-->>DiscountCodeSteps: Return no-code response
DiscountCodeSteps-->>Customer: Compare summaries
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![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: e584acb570
ℹ️ 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".
| browser.currentHtml.match( | ||
| /<table class="order-summary">[\s\S]*?<\/table>/, | ||
| )?.[0], |
There was a problem hiding this comment.
The TestBrowser replaces currentHtml with the raw /calculate response, so it does not contain the booking form or the customer's input. Extracting only the table therefore hides any adjacent disclosure: for example, a response containing <p>Promo code TENOFF was not found</p> followed by the normal full-price table would satisfy every new scenario step. Keep the full response available and check its visible output—ideally against the no-code response—so both named and generic invalid-code messages fail this privacy scenario.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
You're right, and the narrowing you flagged was mine — fixed in 860eead.
The whole /calculate answer is kept again, so a sentence beside the table is the story's to see. Two things now guard the rule:
- the quote step insists the answer holds an
order-summarytable, so a refusal to quote can't be read as a summary; and - a new step asks the same price with the code box empty and demands the two answers match word for word:
When a customer asks the price of a Pottery place with the code "TENOFF"
Then the summary total is 10.00
And the summary shows no discount line
And the summary never names "TENOFF"
And the summary reads exactly as it does for a Pottery place with no codeI checked it fails for the right reason using your exact example: with the site temporarily emitting <p>That code was not found</p> only when an unmatched code was typed, the total, no-discount-line and never-names steps all still passed — only the new comparison step failed. That's the generic-message case the earlier steps could not see.
Full precommit green with coverage at 100%; the new shared helper it introduced (keepsAnswerAs) has its own direct unit tests, since a story alone never counts as coverage here.
Generated by Claude Code
A code that matches nothing must give nothing away, but the story only read the summary table. A message beside the table — "that code was not found" — would have passed every step, and a general message that never names the code would pass even a check for the guess itself. The quote step now keeps the site's whole answer, and insists it holds a summary table so a refusal to quote can't be read as a summary. A new step asks the same price with the code box empty and demands the two answers match word for word, which no disclosure can survive. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/specs/support/site-pages.ts`:
- Around line 175-178: Update the exported ownerTriesToTakePageDown wrapper
around keepsAnswerAs with an explicit function type using Parameters<typeof
ownerTakesPageDown> for its arguments and Promise<void> as its return type.
In `@test/specs/support/world.ts`:
- Around line 175-182: Define named function types for the exported journeys and
annotate each exported value: add an explicit return type to keepsAnswerAs in
test/specs/support/world.ts (lines 175-182), and explicit function-type
annotations to quoteFor and customerAsksPrice in
test/specs/support/discount-codes.ts (lines 114-120). Preserve their existing
parameters and behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: cb57abae-1aa2-4ffc-bf4b-5ec825737ce4
📒 Files selected for processing (6)
specs/payments/booking-with-a-discount-code.featuretest/specs/steps/discount-codes.tstest/specs/support/discount-codes.tstest/specs/support/site-pages.tstest/specs/support/world.test.tstest/specs/support/world.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 860eead500
ℹ️ 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".
| export const quoteFor = fromBookingPage((_world, browser, code) => | ||
| askForTotal(browser, code), | ||
| ); |
There was a problem hiding this comment.
The new quoteFor export relies on inference, and the same regression appears on the new keepsAnswerAs and refactored ownerTriesToTakePageDown exports. This repository requires every exported function to carry an explicit return type so Deno records a stable contract and reports drift at the definition; add named function types or annotations rather than exporting these inferred curried shapes.
AGENTS.md reference: AGENTS.md:L97-L97
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in bd98deb, along with customerPaysWithCode — it carried the same inferred shape from #2017 and lives in a file this change touches.
One named type covers them all, rather than a separate one per export:
export type StoryJourney<Args extends unknown[], Answer> = (
world: TicketsWorld,
...args: Args
) => Promise<Answer>;keepsAnswerAs takes a StoryJourney<Args, string> and returns a StoryJourney<Args, void>; quoteFor is StoryJourney<APlaceAndACode, string>; customerAsksPrice, customerPaysWithCode and ownerTriesToTakePageDown are the void form. Full precommit green.
Generated by Claude Code
The shared helper and the quote journeys were exported with types the compiler worked out, so a change in shape would have surfaced at some faraway caller rather than at the definition. Each now says what it hands back: a journey the site answers in words, or that same journey with its answer put away under the story's name for it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
2 participants
Follow-up to #2017, from a Codex review point on that pull request.
The gap
The discount-code story has a rule saying a code that matches nothing buys nothing and gives nothing away — someone guessing at codes should not be able to tell a real one from a made-up one. The story only checked that no negative amount appeared on the price summary, so three ways of breaking that rule would still have passed:
All three tell a guesser whether they were close.
What this changes
The quote step keeps the site's whole answer to "Show total", and insists that answer holds a summary table — so a refusal to quote can never be read as a summary. Two new steps then close the rule:
The last step asks the same price again with the code box left empty and demands the two answers match word for word. Nothing a guesser could read anything into survives that.
I checked the comparison fails for the right reason: with the site temporarily emitting "That code was not found" only when an unmatched code was typed, the total, no-discount-line and never-names steps all still passed — only the new step failed.
A merge along the way
Two helpers were doing the same job in different words: one ran a price quote and kept the answer, another ran a page deletion and kept the answer. They are now a single
keepsAnswerAsin the shared story support, which turned both call sites into one line each. It carries its own direct unit tests, because a story is never allowed to be the only thing covering a line here.No behaviour changed and no site code was touched — this only makes an existing rule provable.
Checks
Full precommit green: typecheck, lint, duplication at zero, copy checks, edge build, and the whole suite with 100% line and branch coverage.
🤖 Generated with Claude Code
https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
Summary by CodeRabbit
Bug Fixes
Tests