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

emrg/server/scheduler.py

@@ -10,8 +10,13 @@
 
 Task config schema:
   name, type, enabled, interval — common base fields.
-  config — type-specific config. For evolution: config.project links to projects.yml name.
-  sandbox / description / extra_prompt — optional; anything else is ignored.
+  config — type-specific config, and the mapping the templates see as ``task``. For
+    evolution: config.project links to projects.yml name.
+  sandbox — optional; also accepted under config, where the record-level value wins.
+  extra_prompt — optional; also accepted at the record's top level, where config wins
+    (the templates read it as ``task.extra_prompt``; see ``_template_task_context``).
+  description — optional; carried for humans, read by no code today.
+  Anything else is ignored.
 """
 
 from __future__ import annotations
@@ -456,6 +461,75 @@ def write_table(records: list[dict], path: Path | None = None) -> None:
     atomic_write_yaml(records, path, prefix=".tasks_", dumper=_TaskTableDumper)
 
 
+def _template_task_context(config: dict, record: dict) -> dict:
+    """The ``task`` mapping the prompt templates render against.
+
+    ``task`` is the record's ``config``: every name a template reads there —
+    ``extra_prompt``, ``project``, ``role``, ``author_id``, ``keywords``, ``platform`` —
+    lives under ``config:`` in tasks.yml.
+
+    ``extra_prompt`` is **also honoured at the record's top level**, which is the position
+    this module's schema comment advertised beside ``sandbox`` until 2026-10-11. A file that
+    used that position lost the whole section in silence: the templates read
+    ``{% if task.extra_prompt %}`` under ``jinja2.Undefined``, so a name that is not in the
+    context renders as the empty string, the condition takes the false branch, and the
+    heading and body both disappear — no error, no warning, no log line. Measured
+    2026-10-11 (``cyc20261011-193407``) through the real builder: a record carrying
+    ``extra_prompt`` at its top level rendered **without** it; the same value under
+    ``config`` rendered with it.
+
+    The host reported the cost of that silence (rant 2026-10-11T11:23:55): the ``promote``
+    task's 4601-char ``extra_prompt`` and the ``competition`` task's 3042-char one sat at the
+    top level without ever entering a prompt, and one competition round was spent
+    diagnosing the missing section instead of doing its work. The feedback the host asks
+    for first is not leniency but **audibility** — so the losing copy is named in a warning
+    here rather than dropped quietly.
+
+    Precedence, when a record carries both: ``config`` wins. It is the canonical home — the
+    whole ``task`` namespace comes from it, and the design that introduced ``extra_prompt``
+    (PR #964) renders ``task`` *as* the config — so the fallback exists for the file that
+    was written against the old comment, not to make the top level a second equal spelling.
+    ``sandbox`` resolves the other way round (``_resolve_sandbox`` prefers the record-level
+    value); the asymmetry is deliberate and stated in both docstrings, because the two
+    fields have different canonical homes.
+
+    Blank counts as absent on both sides: ``{% if task.extra_prompt %}`` renders nothing for
+    ``""``, so a blank copy is not a value anyone can lose by falling back.
+
+    The rule this leaves is one-sided and deliberate: **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). A malformed
+    ``config.extra_prompt`` beside a usable top-level one is the one case left unwarned, since
+    the value the file meant does render.
+    """
+    task = dict(config)
+    top = record.get("extra_prompt")
+
+    def usable(value: object) -> bool:
+        return isinstance(value, str) and bool(value.strip())
+
+    if usable(top):
+        if usable(task.get("extra_prompt")):
+            logger.warning(
+                "task %r carries extra_prompt both under config: and at its top level — "
+                "config: wins and the top-level copy (%d chars) is ignored; keep one",
+                record.get("name"), len(top),
+            )
+        else:
+            task["extra_prompt"] = top
+    elif top is not None:
+        # Present at the top level, and not a non-blank string: it renders nothing whether or
+        # not `config` supplies one. Warned here rather than skipped, because "nothing renders
+        # it" is the same silent loss this function exists to end — a `{% if %}` over a value
+        # the templates cannot interpolate is not a value anyone can see going missing.
+        logger.warning(
+            "task %r has a top-level extra_prompt of type %s — the templates interpolate a "
+            "non-blank string, so this one renders nothing; move it under config: as text",
+            record.get("name"), type(top).__name__,
+        )
+    return task
+
+
 def validate_task_record(record: dict) -> str | None:
     """Return why a record cannot be run, or None when it is usable.
 
