fix: keep scalar strings as text in template contexts by v0ropaev · Pull Request #1119 · NVIDIA-NeMo/DataDesigner

v0ropaev · 2026-10-30T13:58:18Z

📋 Summary

Before a template is rendered, every string cell went through json.loads, so a cell with 1.10 rendered as 1.1, true as True, null as None and 1e3 as 1000.0. Following the direction in #1088, template contexts now keep strings that parse to a JSON scalar as they are and only replace strings that hold a JSON object or array, so {{ col.key }} keeps working.

🔗 Related Issue

Closes #1088

🔄 Changes

deserialize_json_values gets a keyword-only keep_scalar_strings flag (default False, so existing callers behave as before). With it set, a string is only replaced when it decodes to a dict or list. This applies at every level, so a string nested in a dict stays a string too.
The template consumers pass keep_scalar_strings=True: the LLM completion prompt record (llm_completion.py), the image prompt (image.py), expression columns (expression.py), skip.when evaluation (skip_evaluator.py) and the schema transform processor (schema_transform.py).
Embedding input handling (embedding.py) and structured-response parsing (_process_serialized_output in llm_completion.py) are unchanged.
Regression tests for each consumer: test_utils.py (the flag itself, top level and nested), test_expression.py (v{{ version }}, {{ flag }}, {{ flag == 'true' }} and {{ meta.score }} in one template, plus {{ n | int + 1 }} on string cells), test_skip_evaluator.py, test_schema_transform.py, test_llm_completion_generators.py (the record passed to the prompt renderer) and test_image.py (the rendered prompt).
The four expression tests that patch deserialize_json_values with lambda x: x now accept the keyword.
columns.mdx gets a short note under expression columns that text cells render as written and numbers stored as text need | int or | float before arithmetic, plus a note on text flags in skip.when.

Release note

Templates (LLM and image prompts, expression columns, skip.when and schema transforms) now see text cells as the text they contain. A cell like "1.10", "true" or "null" is no longer converted to 1.1, True or None before rendering. Arithmetic or numeric comparisons on numbers stored as text now need an explicit cast, for example {{ quantity | int + 1 }} or {{ score | float > 0.5 }}. Without a cast, such an expression fails to render and the row is dropped, and a skip.when condition that fails is treated as true, so the cell is skipped. An equality check against a number, like {{ n == 5 }}, is now simply false for the text "5". In skip.when, a text flag "false" or "null" is now a non-empty string, so when="{{ flag }}" skips the cell where it used to generate it; compare explicitly with {{ flag == 'true' }}. Columns holding a JSON object or array still support nested access like {{ result.score }}. Cells that already have a numeric or boolean dtype are not affected.

🧪 Testing

make test passes
Unit tests added/updated
E2E tests added/updated (if applicable)

All config, engine and interface tests pass (5012 passed, 11 skipped, with LC_ALL=C for the import perf test as before). The 10 new test cases that check the new behavior fail against main's source. make check-all is clean.

The repro from the issue, plus a {{ n | int + 1 }} column on "5".."8", now prints v1.10, true, True, v1e3, null and 6..9.

✅ Checklist

Follows commit message conventions
Commits are signed off (DCO)
If I am an external contributor, the linked issue is open and labeled triaged
Architecture docs updated (if applicable)

Template contexts ran every string cell through json.loads, so "1.10" rendered as 1.1, "true" as True and "null" as None. Only replace strings that hold a JSON object or array there, so nested access keeps working and text renders as written. Embedding inputs and structured-response parsing still use the full conversion. Closes NVIDIA-NeMo#1088 Signed-off-by: Dmitry Voropaev <dy.voropaev@gmail.com>

greptile-apps · 2026-10-30T14:01:10Z

Confidence Score: 4/5

[Medium impact] The change reaches several template-based workflows in data generation.

The remaining annotation requirement must be satisfied before merging; no new blocking behavior was found.

Findings

Required types are missing ▶

Fix with agent prompt

### Issue 1
packages/data-designer-engine/tests/engine/processing/test_utils.py:undefined-169
The new `test_deserialize_json_values_keep_scalar_strings` function has no parameter or return annotations. The repository guide requires annotations on all functions, including tests. Add them here and to `test_image_cell_generator_keeps_scalar_strings_in_prompt`; the new `keep_scalar_strings` parameter on `deserialize_json_values` also needs an annotation. This requirement must be satisfied before merging.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

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

Summary

Templates keep scalar text such as "1.10", "true" and "null" unchanged while still decoding JSON objects and arrays.

Template inputs keep scalar text while JSON objects remain usable.
Skip conditions treat text flags as text.

_{Reviews (3) · Last reviewed commit: "chore: annotate new tests and move priva..." · Reviewed by Greptile}

andreatnvidia · 2026-10-30T18:44:44Z

 - **Conditional logic**: if/elif/else support
 - **Arithmetic**: Add, subtract, multiply, divide

+Values from other columns keep their type in templates, so a text cell such as `"1.10"` or `"true"` renders exactly as written. Cast numbers stored as text before doing math with them, for example `{{ quantity | int + 1 }}` or `{{ price | float * 2 }}`. Columns holding a JSON object or array still support nested access such as `{{ result.score }}`.


Could we also cover text flags in skip.when here? With when="{{ flag }}", text "false" and "null" now cause the cell to be skipped, and {{ not flag }} flips the other way. That follows the intended string semantics, but is easy to miss from the numeric-cast guidance. A short note recommending explicit comparisons like {{ flag == 'true' }}, plus regression cases, would help.

Added a note right after the expression paragraph: text "false" and "null" are non-empty strings, so when="{{ flag }}" skips and {{ not flag }} doesn't, and the recommended form is {{ flag == 'true' }}. Regression cases for both forms plus the explicit comparison are in test_skip_evaluator.py (04cb220). They fail on main, as expected.

andreatnvidia · 2026-10-30T18:44:55Z

+        ),
+    ],
+)
+def test_deserialize_json_values_keep_scalar_strings(input_data, expected_result):


A few default-mode cases would help protect the compatibility boundary too. The new tests cover keep_scalar_strings=True, but the existing default-mode cases don't exercise scalar JSON strings. Consider adding "1.10", "true", "null" and a JSON-quoted string, including nested values, to pin the behavior that non-template callers still rely on.

Done in 04cb220. Default mode now has cases for "1.10", "true", "null", a JSON-quoted string, a list of those, and a dict with them nested two levels down. They pass on both main and this branch, so they pin what non-template callers get today.

andreatnvidia

Thanks for keeping this focused and covering all template consumers. Please address the comments on skip-condition guidance and default-mode scalar coverage before merging.

Document that text "false"/"null" are truthy in skip.when and that explicit comparisons are the safe pattern for text flags. Signed-off-by: Dmitry Voropaev <dy.voropaev@gmail.com>

greptile-apps · 2026-10-30T18:57:43Z

+        ),
+    ],
+)
+def test_deserialize_json_values_keep_scalar_strings(input_data, expected_result):


Required types are missing

The new test_deserialize_json_values_keep_scalar_strings function has no parameter or return annotations. The repository guide requires annotations on all functions, including tests. Add them here and to test_image_cell_generator_keeps_scalar_strings_in_prompt; the new keep_scalar_strings parameter on deserialize_json_values also needs an annotation. This requirement must be satisfied before merging.

Context Used: AGENTS.md (source)

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/data-designer-engine/tests/engine/processing/test_utils.py Line: 169 Comment: **Required types are missing** The new `test_deserialize_json_values_keep_scalar_strings` function has no parameter or return annotations. The repository guide requires annotations on all functions, including tests. Add them here and to `test_image_cell_generator_keeps_scalar_strings_in_prompt`; the new `keep_scalar_strings` parameter on `deserialize_json_values` also needs an annotation. This requirement must be satisfied before merging. **Context Used:** AGENTS.md ([source](https://github.com/nvidia-nemo/datadesigner/blob/main/AGENTS.md)) --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fixed in 9105c1d.

greptile-apps · 2026-10-30T18:57:43Z

        return data_copy


+def _loads_json_string(value: str, keep_scalar_strings: bool) -> Any:


Functions break the required order

The new _loads_json_string helper appears before the public parse_list_string function. The repository guide requires public functions before private ones. Move the helper below the public functions before merging.

Context Used: AGENTS.md (source)

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/data-designer-engine/src/data_designer/engine/processing/utils.py Line: 110 Comment: **Functions break the required order** The new `_loads_json_string` helper appears before the public `parse_list_string` function. The repository guide requires public functions before private ones. Move the helper below the public functions before merging. **Context Used:** AGENTS.md ([source](https://github.com/nvidia-nemo/datadesigner/blob/main/AGENTS.md)) --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fixed in 9105c1d.

andreatnvidia · 2026-10-30T21:20:11Z

Thanks for the fix and for adding the migration guidance.

I'm just not sure about the compatibility tradeoff here. Existing expressions like {{ quantity + 1 }} work with text values such as "5" today and will now need a cast. Making this opt-in would avoid that break, but having two behaviors feels a bit heavy for this. @nabinchha wdyt, is the current migration guidance enough, or do you see a better approach?

Signed-off-by: Dmitry Voropaev <dy.voropaev@gmail.com>

v0ropaev requested a review from a team as a code owner October 30, 2026 13:58

andreatnvidia reviewed Oct 30, 2026

View reviewed changes

andreatnvidia requested changes Oct 30, 2026

View reviewed changes

test: pin skip.when text flags and default-mode scalar parsing

04cb220

Document that text "false"/"null" are truthy in skip.when and that explicit comparisons are the safe pattern for text flags. Signed-off-by: Dmitry Voropaev <dy.voropaev@gmail.com>

greptile-apps Bot reviewed Oct 30, 2026

View reviewed changes

chore: annotate new tests and move private helper below public functions

9105c1d

Signed-off-by: Dmitry Voropaev <dy.voropaev@gmail.com>

Navigation Menu

Repository navigation

fix: keep scalar strings as text in template contexts - #1119

fix: keep scalar strings as text in template contexts#1119
v0ropaev wants to merge 3 commits into
NVIDIA-NeMo:mainfrom
v0ropaev:fix/template-keep-scalar-strings

v0ropaev commented Oct 30, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Oct 30, 2026 •

edited

Loading

Uh oh!

andreatnvidia Oct 30, 2026

Uh oh!

v0ropaev Oct 30, 2026

Uh oh!

andreatnvidia Oct 30, 2026

Uh oh!

v0ropaev Oct 30, 2026

Uh oh!

andreatnvidia left a comment

Uh oh!

greptile-apps Bot Oct 30, 2026

Uh oh!

v0ropaev Oct 30, 2026

Uh oh!

greptile-apps Bot Oct 30, 2026

Uh oh!

v0ropaev Oct 30, 2026

Uh oh!

andreatnvidia commented Oct 30, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		return data_copy


		def _loads_json_string(value: str, keep_scalar_strings: bool) -> Any:

Conversation

v0ropaev commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📋 Summary

🔗 Related Issue

🔄 Changes

Release note

🧪 Testing

✅ Checklist

Uh oh!

greptile-apps Bot commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 4/5

Findings

Summary

Uh oh!

andreatnvidia Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

v0ropaev Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

andreatnvidia Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

v0ropaev Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

andreatnvidia left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

v0ropaev Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

v0ropaev Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

andreatnvidia commented Oct 30, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

v0ropaev commented Oct 30, 2026 •

edited

Loading

greptile-apps Bot commented Oct 30, 2026 •

edited

Loading