generator: support multiple parentheses per conditional cell - #1765
generator: support multiple parentheses per conditional cell#1765kaligrafy wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📜 Recent review details🧰 Additional context used🧠 Learnings (1)📚 Learning: 2026-03-31T18:20:32.322ZApplied to files:
🔇 Additional comments (6)
WalkthroughThe conditional evaluator and generator now support multiple consecutive opening or closing parentheses. Both paths validate characters and ordering, track group balance, split parentheses into opening and closing runs, and apply them to generated expressions. Tests cover nested expressions, malformed cells, balance checks, helper behavior, and questionnaire dictionary formatting. Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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
`@packages/evolution-common/src/services/widgets/conditionals/__tests__/checkConditionals.test.ts`:
- Around line 564-575: Add a positive runtime test case in the conditional test
suite with a balanced expression that uses `parentheses: '(('` on a single
conditional, alongside the existing multi-character closing-parentheses case.
Ensure the expression evaluates true and preserves the established conditional
structure.
In `@packages/evolution-generator/src/scripts/conditionals_generator.py`:
- Around line 288-297: Update the invalid-parentheses error message in the row
validation logic to use f-string conversion syntax for the value representation,
replacing the standalone repr call with the !r conversion for parentheses_value
while preserving the existing message and validation behavior.
In `@packages/evolution-generator/src/tests/test_conditionals_generator.py`:
- Around line 745-747: Wrap each adjacent expected-message string expression in
parentheses within the affected test list literals, including the cases near the
invalid-parentheses message and the locations referenced at 755-757, 774-775,
and 855-856. Preserve the exact message text and test behavior while eliminating
Ruff ISC004 warnings.
In
`@packages/evolution-generator/src/tests/test_generate_questionnaire_dictionary.py`:
- Around line 24-37: Update the test helpers MockCell.__init__, conditional_row,
and conditionals_data_rows with appropriate parameter and return annotations,
and replace the dictionary comprehension in conditional_row with dict.fromkeys
while preserving the existing values and row structure.
🪄 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: 7f4d8605-c48a-44a2-b895-56ab8358397d
📒 Files selected for processing (6)
packages/evolution-common/src/services/widgets/conditionals/__tests__/checkConditionals.test.tspackages/evolution-common/src/services/widgets/conditionals/checkConditionals.tspackages/evolution-generator/src/scripts/conditionals_generator.pypackages/evolution-generator/src/scripts/generate_questionnaire_dictionary.pypackages/evolution-generator/src/tests/test_conditionals_generator.pypackages/evolution-generator/src/tests/test_generate_questionnaire_dictionary.py
📜 Review details
⏰ Context from checks skipped due to timeout. (6)
- GitHub Check: check-format
- GitHub Check: test-sequential (24.x)
- GitHub Check: code-lint
- GitHub Check: pr-build-check
- GitHub Check: build-and-test (24.x)
- GitHub Check: generate-and-build-generator (24.x)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-03-31T18:20:32.322Z
Learnt from: kaligrafy
Repo: chairemobilite/evolution PR: 1462
File: packages/evolution-common/src/services/baseObjects/Completable.ts:1-6
Timestamp: 2026-03-31T18:20:32.322Z
Learning: In this repository (chairemobilite/evolution), do not flag or require a copyright year in file headers. For new files, the header format `Copyright Polytechnique Montreal and contributors` (without a year) is acceptable. Only verify the header presence/consistency, not the presence of the year.
Applied to files:
packages/evolution-common/src/services/widgets/conditionals/__tests__/checkConditionals.test.tspackages/evolution-common/src/services/widgets/conditionals/checkConditionals.ts
🪛 Ruff (0.16.0)
packages/evolution-generator/src/tests/test_generate_questionnaire_dictionary.py
[warning] 25-25: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
[warning] 29-29: Missing type annotation for **kwargs
(ANN003)
[warning] 30-30: Unnecessary dict comprehension for iterable; use dict.fromkeys instead
Replace with dict.fromkeys(iterable, value))
(C420)
[warning] 35-35: Missing type annotation for *rows
(ANN002)
packages/evolution-generator/src/tests/test_conditionals_generator.py
[warning] 745-747: Unparenthesized implicit string concatenation in collection
Did you forget a comma?
(ISC004)
[warning] 755-757: Unparenthesized implicit string concatenation in collection
Did you forget a comma?
(ISC004)
[warning] 774-775: Unparenthesized implicit string concatenation in collection
Did you forget a comma?
(ISC004)
[warning] 855-856: Unparenthesized implicit string concatenation in collection
Did you forget a comma?
(ISC004)
packages/evolution-generator/src/scripts/conditionals_generator.py
[warning] 295-295: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🔇 Additional comments (6)
packages/evolution-generator/src/scripts/conditionals_generator.py (1)
26-40: LGTM!Also applies to: 88-88, 395-415, 475-487
packages/evolution-generator/src/tests/test_conditionals_generator.py (1)
11-11: LGTM!Also applies to: 760-767, 790-804, 837-845
packages/evolution-generator/src/scripts/generate_questionnaire_dictionary.py (1)
11-11: LGTM!Also applies to: 372-375
packages/evolution-generator/src/tests/test_generate_questionnaire_dictionary.py (1)
41-136: LGTM!packages/evolution-common/src/services/widgets/conditionals/checkConditionals.ts (1)
14-14: LGTM!Also applies to: 24-58, 92-105, 207-207
packages/evolution-common/src/services/widgets/conditionals/__tests__/checkConditionals.test.ts (1)
697-724: LGTM!
The Conditionals sheet integrity check only accepted a single '(' or ')'
in the parentheses column, which made it impossible to express nested
groups like `c1 && (c2 || (c3 && c4))` that need to close two groups on
the same row (e.g. '))').
The parentheses cell now accepts any number of parentheses, with
openings before closings within a cell (e.g. '((', '))', but not ')(',
which would be silently reordered and change the grouping):
- evolution-generator: validate the new format in the row-level check
and in the cross-row balance check; extract a public
split_parentheses_cell helper reused by the questionnaire dictionary
generation.
- evolution-common: checkConditionals validates and evaluates
multi-parentheses cells the same way at runtime.
code by Cursor Composer 2.5, validated by coderabbit and
Claude 5 Fable Low
closes chairemobilite#1616
7914155 to
26c1dc1
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
The Conditionals sheet integrity check only accepted a single '(' or ')' in the parentheses column, which made it impossible to express nested groups like
c1 && (c2 || (c3 && c4))that need to close two groups on the same row (e.g. '))').The parentheses cell now accepts any number of parentheses, with openings before closings within a cell (e.g. '((', '))', but not ')(', which would be silently reordered and change the grouping):
code by Cursor Composer 2.5, validated by coderabbit and Claude 5 Fable Low
closes #1616
Summary by CodeRabbit