Skip to content

fix: handle YAML mapping indicators in driver names - #625

Merged
tariknz merged 2 commits into
mainfrom
fix/session-yaml-mapping-indicator
Jul 19, 2026
Merged

fix: handle YAML mapping indicators in driver names#625
tariknz merged 2 commits into
mainfrom
fix/session-yaml-mapping-indicator

Conversation

@tariknz

@tariknz tariknz commented Jul 19, 2026

Copy link
Copy Markdown
Owner

Description

Screenshots

Before

After

Type of Change

  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Performance improvement
  • Refactoring (no functional changes)
  • Documentation update
  • Dependency update

Checklist

  • I have discussed this change in the discord server
  • I have tested this in iRacing (either in an online session or with AI)
  • All tests pass locally via npm test
  • I have added tests that prove my fix is effective or that my feature works
  • I have run npm run lint and fixed any issues
  • I have performed a self-review of my own code
  • I have added/updated Storybook stories for visual changes
  • I have updated the README.md (if applicable)
  • I have updated defaultDashboard.ts if introducing new widgets or configurations (if applicable)

Summary by CodeRabbit

  • Bug Fixes
    • Fixed YAML session-data parsing when certain field values begin with the YAML mapping indicator (?).
    • These values are now preserved as literal text, avoiding misinterpretation as YAML structure.
  • Tests
    • Added additional Vitest coverage for YAML parsing edge cases involving leading mapping indicators, including UserName values and ensuring subsequent keys still parse correctly.

@coderabbitai

coderabbitai Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ee7c0c4f-8a1e-45e3-9256-dce7cc3e72d8

📥 Commits

Reviewing files that changed from the base of the PR and between 6354598 and f2e5be3.

📒 Files selected for processing (2)
  • src/app/irsdk/node/irsdk-node.spec.ts
  • src/app/irsdk/node/irsdk-node.ts

📝 Walkthrough

Walkthrough

Session-data YAML preprocessing now quotes scalar values beginning with ? before parsing. Regression tests cover literal and bare ? usernames and verify subsequent driver fields remain correctly parsed.

Changes

YAML Mapping Indicator Parsing

Layer / File(s) Summary
Mapping-indicator preprocessing and validation
src/app/irsdk/node/irsdk-node.ts, src/app/irsdk/node/irsdk-node.spec.ts
Adds mapping-indicator quoting to session-data YAML preprocessing and tests literal ?-prefixed username values, empty abbreviations, and subsequent fields.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • tariknz/irdashies#554: Updates the same YAML preprocessing pipeline and includes related parsing regression tests.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is mostly the template with no actual PR summary, testing details, or filled screenshots/checklist items. Add a real Description, note how it was tested, and complete the relevant screenshots and checklist items.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the YAML driver-name fix.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/session-yaml-mapping-indicator

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@src/app/irsdk/node/irsdk-node.ts`:
- Around line 222-227: Update fixMappingIndicatorValues so the mapping-indicator
regex suffix cannot consume newline characters or span into the next YAML line;
allow only horizontal whitespace and other non-newline characters after ?. Add a
regression test covering a bare UserName: ? followed by another key, verifying
the next key remains intact after parsing.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: cb47df25-bf00-4a23-a20c-58284dbf6ef1

📥 Commits

Reviewing files that changed from the base of the PR and between cc3d57d and 6354598.

📒 Files selected for processing (2)
  • src/app/irsdk/node/irsdk-node.spec.ts
  • src/app/irsdk/node/irsdk-node.ts

Comment thread src/app/irsdk/node/irsdk-node.ts
@tariknz
tariknz merged commit 26ea088 into main Jul 19, 2026
2 checks passed
@tariknz
tariknz deleted the fix/session-yaml-mapping-indicator branch July 19, 2026 03:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant