Skip to content

Fix string literal corruption in Python evaluation mode#17

Merged
hardbyte merged 1 commit into
mainfrom
fix-string-literal-corruption
Sep 11, 2025
Merged

Fix string literal corruption in Python evaluation mode#17
hardbyte merged 1 commit into
mainfrom
fix-string-literal-corruption

Conversation

@hardbyte

@hardbyte hardbyte commented Sep 2, 2025

Copy link
Copy Markdown
Owner

Replaces brittle string-based integer promotion with robust AST-based approach. Previously, string literals like "epa1" were incorrectly modified to "epa1.0" when floats existed in the evaluation context, causing string comparisons to fail.

The new implementation:

  • Parses expressions using CEL's AST to identify actual integer literals
  • Preserves string literals exactly as written
  • Maintains proper array indexing (keeps indices as integers)
  • Gracefully handles complex constructs like comprehensions via fallback
  • Ensures consistent behavior across Python and Strict evaluation modes

Fixes string comparison expressions like var == "epa1" returning False when floats are present in context. Includes comprehensive test coverage for both evaluation modes.

Resolves #16

@hardbyte hardbyte requested a review from Copilot September 3, 2025 06:50

Copilot AI 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.

Pull Request Overview

This PR fixes a critical bug where string literals containing numbers were incorrectly corrupted during evaluation when floats existed in the context. The solution replaces brittle string-based preprocessing with a robust AST-based approach that preserves string literals exactly while still enabling proper integer-to-float promotion for arithmetic operations.

  • Implements AST-based integer promotion that distinguishes between string literals and actual integer literals
  • Preserves array indexing by keeping indices as integers rather than promoting them to floats
  • Adds comprehensive test coverage for the fix across both Python and Strict evaluation modes

Reviewed Changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/lib.rs Core implementation replacing string-based integer promotion with AST-based approach
tests/test_issue16_string_literal_regression.py Focused regression tests for the string literal corruption issue
tests/test_dual_mode_comprehensive.py Comprehensive test suite validating both evaluation modes work correctly
tests/test_enhanced_error_handling.py Minor test update to use explicit float literal
CHANGELOG.md Documentation of the bug fix and new AST-based approach

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread src/lib.rs Outdated
Comment thread src/lib.rs Outdated
Comment thread src/lib.rs Outdated
@hardbyte hardbyte force-pushed the fix-string-literal-corruption branch from a28b1de to 0d4c002 Compare September 11, 2025 04:39
The library now operates exclusively in strict CEL mode
  - **Removed**: `EvaluationMode.PYTHON` and all automatic integer-to-float promotion
  - **Removed**: `mode` parameter from `evaluate()` function
  - **Removed**: `--mode` CLI option
  - **Behavior change**: Mixed arithmetic like `1 + 2.5` now raises `TypeError` instead of automatically promoting to `3.5`
  - **Migration**: Use explicit type conversion (e.g., `double(1) + 2.5`) for mixed arithmetic
  - **Rationale**: Eliminates complex AST preprocessing that was breaking `has()` short-circuiting and other CEL functions

### 🐛 Fixed

- **CEL function short-circuiting**: Fixed issue where `has()` and other CEL functions failed due to AST preprocessing interference
- **String literal corruption**: Eliminated string literal modification that occurred during integer promotion preprocessing

### Updated

- Updated cel crate from v0.11.0 to v0.11.1
- Updated documentation to reflect strict CEL mode operation
- Updated tests to work with strict CEL mode only
- Removed complex preprocessing logic
@hardbyte hardbyte force-pushed the fix-string-literal-corruption branch from c5ae6b8 to 6b07f20 Compare September 11, 2025 19:16
@hardbyte hardbyte merged commit 54d0d68 into main Sep 11, 2025
16 checks passed
@hardbyte hardbyte deleted the fix-string-literal-corruption branch February 7, 2026 02:06
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.

String variables misinterpreted as floats when floats exist in context

2 participants