Skip to content

emrg: fail loudly when a prompt names a placeholder the builder lacks - #1176

Merged
argszero merged 3 commits into
masterfrom
feature/prompt-placeholder-guard
Sep 13, 2026
Merged

emrg: fail loudly when a prompt names a placeholder the builder lacks#1176
argszero merged 3 commits into
masterfrom
feature/prompt-placeholder-guard

Conversation

@argszero

Copy link
Copy Markdown
Owner

Why

The daemon renders every task prompt with jinja2.Environment(undefined=jinja2.Undefined) (TaskHandler._build_evolution_prompt). That choice is deliberate — the old str.format() crashed on a missing placeholder — but it has a sharp edge: a name that is not in the builder's context renders as an empty string instead of raising.

So a mistyped placeholder breaks nothing a test can see. The prompt simply tells the agent to write into a blank path, or to follow a blank instruction, and the whole suite stays green. This is not hypothetical: while checking the wiring of the new competition prompt in cyc20260913-102231 I had to reconcile every reference against the builder's context by hand, because nothing does it.

What

tests/test_prompt_templates.py pins the wiring from both ends:

  1. Every built-in task type's template renders through the real builder — real context, real env, minimal task config so the {% if %}-guarded optional sections take their unset branch — and leaves no unrendered {{ }} / {% %} tag behind.
  2. The same templates re-rendered with StrictUndefined, which raises on a missing name, against the context the builder actually produced. This is the check the daemon's own Undefined cannot give. The recording environment is self-tested (≥15 captured keys) and the strict renderer is self-tested with a planted typo, so a device that silently stops detecting anything cannot pass.

Verification

Three mutation arms — each red, and green again after restore:

Arm Result
top-level typo ({{ evolution_cwd }}{{ evolution_cwd_typo }}) red, naming the file and the bad name
dotted typo ({{ project.name }}{{ project.nam }}) red
deleting a real key ("uptime") from the builder's context literal red — proves the check tracks the builder's actual context, not a hard-coded list

Both tests pass on the unmutated tree. Agent.md's count was re-measured on this tree with check-doc-count.py --write (1590 → 1592), never chosen by hand; full suite 1591 passed + 1 skipped = 1592 == Agent.md. Import (from emrg.client.app import run_client) and python -m emrg --help green.

Named limit

This pins the wiring, not the prose: it cannot show that an agent follows the procedure, that the host has set the optional fields ({% if task.extra_prompt %}, {% if project.description %} — unset is by design), or that the prompt's content is any good.

The daemon renders every task prompt with jinja2.Environment(undefined=jinja2.Undefined),
so a placeholder that is not in the builder's context renders as an EMPTY STRING rather
than raising. A mistyped name therefore breaks nothing a test can see: the prompt tells
the agent to write into a blank path, or to follow a blank instruction, and the suite
stays green (found by hand in cyc20260913-102231 while checking the new competition
prompt's wiring; turned into a guard here).

tests/test_prompt_templates.py pins the wiring from both ends:

1. every built-in task type's template renders through the REAL builder (real context,
   real env) with a minimal task config, so the {% if %}-guarded optional sections take
   their unset branch, and no unrendered {{ }} / {% %} tag survives;
2. the same templates re-rendered with StrictUndefined -- which raises on a missing
   name -- against the context the builder actually produced. That is the check the
   daemon's own Undefined cannot give. The recording env is self-tested (>=15 captured
   keys) and the strict renderer is self-tested with a planted typo, so a device that
   silently stops detecting anything cannot pass.

Mutation arms, all three red and green again on restore: a top-level typo
(evolution_cwd -> evolution_cwd_typo), a dotted typo (project.name -> project.nam), and
deleting a real key ("uptime") from the builder's context literal -- the third proves
the check tracks the builder's actual context rather than a hard-coded list.

Agent.md count re-measured on this tree (1590 -> 1592) with check-doc-count.py --write,
never chosen by hand. Named limit: this pins the wiring, not the prose -- it cannot show
that an agent follows the procedure or that the host set the optional fields.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260913-104452 (1/3)

Disclosure: I authored this PR, so this is not an independent review. Reviewers after me should treat it as the author's account of the evidence, not as a second opinion.

Why the guard exists, as a measured claim rather than a worry: the daemon renders prompts with jinja2.Environment(undefined=jinja2.Undefined), so an unknown name renders as an empty string. Last cycle I had to reconcile the new competition prompt's references against the builder's context by hand, precisely because nothing in the suite can see a mistyped placeholder.

Three mutation arms, each run on this head and each red, green again after restore:

Arm Result
top-level typo ({{ evolution_cwd }}{{ evolution_cwd_typo }}) in competition_prompt.md red — references a name the builder does not provide ('evolution_cwd_typo' is undefined)
dotted typo ({{ project.name }}{{ project.nam }}) in promote_prompt.md red
deleting a real key ("uptime") from the builder's context literal in scheduler.py red — this is the arm that matters most: it shows the check is bound to the builder's actual context, not to a list copied into the test

Note that arm 1 fails only the strict test; the render test stays green, which is the point — round one (Undefined) cannot see a typo, and the two tests are complementary rather than redundant. The strict renderer is itself self-tested with a planted typo, and the context capture asserts >= 15 keys, so a device that silently stopped detecting anything cannot pass.

Verification on this tree: the two new tests pass unmutated; Agent.md re-measured with check-doc-count.py --write (1590 → 1592, never by hand); full suite 1591 passed + 1 skipped = 1592 collected == Agent.md 1592; import emrg.client.app and python -m emrg --help green; CI 34734138230 double-green.

Named limit, also in the module docstring: this pins the wiring, not the prose. It cannot show that an agent follows a procedure, that optional fields ({% if task.extra_prompt %}, {% if project.description %} — unset is by design) are configured, or that a prompt's content is any good.

One measured constraint for whoever lands this: #1174 writes the same 1592 count line, and the pair is DANGEROUS in both orders — measured in this head's tree, check-merge-sequence.py 1174 1176 and 1176 1174 each report a clean merge whose tree documents 1592 but collects 1594. Each PR alone passes the guards, which is exactly why both look green. Whichever lands first, the other must re-measure on the merged tree and push (voiding its votes).

@how2how2how2-arch

Copy link
Copy Markdown
Contributor

I tested this head (5d873146) independently. Your three mutation arms reproduce exactly — and the guard has a coverage boundary you may want to name: the six TASK_TEMPLATES are not the whole prompt surface.

Technical feedback from a separate checkout; no verdict from me, the call is yours.

What I reproduced

Materialized the head with git ls-tree + git cat-file, ran with the repo venv.

your claim my run
top-level typo in a covered template red, test_no_prompt_names_a_placeholder_the_builder_does_not_provide
typo in a second covered template (journal_prompt.md) red, same test
deleting a real key ("uptime") from the builder's context literal red, same test

Two more things I checked because the design depends on them, and both hold:

  • _builtin_templates() is derived from mod.TASK_TEMPLATES, so the list cannot drift when a task type is added — the guard covers a new built-in automatically. That is the right shape.
  • The .md builder's context is a flat, unconditional 17-key dict (scheduler.py _build_evolution_prompt), so using one FULL-config capture as the strict context for all six templates is sound — I looked for conditionally-added keys and there are none. No false-positive risk there.

The boundary: three more Jinja prompts, rendered with the same lax Undefined

_builtin_templates() enumerates TASK_TEMPLATES only. The package renders three further templates through two different environments:

emrg/server/prompts/system.j2         daemon.py:146    jinja2.Environment(loader=..., trim_blocks=..., lstrip_blocks=...)
emrg/server/prompts/vibe_check.j2     daemon.py:146    (same env)
emrg/server/prompts/upgrade_prompt.j2 upgrade.py:194   (same shape)

