Refactor variable conditions to structured objects#200
Merged
mikkisguy merged 3 commits intoJun 12, 2026
Conversation
…tion component - Extract duplicated .map rendering pattern into FormattedCondition component - Return FormattedConditionPart[] from format utils for differentiated styling - Replace operator symbols with plain English text (≥ → is at least, etc.) - Remove duplicate formatStatCondition definitions from both components
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR refactors variable conditions from string arrays to structured objects: ChangesVariable Conditions Data Model Refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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 |
Contributor
|
React Doctor found 3 issues in 2 files · 3 warnings · score 85 / 100 (Great) · vs 3 warnings
Reviewed by React Doctor for commit |
coderabbitai
Bot
changed the title
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/backend/scripts/seed-technical-badges.ts (1)
809-832:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the walkthrough copy to match plain-English condition rendering.
Lines 818-825 still describe symbol-based output (e.g.,
≥,≠,¬,=), but the formatter now renders plain-English phrases. This can mislead QA when validating seeded badge output.Suggested text update
- console.log( - " 6. Click/hover on badges on lines 3, 8, and 10 to see operator symbols" - ); - console.log(" 7. Verify symbols: ≥, ≤, =, ≠ are displayed correctly"); + console.log( + " 6. Click/hover on badges on lines 3, 8, and 10 to verify condition text" + ); + console.log( + ' 7. Verify plain-English operators are shown (for example: "is at least", "is not")' + ); console.log(" 8. Line 8 showcases all variable condition types:"); - console.log(" - truthy: has_spell (displayed as just the name)"); - console.log(" - falsy: ¬is_main_quest (displayed with ¬ prefix)"); - console.log(" - ==: alignment = good, has_drink = True"); - console.log(" - !=: faction ≠ evil, is_cursed ≠ False"); + console.log(' - truthy: "is has_spell"'); + console.log(' - falsy: "not is_main_quest"'); + console.log(' - ==: alignment is "good", has_drink is true'); + console.log(' - !=: faction is not "evil", is_cursed is not false');🤖 Prompt for 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. In `@apps/backend/scripts/seed-technical-badges.ts` around lines 809 - 832, Update the walkthrough console.log copy that currently lists symbol-based output (e.g., the message containing "Verify symbols: ≥, ≤, =, ≠ are displayed correctly" and the subsequent lines describing "truthy: has_spell", "falsy: ¬is_main_quest", "==: alignment = good", "!=: faction ≠ evil", etc.) so it describes the formatter's plain‑English rendering instead of symbols; replace symbol examples with their word forms (e.g., "greater than or equal to", "less than or equal to", "equals", "not equals") and change negation/variable-condition lines to plain phrases like "truthy: has_spell", "falsy: not is_main_quest (shown as 'not ...')", "equality: alignment equals good", "inequality: faction does not equal evil", and note that boolean conditions render as True/False words (refer to the console.log statements that use TEST_EMAIL and TEST_PASSWORD for proximity).
🤖 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 `@apps/backend/src/services/rpy-generator.service.ts`:
- Around line 96-119: The string branches in the switch inside the function
building conditions push unescaped condition.value into Ren'Py code, allowing
quotes/backslashes to break the RPY; create and use a small helper (e.g.
escapeRenpyString(value: string): string) and call it in both the `==` and `!=`
string branches to replace backslashes with \\ and double quotes with \" (and
optionally escape newlines) before embedding, leaving the boolean branches as-is
so generated conditions are always valid Ren'Py string literals.
In `@apps/backend/src/services/rpy-parser.service.ts`:
- Around line 2281-2284: The current split of conditionExpr into varParts using
.split(/\s+(?:and|or|\|\||&&)\s+/) loses connective semantics and breaks quoted
strings; replace this approach by tokenizing/parsing the condition expression to
preserve logical operators and quoted literals (e.g., write a small tokenizer
that recognizes strings in quotes, identifiers, comparison operators, and the
connectives "and"/"or"/"&&"/"||"), produce an AST or an array of {expr,
connective} nodes instead of flat strings, and update downstream code that used
varParts to consume this structured representation; target the code around the
varParts variable and any functions that process those parts so quoted values
and operator tokens are preserved.
---
Outside diff comments:
In `@apps/backend/scripts/seed-technical-badges.ts`:
- Around line 809-832: Update the walkthrough console.log copy that currently
lists symbol-based output (e.g., the message containing "Verify symbols: ≥, ≤,
=, ≠ are displayed correctly" and the subsequent lines describing "truthy:
has_spell", "falsy: ¬is_main_quest", "==: alignment = good", "!=: faction ≠
evil", etc.) so it describes the formatter's plain‑English rendering instead of
symbols; replace symbol examples with their word forms (e.g., "greater than or
equal to", "less than or equal to", "equals", "not equals") and change
negation/variable-condition lines to plain phrases like "truthy: has_spell",
"falsy: not is_main_quest (shown as 'not ...')", "equality: alignment equals
good", "inequality: faction does not equal evil", and note that boolean
conditions render as True/False words (refer to the console.log statements that
use TEST_EMAIL and TEST_PASSWORD for proximity).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 38314c1a-3d23-4473-b364-a611cf25fce3
📒 Files selected for processing (18)
apps/backend/scripts/seed-technical-badges.tsapps/backend/src/db/schema/tables/label-lines.tsapps/backend/src/db/schema/tables/labels.tsapps/backend/src/lib/validation.tsapps/backend/src/services/__tests__/technical-parser.service.unit.test.tsapps/backend/src/services/label-line-mapper.tsapps/backend/src/services/labels.service.tsapps/backend/src/services/rpy-generator.service.tsapps/backend/src/services/rpy-parser.service.tsapps/frontend/src/components/__tests__/TechnicalPopover.test.tsxapps/frontend/src/components/write-mode/FormattedCondition.tsxapps/frontend/src/components/write-mode/LabelPropertiesPanel.tsxapps/frontend/src/components/write-mode/TechnicalBadge.tsxapps/frontend/src/components/write-mode/TechnicalPopover.tsxapps/frontend/src/lib/api/labels.tsapps/frontend/src/lib/format-conditions.tsapps/frontend/src/lib/prose-types.tspackages/shared/src/index.ts
…tions and improve condition parsing logic
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
mikkisguy
deleted the
181-display-variable-condition-values-in-technical-badges
branch
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.
No need for data migrations as this project is in development only.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation