Skip to content

Read references with the parser, not with str.split (#104) - #458

Merged
jeremymanning merged 1 commit into
mainfrom
fix/reference-extraction
Aug 3, 2026
Merged

Read references with the parser, not with str.split (#104)#458
jeremymanning merged 1 commit into
mainfrom
fix/reference-extraction

Conversation

@jeremymanning

Copy link
Copy Markdown
Member

Follow-on from the triage in #457. The single largest remaining failure cluster — Undefined task reference across 23 files — was mostly the validator misreading its own input.

The defect

_extract_template_variables chopped raw template text: split('|')[0], then split(' ')[0]. That does not survive an expression, so the fragment was looked up as a task id and reported against the pipeline:

Undefined task reference: '(row'
Undefined task reference: 'from_json)'
Undefined task reference: 'analysis_topics[loop'
Undefined task reference: '(fact_check_loop'

None of those is a name anyone wrote.

Jinja already knows what a reference is, so it's asked instead — the same move #451 made for global calls. The AST also supplies what text-chopping could not: which names a {% for %} binds, and where a chain like a.b.c starts and stops.

This catches more, not less

A 14-file jump in the catalogue is exactly the shape of a validator going blind, so here's the check. split(' ')[0] on

{{ 'x' if ghost_step.done else 'y' }}

yielded 'x' — a literal, discarded — so a reference to a step that does not exist went entirely unchecked. It's an error now, and test_a_typo_inside_an_expression_is_now_caught pins it. Verified end to end through the CLI as well:

  - Undefined variable: 'ghost_step'
  - Undefined task reference: 'ghost_step'
Validation ✗ FAILED (STRICT)

Two smaller fixes in the same area

  • loop.index was an undefined task. loop is Jinja's own variable and item is ours, but the allowlist compared the whole reference — so a bare loop passed while {{ loop.index }}, the way anyone actually writes it, did not. It compares the base name now.
  • {{ range(3) }} was a task named range. Jinja's globals and the pipeline's are both excluded.

Measured

before after
catalogue validating 34 / 117 48 / 117
blocking suite 711 passed 725 passed, 0 failed

For context, the catalogue was at 9/117 when this line of work started.

Mutations — three, all killed

mutation tests that failed
restore the text-chopping extractor 7
compare the whole reference to the loop allowlist again 2
stop excluding environment-provided names 2

🤖 Generated with Claude Code

`_extract_template_variables` chopped raw template text: `split('|')[0]`,
then `split(' ')[0]`. That does not survive an expression, so anything with a
space or a bracket in it came out mangled -- and the fragment was then looked
up as a task id and reported against the pipeline:

    Undefined task reference: '(row'
    Undefined task reference: 'from_json)'
    Undefined task reference: 'analysis_topics[loop'
    Undefined task reference: '(fact_check_loop'

None of those is a name anyone wrote. Jinja already knows what a reference
is, so it is asked instead -- the same move #451 made for global calls. The
AST also gives what text-chopping could not: which names a `{% for %}` binds,
and where a chain like `a.b.c` starts and stops.

This catches *more*, not less. `split(' ')[0]` on

    {{ 'x' if ghost_step.done else 'y' }}

yielded `'x'` -- a literal, discarded -- so a reference to a step that does
not exist went unchecked. It is an error now, and there is a test for it,
because a 14-file jump in the catalogue is only trustworthy with one.

Two smaller fixes in the same area:

- `loop` is Jinja's own variable and `item` is ours, but the allowlist
  compared the *whole* reference. A bare `loop` passed while `{{ loop.index }}`
  -- the way anyone actually writes it -- was reported as an undefined task.
  It compares the base name now.

- `{{ range(3) }}` names a function the environment provides, not a step.
  Jinja's globals and the pipeline's are both excluded.

Measured: catalogue validating 34 -> 48 of 117; blocking suite 711 -> 725
passed, 0 failed.

Three mutations, all killed:
  - restore the text-chopping extractor      -> 7 tests
  - compare the whole reference to the loop  -> 2 tests
    allowlist again
  - stop excluding environment-provided      -> 2 tests
    names

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jeremymanning
jeremymanning merged commit febadfb into main Aug 3, 2026
9 checks passed
@jeremymanning
jeremymanning deleted the fix/reference-extraction branch August 3, 2026 12:05
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