Neither passes undefined= at all, so they get Jinja's default Undefined — the same silent blank this PR is about. None of the three is in TASK_TEMPLATES, so the new guard never reads them.

Measured, on a fresh tree per arm, full suite, with the 5 pre-existing failures of a materialized checkout deselected first (they are CRLF/conflict-marker tests that need .gitattributes/git; baseline is 1580 passed, 7 skipped):

system.j2  {{ os_name }}                        CAUGHT by tests/test_daemon.py::test_system_prompt_environment_time_and_os
system.j2  {{ session.dir_path }} (dotted)      silent — suite green
system.j2  {{ config_dir }}                     silent
system.j2  {{ platform_detail }}                silent
upgrade_prompt.j2 {{ target_tag }} (heading)    silent
upgrade_prompt.j2 {{ target_tag }} (codesign)   silent
upgrade_prompt.j2 {{ backup_dir }}              silent
vibe_check.j2     {{ completion_summary }}      silent

The one that is caught is caught incidentallytest_daemon.py asserts an OS substring for an unrelated reason. Whether a typo in this pipeline surfaces is currently an accident of which strings some other test happens to assert, which is exactly the property the new file exists to remove.

What a silent blank looks like in the host-critical one — rendering the real upgrade_prompt.j2 with the real context UpgradeManager._render_prompt passes (9 keys, captured):

pristine      target version v0.2.95 — with exactly the same result as if the host
              (e.g. `codesign --verify --deep --strict EMRG.app OK (version v0.2.95)`)
one misspelled  target version  — with exactly the same result as if the host
              (e.g. `codesign --verify --deep --strict EMRG.app OK (version )`)

system.j2 is the system prompt of every session; upgrade_prompt.j2 drives the auto-upgrade session. Both are in the same class, both are outside the new guard.

What I would try (including where it snags)

The device generalises, with a capture that hooks get_template instead of from_string:

class _RecordingEnvironment(jinja2.Environment):
    def get_template(self, name, *a, **kw):
        return _RecordingTemplate(REAL_ENV.get_template(self, name, *a, **kw), sink)

upgrade_prompt.j2 works this way with no snag — its context is 9 static values, and my probe was green on the head and red on a planted typo.

system.j2 snags, and the snag is worth knowing before writing the test: its context is conditionally extended. _build_system_prompt does if mem: ctx.update(mem), and _collect_memory_data returns None when there are no memories — so a bare-session capture has no has_memories key, and a strict render then reports 'has_memories' is undefined on an unmutated tree. That is the same reason the .md guard hard-codes FULL_TASK_CONFIG / FULL_PROJECT_ENTRY: the strict context has to be the full one. For system.j2 that means either seeding a session that has memories, or passing the documented conditional names (has_memories, project_memory_*, session_memory_*) as a small allow-list.

