Skip to content

fix(consolidation): escape literal braces in caller-supplied mission text#1676

Open
cdbartholomew wants to merge 3 commits into
mainfrom
fix/consolidation-mission-brace-escape
Open

fix(consolidation): escape literal braces in caller-supplied mission text#1676
cdbartholomew wants to merge 3 commits into
mainfrom
fix/consolidation-mission-brace-escape

Conversation

@cdbartholomew
Copy link
Copy Markdown
Contributor

Summary

The batch consolidation prompt is assembled from caller-supplied text (observations_mission, observation_capacity_note) interleaved with literal {facts_text} / {observations_text} placeholders, then passed through str.format() at call time. Any { or } in the caller-supplied text was previously interpreted as a format placeholder and raised KeyError, taking down consolidation for that bank — for example, a mission containing a JSON example like {"key": value} would crash with KeyError: "'key'".

This change escapes lone braces before assembly so the rendered prompt contains the original characters verbatim.

The escape is idempotent: text that already contains {{ / }} pairs is left untouched, so callers that pre-escape (or future call sites that defensively double-escape) still produce the same rendered output.

Implementation

  • prompts.py adds _escape_braces() which doubles only lone { and } characters, leaving already-doubled pairs alone.
  • Both observations_mission and observation_capacity_note are routed through it before being f-string-interpolated into the assembled prompt.
  • Real format placeholders ({facts_text}, {observations_text}) live in module-level constants that already use {{ / }} for any literal JSON in the prompt, so they are unaffected.

Tests

17 new tests in tests/test_consolidation_prompt_brace_escape.py:

  • _escape_braces unit coverage (lone brace doubling, idempotency, no-op on plain prose, mixed lone + already-escaped input).
  • End-to-end coverage on build_batch_consolidation_prompt:
    • JSON-shaped mission text renders without crashing and appears verbatim.
    • Pre-escaped mission renders to single literal braces (no double-escape artefacts).
    • Multiple brace pairs in a single mission.
    • Capacity note containing braces.
    • The unchanged {facts_text} / {observations_text} substitution path still works.
    • Parametrised edge cases: {single}, }}weird{{, trailing {, leading }, empty string.

Test plan

  • uv run pytest tests/test_consolidation_prompt_brace_escape.py -v → 17 passed
  • uv run pytest tests/test_consolidation_failure_recovery.py tests/test_consolidation_prompt_brace_escape.py tests/test_format_facts_for_prompt.py → 30 passed (pre-existing collection errors in test_consolidation_round_limit.py reproduce on clean origin/main and are unrelated)
  • uv run ruff check clean
  • uv run ty check hindsight_api/engine/consolidation/prompts.py clean
  • CI green on this PR before merge

…text

The batch consolidation prompt is assembled from caller-supplied text
(observations_mission, observation_capacity_note) interleaved with
literal {facts_text} / {observations_text} placeholders, then passed
through str.format() at call time. Any { or } in the caller-supplied
text was previously interpreted as a format placeholder and raised
KeyError, taking down consolidation for that bank.

Escape lone braces before assembly so the rendered prompt contains the
original characters verbatim. The escape is idempotent — text that
already contains {{ / }} pairs is left untouched, so callers that
pre-escape for any reason still produce the same rendered output.

Includes regression tests covering JSON-shaped mission text,
pre-escaped input, multiple brace pairs, and the unchanged
substitution path for the real placeholders.
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