Skip to content

Conversation

@sensei-hacker
Copy link
Member

@sensei-hacker sensei-hacker commented Dec 31, 2025

Summary

Fixes two issues identified by Qodo bot code review on PR #2482:

  1. Conflict detection indexing bug - Prevents false positives and enables race condition detection
  2. AST consistency - Wraps raw values in Literal nodes as expected by AST conventions

Changes

Issue #3: Conflict Detection Indexing Bug

Problem:

  • Only ifthen handlers received unique keys
  • All on.always() blocks collapsed to single key "on.always"
  • This caused two problems:
    • False positives: Assignments in different on.always() blocks incorrectly flagged as conflicts
    • Disabled race detection: Race check requires handlers.length > 1, but all blocks collapsed to 1 key

Fix:

  • Give every handler a unique index: ${stmt.handler}:${stmtIndex}
  • Location: js/transpiler/transpiler/analyzer.js:528-530

Impact:

  • Eliminates false positive conflict warnings that erode user trust
  • Enables race condition detection for on.always() handlers

Issue #5: Raw Values Not Wrapped in Literal Nodes

Problem:

  • >= and <= optimization passed raw numbers instead of AST nodes
  • Created type inconsistency: condition.right should always be an AST node

Fix:

  • Wrap constValue±1 in {type: 'Literal', value: ...} objects
  • Location: js/transpiler/transpiler/condition_generator.js:300,307

Impact:

  • Maintains AST consistency throughout transpiler pipeline
  • Prevents potential issues if code expects AST node properties

Testing

✅ All 27 transpiler test suites pass with 0 failures

  • Existing tests validate the fixes work correctly
  • No regressions detected

Related

This addresses two issues identified by Qodo bot code review:

Issue #3: Conflict detection indexing bug
- Previously only ifthen handlers got unique keys
- All on.always() blocks collapsed to single key, causing:
  * False positives: assignments in different blocks flagged as conflicts
  * Disabled race detection: check requires multiple handlers but all collapsed to 1
- Fix: Give every handler a unique index regardless of type
- Location: js/transpiler/transpiler/analyzer.js:528-530

Issue #5: Raw values not wrapped in Literal nodes
- >= and <= optimization passed raw numbers instead of AST nodes
- Fix: Wrap constValue±1 in {type: 'Literal', value: ...} objects
- Maintains AST consistency throughout pipeline
- Location: js/transpiler/transpiler/condition_generator.js:300,307

All 27 transpiler test suites pass with 0 failures.
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