Add YAML template validation API for plain scalar interpolations#25
Conversation
WalkthroughAdds a validated parsing layer across YAML/JSON/TOML template parsers, exposing parse_validated_* and validate_* public functions, updating format/check entry points to use validated parsing, and updating Python bindings and tests to exercise the new validation behavior. Validation enforces constraints like rejecting plain scalars that mix whitespace with interpolations. Changes
Sequence DiagramsequenceDiagram
participant Client
participant ValidatedParser as parse_validated_template_with_profile
participant RawParser as parse_template_with_profile
participant Validator as validate_template_stream
participant NodeValidator as validate_value_node
Client->>ValidatedParser: submit(template, profile)
ValidatedParser->>RawParser: parse(template, profile)
RawParser-->>ValidatedParser: stream (Yaml/Json/Toml nodes)
ValidatedParser->>Validator: validate(stream)
Validator->>Validator: iterate documents
Validator->>NodeValidator: validate nodes recursively
NodeValidator->>NodeValidator: validate keys/scalars/interpolations
NodeValidator-->>Validator: ok / error
Validator-->>ValidatedParser: ok / error
ValidatedParser-->>Client: Parsed stream or validation error
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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.
🧹 Nitpick comments (1)
rust/yaml-tstring-rs/src/lib.rs (1)
2051-2070: Reduce duplicated parse+validate flow in public APIs.
parse_validated_template_with_profileandvalidate_template_with_profilerun the same pipeline. Reusing one from the other keeps behavior unified if validation wiring changes later.♻️ Proposed refactor
pub fn validate_template_with_profile( template: &TemplateInput, profile: YamlProfile, ) -> BackendResult<()> { - let stream = parse_template_with_profile(template, profile)?; - validate_template_stream(&stream) + parse_validated_template_with_profile(template, profile).map(|_| ()) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/yaml-tstring-rs/src/lib.rs` around lines 2051 - 2070, Duplicate parse+validate logic exists between parse_validated_template_with_profile and validate_template_with_profile; update validate_template_with_profile to call parse_validated_template_with_profile(template, profile) and map or convert its BackendResult<YamlStreamNode> to BackendResult<()> (e.g., by discarding the returned stream or using .map(|_| ())) so all parsing+validation is centralized in parse_validated_template_with_profile and behavior stays unified across APIs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@rust/yaml-tstring-rs/src/lib.rs`:
- Around line 2051-2070: Duplicate parse+validate logic exists between
parse_validated_template_with_profile and validate_template_with_profile; update
validate_template_with_profile to call
parse_validated_template_with_profile(template, profile) and map or convert its
BackendResult<YamlStreamNode> to BackendResult<()> (e.g., by discarding the
returned stream or using .map(|_| ())) so all parsing+validation is centralized
in parse_validated_template_with_profile and behavior stays unified across APIs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5c373fdc-ebe5-4b70-81e0-41e695026f84
📒 Files selected for processing (4)
rust/python-bindings/src/lib.rsrust/yaml-tstring-rs/src/lib.rsrust/yaml-tstring-rs/tests/parser.rsyaml-tstring/tests/test_yaml_tstring.py
There was a problem hiding this comment.
🧹 Nitpick comments (1)
rust/toml-tstring-rs/src/lib.rs (1)
1926-1931: Tighten negative-infinity assertions in special-float tests.These checks currently assert only negative sign. Adding
is_infinite()would better lock the expected-infshape and prevent false positives from other negative float values.Optional test-strengthening diff
- assert!( - special_floats[1] - .as_float() - .expect("float") - .is_sign_negative() - ); + let neg = special_floats[1].as_float().expect("float"); + assert!(neg.is_infinite() && neg.is_sign_negative()); - assert!( - special_float_deeper_arrays[1][0][0] - .as_float() - .expect("float") - .is_sign_negative() - ); + let neg = special_float_deeper_arrays[1][0][0].as_float().expect("float"); + assert!(neg.is_infinite() && neg.is_sign_negative()); - assert!( - rendered.data["special_float_inline_table"]["neg"] - .as_float() - .expect("neg float") - .is_sign_negative() - ); + let neg = rendered.data["special_float_inline_table"]["neg"] + .as_float() + .expect("neg float"); + assert!(neg.is_infinite() && neg.is_sign_negative()); - assert!( - rendered.data["special_float_mixed_nested"][0][1] - .as_float() - .expect("nested neg") - .is_sign_negative() - ); + let neg = rendered.data["special_float_mixed_nested"][0][1] + .as_float() + .expect("nested neg"); + assert!(neg.is_infinite() && neg.is_sign_negative());Also applies to: 1949-1954, 2964-2969, 2982-2987
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/toml-tstring-rs/src/lib.rs` around lines 1926 - 1931, Tighten the negative-infinity test assertions by requiring both a negative sign and infinity: replace the lone is_sign_negative() check on the float returned by special_floats[..].as_float().expect("float") with a combined assertion that the value is_sign_negative() && is_infinite(), e.g. assert!(value.is_sign_negative() && value.is_infinite()); apply the same change in the other occurrences that use special_floats[..].as_float().expect("float") (the blocks around the other reported assertion sites).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@rust/toml-tstring-rs/src/lib.rs`:
- Around line 1926-1931: Tighten the negative-infinity test assertions by
requiring both a negative sign and infinity: replace the lone is_sign_negative()
check on the float returned by special_floats[..].as_float().expect("float")
with a combined assertion that the value is_sign_negative() && is_infinite(),
e.g. assert!(value.is_sign_negative() && value.is_infinite()); apply the same
change in the other occurrences that use
special_floats[..].as_float().expect("float") (the blocks around the other
reported assertion sites).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 792f4d24-b077-4f57-825c-a84b5c8d4645
📒 Files selected for processing (5)
rust/json-tstring-rs/src/lib.rsrust/json-tstring-rs/tests/parser.rsrust/python-bindings/src/lib.rsrust/toml-tstring-rs/src/lib.rsrust/toml-tstring-rs/tests/parser.rs
github-actions
Bot
added
breaking-change-analyzed
Breaking Change AnalysisResult: Breaking changes detected Reasoning: The PR routes all Python bindings (JSON, TOML, YAML) through new validated parsing paths. For YAML specifically, new validation rejects plain scalars that combine whitespace-containing text with interpolations. This is a breaking change because previously valid YAML templates like Content for Release NotesTemplate String Parsing Changes
This analysis was performed by Claude Code Action |
|
Released in 0.2.0. |
1 participant
Summary
parse_validated_template*andvalidate_template*APIs for JSON and TOML so the backend surface stays alignedTesting
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests