diff --git a/emrg/server/scheduler.py b/emrg/server/scheduler.py index dd1ebdc6..33f1526a 100644 --- a/emrg/server/scheduler.py +++ b/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)), } diff --git a/tests/test_prompt_templates.py b/tests/test_prompt_templates.py index dca19d11..9d45fcd6 100644 --- a/tests/test_prompt_templates.py +++ b/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}" + )