@@ -2897,7 +2971,7 @@ def _build_evolution_prompt(self) -> str:
             "session_id": self._session_id,
             "timestamp": datetime.now().strftime("%Y%m%d-%H%M%S"),
             "current_time_human": datetime.now().strftime("%Y-%m-%d %H:%M"),
-            "task": self._config,
+            "task": _template_task_context(self._config, self._task),
             "project": _load_project_config(self._project_name, str(self._source_dir)),
         }
 

tests/test_prompt_templates.py

@@ -1763,3 +1763,161 @@ def test_the_prompt_path_scan_answers_both_ways() -> None:
     # The `.github/` exclusion is a decision rather than an oversight — the open-source flow reads the
     # *target* project's conventions. Pinned here so widening the scope is a visible change.
     assert _repo_paths_named_in("cat .github/pull_request_template.md 2>/dev/null") == []
+
+
+# ── Where `extra_prompt` may sit in a record ────────────────────────────────────
+#
+# The module docstring listed `extra_prompt` beside `sandbox` as a record-level optional
+# field, while the render site passed only `self._config` as `task`. A file written against
+# that comment therefore rendered `{% if task.extra_prompt %}` **false**, and the section —
+# heading and body together — disappeared with no error, no warning and no log line, because
+# the daemon renders under `jinja2.Undefined`.
+#
+# Measured on master `60541e39` (2026-10-11, `cyc20261011-193407`) through the real builder:
+# a record carrying `extra_prompt` at its top level rendered **without** it; the same value
+# under `config` rendered with it. The host reported the cost (rant 2026-10-11T11:23:55,
+# issue #2110): the `promote` task's 4601-char and the `competition` task's 3042-char
+# `extra_prompt` sat at the top level without ever entering a prompt.
+#
+# Both directions are pinned below, and the unset control matters as much as the set one:
+# a test that only asserts the marker is *present* would pass an empty render.
+
+EXTRA_TOP = "TOP-LEVEL-EXTRA-PROMPT-MARKER"
+EXTRA_CFG = "CONFIG-EXTRA-PROMPT-MARKER"
+
+
+def _handler_for_record(tmp_path, monkeypatch, *, top=None, under_config=None, template=None):
+    """A real handler whose record carries `extra_prompt` in the named position(s).
+
+    Built through `_apply_record`, which is the seam the daemon itself calls once per cycle
+    with the record read at that moment — so this is the production path, not a copy of it.
+    `make_handler` cannot express this case: it puts everything it is given under `config`.
+    """
+    monkeypatch.setattr(mod, "config_dir", lambda: tmp_path)
+    _write_projects_yml(tmp_path, "demoproj", {"name": "demoproj"})
+    handler = TaskHandler(name="demo-task", identity=InstanceIdentity())
+    config: dict = {"project": "demoproj"}
+    if under_config is not None:
+        config["extra_prompt"] = under_config
+    record: dict = {
+        "name": "demo-task",
+        "type": "evolution",
+        "interval": 300,
+        "enabled": True,
+        "config": config,
+    }
+    if top is not None:
+        record["extra_prompt"] = top
+    handler._apply_record(record)
+    handler._derived["template_path"] = PROMPTS_DIR / (template or "evolution_prompt.md")
+    return handler
+
+
+def test_a_record_level_extra_prompt_reaches_the_prompt(tmp_path, monkeypatch) -> None:
+    """The position the old comment advertised must work, in every template that reads it."""
+    for task_type, filename in _builtin_templates():
+        rendered = _handler_for_record(
+            tmp_path, monkeypatch, top=EXTRA_TOP, template=filename
+        )._build_evolution_prompt()
+        assert EXTRA_TOP in rendered, (
+            f"{task_type}/{filename}: a top-level `extra_prompt` did not reach the prompt — "
+            f"the file's instructions would be dropped in silence"
+        )
+
+
+def test_the_control_extra_prompt_under_config_still_reaches_the_prompt(
+    tmp_path, monkeypatch
+) -> None:
+    """The canonical home keeps working — the fallback must not displace it."""
+    rendered = _handler_for_record(
+        tmp_path, monkeypatch, under_config=EXTRA_CFG
+    )._build_evolution_prompt()
+    assert EXTRA_CFG in rendered, "`config.extra_prompt` stopped reaching the prompt"
+
+
+def test_neither_position_renders_the_section(tmp_path, monkeypatch) -> None:
+    """The unset control: without either position the section is absent, so the two tests
+    above are reading the marker rather than a template that always prints something."""
+    rendered = _handler_for_record(tmp_path, monkeypatch)._build_evolution_prompt()
+    assert EXTRA_TOP not in rendered and EXTRA_CFG not in rendered, (
+        "the extra_prompt section rendered with no extra_prompt set anywhere"
+    )
+    assert "Task-specific Instructions" not in rendered, (
+        "the section's heading rendered while its guard was false"
+    )
+
+
+def test_both_positions_warn_and_config_wins(tmp_path, monkeypatch, caplog) -> None:
+    """Ambiguity is audible and resolved the documented way.
+
+    The host's first ask was not leniency but feedback: the expensive part of this defect was
+    that a copy in the unhonoured position produced *nothing at all* to read. With the
+    fallback in place the only silent loss left is a record that carries both, so that one
+    names the task and the size of the copy it is ignoring.
+    """
+    with caplog.at_level("WARNING", logger=mod.__name__):
+        rendered = _handler_for_record(
+            tmp_path, monkeypatch, top=EXTRA_TOP, under_config=EXTRA_CFG
+        )._build_evolution_prompt()
+
+    assert EXTRA_CFG in rendered and EXTRA_TOP not in rendered, (
+        "config must win when a record carries both — it is the canonical home of the "
+        "`task` namespace"
+    )
+    warnings = [r.getMessage() for r in caplog.records if r.levelname == "WARNING"]
+    naming = [w for w in warnings if "extra_prompt" in w and "demo-task" in w]
+    assert naming, (
+        f"a record carrying extra_prompt twice was resolved without a warning: {warnings}"
+    )
+    assert str(len(EXTRA_TOP)) in naming[0], (
+        f"the warning must name the copy it is ignoring by size, so the host can find it: "
+        f"{naming[0]!r}"
+    )
+
+
+def test_a_blank_copy_is_not_a_value(tmp_path, monkeypatch) -> None:
+    """`{% if %}` renders nothing for a blank string, so blank falls back rather than wins."""
+    rendered = _handler_for_record(
+        tmp_path, monkeypatch, top=EXTRA_TOP, under_config="   \n"
+    )._build_evolution_prompt()
+    assert EXTRA_TOP in rendered, (
+        "a blank `config.extra_prompt` shadowed a usable top-level one — the section is lost "
+        "to whitespace"
+    )
+
+
+def test_a_top_level_copy_that_cannot_render_is_named(tmp_path, monkeypatch, caplog) -> None:
+    """The residual silence: a non-string top-level `extra_prompt` renders nothing either way.
+
+    A YAML author who writes a list, a mapping or a bare number has made the same mistake as
+    the one this file's block above describes — the section vanishes without a line of
+    feedback — so the rule the fallback leaves is that the top-level copy is never dropped
+    quietly. Both sub-cases are read, because they fail for different reasons: nothing else
+    supplies a value, and `config` does.
+    """
+    for label, under_config in (("alone", None), ("beside a usable config copy", EXTRA_CFG)):
+        caplog.clear()
+        with caplog.at_level("WARNING", logger=mod.__name__):
+            rendered = _handler_for_record(
+                tmp_path, monkeypatch, top=["do this", "then that"], under_config=under_config
+            )._build_evolution_prompt()
+
+        assert EXTRA_TOP not in rendered, (
+            f"{label}: a list rendered as the extra_prompt section"
+        )
+        if under_config is None:
+            assert EXTRA_CFG not in rendered, label
+        else:
+            assert EXTRA_CFG in rendered, (
+                f"{label}: the usable config copy stopped rendering, so this test is not "
+                f"reading the config/top-level interaction it claims to"
+            )
+        warnings = [r.getMessage() for r in caplog.records if r.levelname == "WARNING"]
+        naming = [w for w in warnings if "extra_prompt" in w and "demo-task" in w]
+        assert naming, (
+            f"a non-string top-level extra_prompt ({label}) was dropped without a warning: "
+            f"{warnings}"
+        )
+        assert "list" in naming[0], (
+            f"the warning must name the type it cannot interpolate: {naming[0]!r}"
+        )