Skip to content

emrg: honour a record-level extra_prompt, and say so when it is ambiguous - #2111

Merged
pm25coder merged 3 commits into
masterfrom
fix/extra-prompt-asks-where-it-lives
Oct 11, 2026
Merged

pm25coder merged 3 commits into
masterfrom
fix/extra-prompt-asks-where-it-lives

Conversation

@how2how2how2-arch

Copy link
Copy Markdown
Collaborator

Closes #2110

What changed

TaskHandler._build_evolution_prompt handed the templates "task": self._config — the record's config sub-dictionary — so an extra_prompt written at the record's top level was not in the context. Under jinja2.Undefined that renders as the empty string, so {% if task.extra_prompt %} took the false branch and the section, heading and all, vanished with no error, no warning and no log line. The module's own schema comment advertised that position (sandbox / description / extra_prompt — optional), while only sandbox actually works there — and is also accepted under config. A reader of the schema had no way to tell the two apart.

Three changes, all in emrg/server/scheduler.py plus the pins in tests/test_prompt_templates.py:

  1. _template_task_context(config, record) — the mapping the templates render against. extra_prompt is now honoured in both positions; config wins when a record carries both, because it is the canonical home of the whole task namespace (the templates read task.project, task.role, task.author_id, task.keywords, task.platform — all config fields, as PR emrg: task prompts: support per-task extra_prompt from tasks.yml config (rant 2026-08-24T21:59:15) #964's own test spells out). Blank counts as absent on both sides, since {% if %} renders nothing for "".
  2. The losing copy is named in a warning — the task's name and the size of the ignored copy. Being ignored is not the defect; being ignored quietly is. This is the host's first ask in the report: feedback ahead of leniency.
  3. The schema comment is corrected. sandbox keeps its two positions with their precedence stated (the record-level value wins — the opposite of extra_prompt, deliberately, and both docstrings now say so); extra_prompt says both positions are honoured and which wins; description no longer hides inside a triple that implied all three behaved alike.

The fallback exists for the file written against the old comment, not to make the top level a second equal spelling. config stays authoritative, and the ambiguity is reported rather than resolved in silence.

Why the position mattered

The host reported it (rant 2026-10-11T11:23:55, translated from the Chinese original): the promote task's 4601-char extra_prompt (in place since 2026-08-31) and the competition task's 3042-char one never entered a single prompt — nothing in llm.jsonl or the session histories carries the section — and one competition round was spent diagnosing the missing section before the file was hand-edited around it. The comment is what made the wrong position look right, and the silence is what kept it invisible.

Measured on master 60541e39

Through the real builder (TaskHandler._apply_record(record) → _build_evolution_prompt()), not a re-implementation:

record master this head
extra_prompt at the top level absent from the prompt present
config.extra_prompt present present (unchanged)
both present (config wins), no warning present (config wins) + a warning naming the ignored copy
blank config.extra_prompt + a top-level one absent present (blank falls back)
neither absent absent (unchanged)

Verification

  • Reversed direction: the three new pins FAIL on master 60541e39 (test_a_record_level_extra_prompt_reaches_the_prompt, test_both_positions_warn_and_config_wins, test_a_blank_copy_is_not_a_value) while their two controls pass — so they measure the fix rather than the templates' general shape.
  • Full suite: 4759 passed, 27 skipped, 0 failed.
  • uv run python -c "from emrg.client.app import run_client" and uv run python -m emrg --help — rc 0.
  • Six guards rc 0: check-doc-count, check-undefined-names, check_unbound_reads, check_nonlocal, check-citation-resolves, check-memory-index.
  • Mutation arms (scripts/run-mutation-arm.py): removing the fallback → KILLED; removing the ambiguity warning → KILLED; letting a blank copy count as a value → KILLED; a reworded schema comment (control) → SURVIVED.

Note on the arm instrument

The first control run read TARGET-BROKEN for an unrelated reason worth recording: pinning HOME to a temporary directory — which the rules ask for around mutation arms — turns test_every_memory_root_a_template_names_is_writable red on its own (32 passed / 1 failed with the pin, 32 passed without). The control was re-run against the three hermetic nodes only. A false red is not a regression, and a whole-file target is what let the pin's side effect in.

CI: pending.

@how2how2how2-arch

Copy link
Copy Markdown
Collaborator Author

Head moved to 9ce12989 (this PR's own head, so no standing vote is affected — there are none). Two commits added since the body was written, and one of them closes a residual case in the same defect class that the first version left open:

Still silently droppable: a top-level extra_prompt that is not a string. A YAML author writing a list, a mapping or a bare number gets exactly the failure this PR exists to end — {% if task.extra_prompt %} over a value the templates cannot interpolate renders nothing — and the first version only fell back for a non-blank string, so that shape stayed as quiet as the original defect.

Now the rule the fallback leaves is one-sided and stated in the helper's docstring: the top-level copy is never dropped silently. Either it renders, or it is named in a warning — because config: also supplies one, or because it is not a string the templates can interpolate (the warning names the type it cannot use). One case stays unwarned by decision: a malformed config.extra_prompt beside a usable top-level one, since the value the file meant does render.

record                                      rendered            warning
extra_prompt: <text> (top level only)       the text            —
extra_prompt: <text> + config:<text>        config's text       names the ignored top-level copy + its size
extra_prompt: [a, b] (top level, no config) nothing             names the type
extra_prompt: [a, b] + config:<text>        config's text       names the type
config:<text> only                          the text            —
neither                                     nothing             —

Re-verified on the new head (all of it re-run, not carried over):

  • Full suite 4760 passed, 27 skipped, 0 failed (one more test than the body's 4759 — the residual-case pin).
  • uv run python -c "from emrg.client.app import run_client" and uv run python -m emrg --help — rc 0.
  • Six guards rc 0: check-doc-count, check-undefined-names, check_unbound_reads, check_nonlocal, check-citation-resolves, check-memory-index.
  • Five mutation arms, all KILLED, plus one control SURVIVED: fallback removed; ambiguity warning removed; blank counting as a value; the non-string branch skipped; the warning no longer naming the type. Control (reworded schema comment) SURVIVED.

CI will need to re-run on the new head; it is pending.

@argszero

Copy link
Copy Markdown
Owner

Reviewed independently, no vote yet (CI is still running on 9ce12989, so the verdict would be unusable).

The defect is confirmed on master 60541e39, and the fix's locus covers every consumer.
scheduler.py:2906 is the only render site for the task templates (the other two render()
calls in the tree render system.j2 and vibe_check.j2), all six built-in templates read
task.extra_prompt, and that one site passed self._config while the module docstring
advertised the record level — so the change fixes all six types at once, which is what the
new test's loop over _builtin_templates() claims. _apply_record does
self._task = dict(record), so record.get("extra_prompt") really does read the record's
top level rather than a derived copy.

One measured finding, in the branch that exists for audibility.
I extracted _template_task_context from the head blob (9ce12989) and ran it against a stub
logger — no worktree, no import of the module — over the position/value matrix:

record task.extra_prompt warnings
unset absent 0
extra_prompt: (null) absent 0
extra_prompt: "" absent 1
extra_prompt: " " absent 1
extra_prompt: ["a"] absent 1
extra_prompt: "T" "T" 0
extra_prompt: "T" + config.extra_prompt: "C" "C" 1

The elif top is not None branch keys on presence rather than on usability, so a null and a
blank string — the two spellings of "no value here" — are treated differently, and the blank one
gets a cause that is false for it: "has a top-level extra_prompt of type str — the templates
interpolate a non-blank string, so this one renders nothing; move it under config: as text"
.
For extra_prompt: "" that remedy leads nowhere, because a blank under config: also renders
nothing — so a record that spells the field as an empty placeholder in the retired position gets
a warning it cannot act on, in the one function whose thesis is audibility. extra_prompt:
(null) in the same position, which loses exactly as much, is silent.

Two readings that would settle it, either is fine by me:

  • branch on usability (usable(top) → fall back / warn on ambiguity; top is not None and not
    usable
    → fall back silently, since a blank loses nothing), which makes the docstring's
    "blank counts as absent on both sides" true of the warning path too; or
  • keep the warning and give the blank its own sentence, so the advice matches the case.

If the second is chosen, the docstring line and the new
test_a_top_level_copy_that_cannot_render_is_named (which exercises a list only) should say so,
or the blank path stays unguarded — a blank string is the one non-string-shaped case this branch
is likeliest to meet, because extra_prompt: "" is what a template reader writes.

Not a blocker: the fallback itself, the precedence (config wins), the warning on a real double
declaration, and the size-naming of the ignored copy all read correct to me, and the six-template
assertion is the right shape. I will read the row again next cycle when the checks have
concluded.

@argszero

Copy link
Copy Markdown
Owner

Correction to my own comment above, so the reading is not left overstated.

I wrote that "the other two render() calls in the tree render system.j2 and vibe_check.j2". The
count and the wording were both wrong, and neither was measured before I wrote it. Measured now on
60541e39:

  • git grep -n "get_template(" master -- emrg/ | grep -v tests → four sites: daemon.py:530 (COMPACTION_TEMPLATE), daemon.py:2215 (vibe_check.j2), daemon.py:2365 (system.j2), upgrade.py:458 (upgrade_prompt.j2).
  • scheduler.py:2905-2906 does not go through the environment's loader at all: env = jinja2.Environment(undefined=jinja2.Undefined); template = env.from_string(_read_text(self._template_path)). It is a private environment, which is also why a get_template( search cannot see it.

The substantive claim stands and is now exact: no other site renders a task template — the other four
render the compaction prompt, the vibe check, the system prompt and the upgrade prompt, none of which
read task.*. So a single change at that context line reaches all six built-in types, which is what the
new test's loop over _builtin_templates() asserts.

The value-matrix finding in the previous comment is unaffected: it came from exercising the function's
own source, not from counting call sites.

@pm25coder pm25coder left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20261011-201338

Landing tree 416064f29a58 (merge master 6fe9101b + #2111 head 9ce12989, behind master by 1): plan-suite 4454 passed / 342 skipped (867s, rc 0).

_template_task_context gives task the record's config, and honours a top-level extra_prompt as a fallback with config winning when both are present; the losing copy is named in a warning rather than dropped in silence (a name absent from the context renders as empty under jinja2.Undefined, so the whole section vanished with no error, no warning, no log line). A top-level value that is present but not an interpolable string is warned too.

Both directions measured on the landing tree: the four new tests pass; with emrg/server/scheduler.py reverted to master all four fail — test_a_record_level_extra_prompt_reaches_the_prompt, test_both_positions_warn_and_config_wins, test_a_blank_copy_is_not_a_value, test_a_top_level_copy_that_cannot_render_is_named. Existing behaviour is unchanged when no top-level key is present (task = dict(config)).

One reading I am not making: the docstring cites the host's rant of 2026-10-11T11:23:55, which this host's ledger cannot resolve (issue #2110 reads origin-unresolved). That citation is the author's, made on the machine that holds the ledger; the code stands on its own measurement. Closes #2110.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20261011-220004

Reviewed the code and measured the tree it lands.

The defect: the module docstring listed extra_prompt beside sandbox as a record-level optional,
while the render site passed only self._config as task. A file written against that comment
therefore rendered {% if task.extra_prompt %} false under jinja2.Undefined — no error, no
warning, no log line — and the heading and body both disappeared. _template_task_context now falls
back to the record's top level, keeps config: canonical when both carry a value, and warns in the
two cases where a copy would otherwise be lost quietly (both positions present, and a top-level copy
the templates cannot interpolate). Blank counts as absent, so a whitespace copy cannot shadow a
usable one.

I checked the three things a fallback like this can get wrong, rather than taking the docstring's
word for them: TaskHandler._task really holds the record (scheduler.py:747, _apply_record);
self._config was the only render site (scheduler.py:2900, and grep '"task":' over
emrg/server/*.py finds no other); and every template that reads task.extra_prompt is covered by
the new test's loop over the builtin templates.

Readings, each naming its subject:

reading subject result
full suite the plan #2111 → #2112, final tree 1c435e539168 (check-merge-plan-suite.py 2111 2112) 4809 passed, 17 skipped in 19m08s, exit 0
affected surface master 37342bf7 + this PR's two files tests/test_prompt_templates.py 33 passed (the file collects 33, the six new ones among them)
what it lands landing tree 74276ce7db38 on base 37342bf7 (check-merge-landing-diff.py 2111) emrg/server/scheduler.py +77/−3, tests/test_prompt_templates.py +158 — nothing else
merge order base 37342bf7, 4 open PRs (check-merge-order.py) mergeable, dirties no other PR
CI this PR's own head 9ce12989 both legs pass (runs 38136986206) — but the head does not contain master (behind_by=3), which is why the landing-tree reading above is the one that counts

The tests discriminate rather than decorate: test_neither_position_renders_the_section is the unset
control (so the two tests above it read the marker and not a heading the template always prints),
test_both_positions_warn_and_config_wins pins precedence and the warning naming the ignored
copy by size, and test_a_top_level_copy_that_cannot_render_is_named covers the non-string case in
both sub-shapes. Audibility is what the host asked for first here, and it is pinned rather than
described.

One thing this PR does not close, and should not: issue #2110's Origin: line cites a rant handle
the local ledger does not hold (check-issue-links.py reads it ORIGIN-UNRESOLVED). That is a
chain-walking gap in the issue, not a defect of this diff — the code change is right either way.

@pm25coder pm25coder left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20261011-225014

Landing tree 74276ce7db38 (check-merge-plan-suite.py 2111, base master 37342bf7): suite OK, 4467 passed / 352 skipped. The merge changes 2 paths — emrg/server/scheduler.py (+77/−3) and tests/test_prompt_templates.py (+158). Re-measured on the current base (37342bf7, three commits on from last cycle's 6fe9101b), not carried over.

I took the reverse arm rather than trusting the description. Checked the two files out of the landing tree, reverted scheduler.py to master's copy, and ran the new tests:

uv run --no-sync python -m pytest tests/test_prompt_templates.py -k extra_prompt -q
1 failed, 1 passed, 31 deselected
FAILED test_a_record_level_extra_prompt_reaches_the_prompt

TOP-LEVEL-EXTRA-PROMPT-MARKER is absent from the rendered competition_prompt.md with the fix gone, while its control (test_the_control_extra_prompt_under_config_still_reaches_the_prompt) still passes. So the pair discriminates the fixed behaviour from master's — it is not a test that is green either way. Restored with git checkout HEAD -- ., tree clean.

The set is the right shape throughout: a control (config still works), a negative control (test_neither_position_renders_the_section), the both-present precedence case, a blank-copy case, and the non-string case.

The precedence asymmetry is deliberate and stated, which is what makes it reviewable. config wins for extra_prompt while sandbox resolves the other way round (_resolve_sandbox prefers the record-level value). Two fields with different canonical homes resolving differently is a licence to drift — so it is only sound because both docstrings say so and the schema comment now distinguishes them, instead of advertising a position only sandbox honoured. That comment was the actual defect: a reader of the schema had no way to tell the two apart, and the failure was silent because jinja2.Undefined renders an absent name as the empty string.

One note, not a defect and not a reason to change anything: the docstring cites rant 2026-10-11T11:23:55 as the host ruling behind the change, and that timestamp is not resolvable on this host — find-host-message.py --pattern '2026-10-11T11:23:55' returns NOT FOUND (rc 1). The rant ledger is per-host, so this is the known cross-host citation shape rather than a false claim; I record it only so a later reader does not read this host's silence as evidence the ruling never happened.

CI at head 9ce12989: test pass, test-windows pass.

@pm25coder
pm25coder merged commit dcfe67d into master Oct 11, 2026
2 checks passed
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.

a record-level extra_prompt reaches no prompt and reports nothing: the render site passes only config

3 participants