fix: don't crash prompt validation on literal braces (valid Jinja text)

[ManoharPaturi]

Fixes #904.

DataDesigner.validate() ran every prompt through string.Formatter().parse() to find f-string-style references, with no handling for the ValueError Python raises on unmatched literal braces — so a prompt containing valid Jinja text like a literal } (e.g. a JSON example) crashed validation instead of returning its normal result.

Now the scan catches the ValueError and falls back to a tolerant regex ((?<!\{)\{\s*(\w+)\s*\}(?!\})) that still detects {column} references while never matching Jinja {{ ... }} expressions, so the advisory check keeps working.

3 new tests (2 engine + 1 public API) fail on main, pass here. Engine suite 2259 passed, data-designer 1120 passed, config 644 passed; ruff clean.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[github-actions]

Linked Issue Check

Issue #904 has not been triaged yet. A maintainer needs to review
the issue and add the triaged label for this check to pass.

You can continue working on the PR in the meantime. The check will
re-run automatically once the issue is triaged.

[github-actions]

All contributors have signed the DCO ✍️ ✅
_{Posted by the DCO Assistant Lite bot.}

[greptile-apps]

Confidence Score: 5/5

The PR appears safe to merge with the prior formatted-reference concern fully addressed.

Findings

1.  Fallback Misses Formatted References ▶

Fix with agent prompt

### Issue 1
packages/data-designer-engine/src/data_designer/engine/validation.py:undefined-454
When an unmatched literal brace triggers this fallback, f-string references with conversions or format specifications, such as `{random_number!r}` or `{random_number:03d}`, are not detected because the regex requires the closing brace immediately after the column name. `Formatter.parse()` detects these references in balanced prompts, so adding an unrelated literal brace now suppresses the intended `F_STRING_SYNTAX` warning.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

• Falls back to a tolerant format-field scan when Formatter.parse() rejects the prompt.
• Preserves advisory detection for basic, converted, formatted, and nested format-field references.
• Adds engine-level and public API regression coverage.

_{Reviews (5) · Last reviewed commit: "fix: don't crash prompt validation on li..."}

[greptile-apps]

Fallback Misses Formatted References

When an unmatched literal brace triggers this fallback, f-string references with conversions or format specifications, such as {random_number!r} or {random_number:03d}, are not detected because the regex requires the closing brace immediately after the column name. Formatter.parse() detects these references in balanced prompts, so adding an unrelated literal brace now suppresses the intended F_STRING_SYNTAX warning.

Knowledge Base Used: Validation and processing

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/data-designer-engine/src/data_designer/engine/validation.py
Line: 454

Comment:
**Fallback Misses Formatted References**

When an unmatched literal brace triggers this fallback, f-string references with conversions or format specifications, such as `{random_number!r}` or `{random_number:03d}`, are not detected because the regex requires the closing brace immediately after the column name. `Formatter.parse()` detects these references in balanced prompts, so adding an unrelated literal brace now suppresses the intended `F_STRING_SYNTAX` warning.

**Knowledge Base Used:** [Validation and processing](https://app.greptile.com/nvidia-public-github/-/custom-context/knowledge-base/nvidia-nemo/datadesigner/-/docs/validation-and-processing.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[ManoharPaturi]

good catch, the fallback now also matches refs with a conversion or format spec ({name!r}, {name:03d}, {name:>10}) since those are valid f-string references too. added a test for it, engine suite green.

[ManoharPaturi]

I have read the DCO document and I hereby sign the DCO.

[nabinchha]

Thanks for putting this together, @ManoharPaturi!

Summary

This fixes the reported DataDesigner.validate() crash by containing string.Formatter failures and falling back to a delimiter-aware reference scan. The crash path is covered, but the fallback does not yet preserve the full reference-detection behavior described by the PR.

Findings

Warnings — Worth addressing

packages/data-designer-engine/src/data_designer/engine/validation.py:454 — Preserve nested format-spec references in the fallback

• What: The fallback rejects any brace inside a format spec, so Literal } value {x:{width}} returns no F_STRING_SYNTAX violation even though the balanced value {x:{width}} form is detected by Formatter.parse().
• Why: Adding an unrelated literal brace changes the advisory result for valid Python format syntax, leaving a small gap in the fallback's goal of preserving reference detection.
• Suggestion: Replace or extend the regex with a tolerant scanner that recognizes nested replacement fields while ignoring unmatched literal braces, and add this case to the formatted-reference regression test.

packages/data-designer-engine/src/data_designer/engine/validation.py:454 — Name the pattern after its actual syntax contract

• What: _F_STRING_REFERENCE_PATTERN suggests that this is a general f-string parser, but it runs over ordinary template strings after Formatter.parse() fails and recognizes only a subset of single-brace replacement fields.
• Why: The name hides the fallback's narrower grammar and makes it easy for future changes to assume behavior the regex does not provide. It also conflates f-string evaluation with string.Formatter-style field syntax.
• Suggestion: Rename the private constant to something precise such as _SINGLE_BRACE_REFERENCE_PATTERN or _FORMAT_FIELD_REFERENCE_PATTERN. The existing ViolationType.F_STRING_SYNTAX can remain unchanged for compatibility.

packages/data-designer-engine/tests/engine/test_validation.py:239 and packages/data-designer/tests/interface/test_data_designer.py:1558 — Annotate the new tests

• What: The four new test functions omit return annotations, and the public-API regression test also leaves its fixture parameters untyped.
• Why: AGENTS.md, STYLEGUIDE.md, and DEVELOPMENT.md require annotations on new test functions and fixtures; the current Ruff configuration does not catch these omissions.
• Suggestion: Add -> None to all four functions and annotate the interface-test fixture parameters with their existing fixture types.

What Looks Good

• The exception handling is narrowly scoped to the advisory parser, so valid Jinja behavior is preserved without weakening the primary Jinja validation.
• Coverage is layered well: focused engine tests cover both brace directions and warning behavior, while the public API test proves DataDesigner.validate() no longer leaks ValueError.
• The latest fallback handles the previously raised simple conversion and format-spec cases (!r and :03d) while excluding normal {{ ... }} Jinja references.

Residual Risk

GitHub's current check job is failing only because linked issue #904 has not received the required triaged label; the changed-file lint/format checks and both changed test files pass locally.

Verdict

Needs changes — preserve nested replacement-field detection, rename the fallback pattern to describe its actual contract, and add the project-required test annotations before merge. The separate issue-triage check also remains an external merge gate.

---

This review was generated by an AI assistant.

[ManoharPaturi]

thanks for the careful review, all three addressed in the updated head:

• nested format specs: the fallback spec part now accepts one level of nested braces, so Literal } value {random_number:{width}} is detected again. added a regression test for exactly that prompt shape.
• renamed the pattern to _FORMAT_FIELD_REFERENCE_PATTERN since it scans plain template strings for single-brace format fields, not f-strings.
• annotated the new tests (-> None everywhere, fixture params typed on the interface test).

engine suite 2261 passed, interface regression test green.

[ManoharPaturi]

@nabinchha Following up on the updates in this branch — all tests and annotations are in place. Whenever you have a chance, could you please add the triaged label to linked issue #904 so the Linked Issue Check workflow can pass? Thanks!

[ManoharPaturi]

@nabinchha just following up, the three findings from your review are addressed in the updated head (nested format-spec refs, pattern renamed to _FORMAT_FIELD_REFERENCE_PATTERN, test annotations). suites are green on my side. anything else you'd like changed before this can move forward?