Lower the code complexity limit to 13 - #1887
Conversation
📝 WalkthroughWalkthroughThis PR refactors mutation execution, lexical and test-quality tooling, admin and booking flows, payment metadata, UI behavior, hosted card handling, shared validation, and related tests and configuration. ChangesMutation runner orchestration
Hosted card interaction
Code-quality tooling
Application flow refactors
Test support maintenance
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@scripts/test-quality-audit.ts`:
- Around line 40-50: Update testBlockEnd to lexically skip comments, quoted
strings, and template literals—including escaped characters—so parentheses
inside them do not change depth; only count delimiters in executable code. Add a
regression case covering a test title containing “)” and verify the boundary
reaches the actual closing call delimiter without a false assertionless-test
finding.
In `@src/features/admin/attendee-form-model.ts`:
- Around line 341-415: Extract the SubmittedLine and LineResolver types plus
submittedLines, parseLine, and parseLines into a dedicated module, preserving
their current behavior and documentation. Update attendee-form-model.ts to
import and use parseLines, and move any required shared imports or type
references without changing parsing semantics.
In `@src/features/admin/settings-features.ts`:
- Around line 41-45: Replace the manual enabled-value comparison in the feature
settings handler with a Valibot picklist schema for "true" and "false"; parse
the value at this external boundary and preserve the existing errorRedirect
behavior for invalid input while using the validated result downstream.
In `@src/ui/client/admin/ticket-quantity-required.ts`:
- Around line 15-16: Update the quantity parsing in the ticket validation logic
around the total calculation to reject partially numeric input such as
“1invalid”; validate that the complete trimmed input is a valid positive
quantity before adding it to total. Add a regression test covering malformed
mixed numeric text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b409b22f-a541-4f06-aabd-899d9eabfd56
📒 Files selected for processing (42)
TODO.mdbiome.jsondeno.jsone2e-payments/src/providers/card.tse2e-payments/src/providers/visible-action.tsscripts/mutation/evaluate.tsscripts/mutation/execution.tsscripts/mutation/generate.tsscripts/run-tests.tsscripts/test-quality-audit.tssrc/features/admin/actions.tssrc/features/admin/attendee-form-model.tssrc/features/admin/listings-parents.tssrc/features/admin/scanner.tssrc/features/admin/settings-features.tssrc/features/public/ticket-payment.tssrc/features/public/ticket-submit.tssrc/shared/booking/fold-tree.tssrc/shared/booking/signed-metadata.tssrc/shared/ledger/validate.tssrc/shared/site-pages/core.tssrc/ui/client/admin/ticket-quantity-required.tssrc/ui/templates/admin/listings/details.tsxsrc/ui/templates/attendee-table.tsxtest/features/admin/attendee-form-model.test.tstest/features/public/ticket-payment/packages.test.tstest/features/public/ticket-submit.test.tstest/integration/test-utils/stubs-and-mocks.test.tstest/lib/code-quality/detectors.tstest/lib/code-quality/duplicate-types.test.tstest/lib/code-quality/relative-import.tstest/lib/db/migration-restore/helpers.tstest/lib/i18n-coverage.test.tstest/lib/server-misc-admin-handlers.test.tstest/scripts/e2e-card-provider.test.tstest/scripts/mutation-evaluate.test.tstest/scripts/mutation-execution.test.tstest/scripts/static-cdn-publish.test.tstest/test-utils/fake-dom.tstest/ui/client/admin/ticket-quantity-required.test.tstest/ui/templates/admin/listings/overview/detail-roster.test.tstest/ui/templates/attendee-table/values.test.ts
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 `@scripts/typescript-lex.ts`:
- Around line 28-35: Update the template-substitution scanner around scanText so
comment contents are skipped before processing brace characters: ignore both
block comments and line comments, including any “}” within them, while
preserving depth updates for actual substitution braces. Add direct lexer
regressions covering comment-contained braces and the nested-template case
described, and audit related scanners for the same premature-termination
behavior.
In `@src/ui/client/admin/ticket-quantity-required.ts`:
- Around line 15-17: Update the quantity parsing in the ticket-quantity guard to
use the same lexical rule as child-selection, accepting only "0" or integers
without leading zeros; preserve the existing positive safe-integer accumulation
behavior and add a regression case covering leading-zero inputs such as "01" and
"0001".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 59ddc6be-3909-4148-b551-b625a232419c
📒 Files selected for processing (16)
scripts/test-quality-audit.tsscripts/typescript-lex.tssrc/features/admin/attendee-form-lines.tssrc/features/admin/attendee-form-model.tssrc/features/admin/servicing/form-model.tssrc/features/admin/settings-features.tssrc/ui/client/admin/ticket-quantity-required.tssrc/ui/templates/admin/attendee-form.tsxtest/integration/servicing/form-schema.test.tstest/lib/code-quality/callsite-scanners.test.tstest/lib/code-quality/detectors.tstest/lib/code-quality/duplicate-types.test.tstest/lib/code-quality/relative-import.tstest/scripts/test-quality-audit.test.tstest/ui/client/admin/ticket-quantity-required.test.tstest/ui/templates/admin/dashboard.test.ts
![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: 1e0b812b76
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@deno.json`:
- Line 19: Update the "typecheck" script in deno.json to remove the standalone
scripts/screenshots.ts entry while retaining scripts/screenshots/**/*.ts and all
other existing typecheck paths.
In `@scripts/test-quality-audit-model.ts`:
- Around line 68-78: The assertionless-test filter in findAssertionlessTests
matches raw test content, allowing comments or strings containing expect/assert
text to hide findings. Use the same lexical blanking applied by
testBlockEnd/testBlockRanges before applying EXPECT_PATTERN, and add a
regression test in the test-quality-audit-model suite with only a comment or
string mention, preserving full branch coverage.
In `@test/scripts/test-quality-audit-model.test.ts`:
- Around line 20-33: Split the test around auditTestContent into two focused
tests: one covering fake test declarations inside strings, templates, line
comments, and block comments, and another covering weak-assertion text in those
lexical contexts. Keep the real passing test only in the declaration-focused
fixture and give each test an assertion that isolates its respective behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d179fe0e-3a3d-465a-bd72-98296bb1f113
📒 Files selected for processing (4)
deno.jsonscripts/test-quality-audit-model.tsscripts/test-quality-audit.tstest/scripts/test-quality-audit-model.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe599921d5
ℹ️ 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".
| /** Add metadata needed after a paid booking returns from the provider. Child | ||
| * allocations always travel with the payment. A single parent's redirect only | ||
| * needs an explicit override when folding added a child listing. */ | ||
| export const withPaidBookingMetadata = ( |
There was a problem hiding this comment.
No production module imports this new export; it is only called inside this file and by the new test/features/public/ticket-submit.test.ts, so the public surface exists just to unit-test a private helper. That is the same test-only API pattern the repo asks us to delete rather than allow-list; keep the helper private and cover it through the submit/paid path that production calls.
AGENTS.md reference: AGENTS.md:L64-L64
Useful? React with 👍 / 👎.
| const isQuoteAt = (content: string, index: number): boolean => | ||
| isQuote(content[index]) && | ||
| !(content[index] === "'" && isIdentifierChar(content[index - 1])); |
There was a problem hiding this comment.
This apostrophe heuristic applies to all TypeScript, not just JSX text, so valid strings that start immediately after an identifier-like keyword are no longer blanked. For example, an assertionless test containing return'expect(1).toBe(1)' leaves the quoted expect( visible to blankSpans(..., true), causing the test-quality audit to treat fixture text as a real assertion; the TSX-text exception needs to be narrower than every identifier-preceded apostrophe.
Useful? React with 👍 / 👎.
|
|
||
| /** The page header whose gallery is shown: a group takes priority, otherwise a | ||
| * sole listing supplies the header. A multi-listing page has no one gallery. */ | ||
| export const ticketGalleryTarget = ( |
There was a problem hiding this comment.
This helper is only used by getTicketContext in this module and by the new package test, with no production importer elsewhere, so exporting it creates another test-only surface rather than a real shared API. Please keep it private and exercise it through the public ticket-context path instead of exporting an internal calculation solely for tests.
AGENTS.md reference: AGENTS.md:L64-L64
Useful? React with 👍 / 👎.
1 participant
Summary
Review
1e0b812b.5551336fafter CI exposed the executable shell as uncovered.fe599921, with fixes for three valid audit issues and checked evidence for five stale or incorrect claims.Checks
deno task cpdpassed with no duplication.deno task precommitwas not rerun, as requested.Summary by CodeRabbit
Bug Fixes
quantity_andpackage_quantity_inputs, including leading-zero values.Quality Improvements