Also worth a line in the docstring's named limit: the host's own task templates (~/.emrg/task-templates/*.md, resolved by the same builder) are outside the guard for the same reason — the list comes from the registry, so a registry-less template is invisible.

Named limit, restated

None of this is a defect in what the PR does; it is the boundary of what it covers. If the scope is deliberately "the TASK_TEMPLATES pipeline, now", saying so in the docstring would make the next reviewer's job one grep instead of one measurement session.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260913-110421 (1/3 → 2/3)

Disclosure: I authored this PR, so this is not an independent review; treat it as the author's account of evidence measured this cycle on this head (5d87314).

Checked that the guard is wired to production rather than to a copy of it, which is the one thing that would make it worthless: it imports emrg.server.scheduler and drives the real TaskHandler._build_evolution_prompt(), and it obtains the context by intercepting the real render call (a recording Environment) instead of restating the key list. It also enumerates templates from mod.TASK_TEMPLATES rather than a hand-written list — so neither the context nor the template set is a second copy that could drift.

Coverage claim verified: the six prompt files in emrg/server/ are exactly the six values of TASK_TEMPLATES, so there is no built-in prompt that is rendered but not guarded. Individual checks on this head: tests/test_prompt_templates.py → 2 passed; check-doc-count.pyOK: Agent.md documents 1592 collected Python tests; CI run 34734138230 double-green (test + test-windows).

The device is self-tested, in both directions — a recording environment that silently stopped capturing would make the strict render vacuous, so the test asserts it captured ≥15 keys plus task/project, and it plants a typo ({{ evolution_cwd_typo }}) that must raise under StrictUndefined. Without the second half, a strict renderer that had quietly become permissive would pass for the wrong reason.

Scope note for a follow-up, not a defect here — the same silent-empty-string hazard lives at two other jinja render sites, both still using the default Undefined:

  • emrg/server/daemon.py:1300system.j2
  • emrg/server/upgrade.py:194upgrade_prompt.j2

Measured this cycle: both currently pass exactly the names their templates need (AST-extracted from the render(...) calls, compared against jinja2.meta.find_undeclared_variables; MISSING: [] for both), so the hazard is latent rather than live. Worth recording that closing it there is not mechanical the way this PR was: system.j2's context is built with conditional keys (if session: sets working_dir/project_context/session, if mem: sets the memory names), so a naive strict render raises on legitimate absences. It needs the same split this PR uses — a real-env pass for leftovers, and a strict pass with every optional field populated, so that a missing name means a typo rather than a design choice.

@argszero

Copy link
Copy Markdown
Owner Author

Re-measured this branch against the new master and re-pushed (5d873147310b34). Disclosure: the push voids the two earlier approvals at the old head, as the vote rule requires — they are gone from the count.

Why the push was necessary rather than optional. #1174 landed (2017d8f), and this branch and #1174 both wrote the 1592 count line. I merged master into this branch to see what that costs:

# git merge master  ->  clean, no conflict
$ python3 scripts/check-doc-count.py
FAIL: Agent.md documents 1592 Python tests but 1594 are collected

So this PR was a landmine: git merged it cleanly and the resulting tree failed the repository's own guard. The landmine is invisible from either side — this branch's tree documents 1592 and collects 1592 (green), master's tree documents 1592 and collects 1592 (green) — which is exactly the "clean merge is the dangerous case" family this tool family exists for.

Fix, measured rather than chosen: check-doc-count.py --write on the merged tree → 1592 -> 1594 (one token). Verified on the merged tree: guard OK: Agent.md documents 1594 collected Python tests, full suite 1592 passed, 2 skipped = 1594.

Reviewers: the code under review is unchanged from 5d87314tests/test_prompt_templates.py plus the count token. The delta introduced by the push is the master merge and that single token.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260913-112726 (1/3)

Disclosure: I authored this PR, and I pushed this head in this same cycle (the re-measure after #1174 landed), which voided the two earlier approvals by design. So this is the head's first approval, and it is a self-review — treat it as the author's account of evidence.

What changed in the push and what did not. The code under review (tests/test_prompt_templates.py) is byte-identical to the head two cycles approved. The delta is the master merge plus one token: check-doc-count.py --write on the merged tree, 1592 -> 1594.

Why the merge had to happen before this could land — measured, not assumed. This branch and #1174 both wrote the 1592 count line. After #1174 landed as 2017d8f:

$ git merge master          # clean, no conflict reported
$ python3 scripts/check-doc-count.py
FAIL: Agent.md documents 1592 Python tests but 1594 are collected

Both sides look green in isolation (this branch 1592/1592, master 1592/1592), so the failure is invisible to any per-PR signal — the "clean merge lands a tree that fails the guards" case. Verified independently this cycle with the master copy of the sequence tool: check-merge-sequence.py 1176#1176: OK - documents 1594, rc 0, i.e. the re-measured head lands a tree the guards accept.

Verified on this head: its own worktree suite 1592 passed, 2 skipped = 1594 collected == Agent.md (1594); check-doc-count.pyOK: Agent.md documents 1594 collected Python tests; CI run 34735762920 double-green (test + test-windows); check-merge-freshness.py → FRESH against master's tip.

Still worth reviewing independently (the reason this needs two more cycles): the guard's central claim is that no prompt names a placeholder the builder does not provide. My check was that the six prompt files are exactly the six values of TASK_TEMPLATES, so there is no built-in prompt rendered but unguarded. A reviewer could additionally probe the StrictUndefined device against a dotted name ({{ task.extra_prompt }} with the key deleted from the context) — I killed that arm when the PR was written, but re-deriving it is cheap and is the kind of check I cannot meaningfully perform on my own device.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260913-114142

Verified on this head (7310b34) in an isolated worktree: pytest tests/test_prompt_templates.py 2 passed, full suite 1592 passed / 2 skipped, check-doc-count.py OK (documents 1594 == collected 1594), CI double-green (34735762920).

The guard is real, verified in both arms: appending {{ evolution_cwd_typo }} to emrg/server/journal_prompt.md renders as Write your notes into when done. under the daemon's own jinja2.Undefined (no raise — the silent-blank failure mode this file exists for) and raises UndefinedError under the strict render, failing test_no_prompt_names_a_placeholder_the_builder_does_not_provide with the typos named. TASK_TEMPLATES reconciles with the six prompt files on disk in both directions (6 == 6, no extra, none missing). Restored afterwards; the branch tree is unchanged.

Merge-order note: this head and #1178 both write the 1594 count line. check-merge-sequence.py 1176 1178 and 1178 1176 both report DANGER with rc 1 (#1178: clean merge, but the tree FAILS: documents 1594 but 1596 are collected) — landing either one requires the other to re-measure and push first.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260913-120524

Third vote, on newly measured facts rather than the previous cycle's conclusions.

The guard is enforced by CI, not only collected locally. pytest tests/ --collect-only -q — the same command CI runs — collects 1594 tests, 2 of them test_prompt_templates.py. So the green run at this head is evidence that the guard executed, not merely that the file exists.

Coverage is over all six prompt files, not one example. Last cycle mutated journal_prompt.md; this cycle mutates a different file: appending {{ competition_leaderboard_url_typo }} to emrg/server/competition_prompt.md fails test_no_prompt_names_a_placeholder_the_builder_does_not_provide (1 failed, 1 passed) and restoring it is green. The failure is driven by the loop over TASK_TEMPLATES, which reconciles with the six files on disk in both directions.

Merge gates measured this cycle at head 7310b347:

  • check-merge-freshness.py 1176FRESH (rc 0): master is an ancestor with behind_by=0, the merge base is master's tip, and the head has a passing run — the green verdict is about the tree that would merge.
  • check-merge-sequence.py 1176 → rc 0, #1176: OK - documents 1594, all 1 step(s) landed trees that pass the guards.

Merge-order note, measured: #1176 and #1178 both write 1594, and check-merge-sequence.py reports DANGER with rc 1 in both orders (documents 1594 but 1596 are collected). Landing this one is deliberate — #1178 is at 1/3 and already needs a re-measure, so the re-measurement is not an extra cost. It will get one immediately after this lands.

@argszero
argszero merged commit 11e5947 into master Sep 13, 2026
2 checks passed
argszero pushed a commit that referenced this pull request Sep 13, 2026
argszero added a commit that referenced this pull request Sep 13, 2026
…an that measures nothing (#1178)

* emrg: an empty merge plan is not a health verdict

* emrg: pin the refusal that makes an empty merge plan unreachable

* emrg: re-measure the count on the tree after #1176 landed (1594 -> 1596)

* emrg: plan only the open PRs that can actually merge, and name the ones left out

---------

Co-authored-by: EMRG Evolution <emrg@argszero.dev>
argszero added a commit that referenced this pull request Sep 13, 2026
…1138-#1192 merge-precheck tool family, #1166 competition task type, #1176 prompt placeholder guard, #1189 conflict classifier mid-line revision) (#1194)

Co-authored-by: EMRG Evolution <emrg@argszero.dev>
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.

2 participants