Skip to content

feat(sub_agent): add mission_log_in format option for prompt caching#1022

Open
andreasronge wants to merge 2 commits into
mainfrom
claude/785-mission-log-in-user-message
Open

feat(sub_agent): add mission_log_in format option for prompt caching#1022
andreasronge wants to merge 2 commits into
mainfrom
claude/785-mission-log-in-user-message

Conversation

@andreasronge
Copy link
Copy Markdown
Owner

@andreasronge andreasronge commented May 25, 2026

Summary

  • Add format_options: [mission_log_in: :user_message] to control where the mission log is injected
  • When :user_message, the system prompt stays static across turns, enabling LLM prompt caching
  • Default :system_prompt preserves existing behavior — no breaking change

Closes #785

Changes

lib/ptc_runner/sub_agent/definition.ex

  • Add :mission_log_in to format_options typespec (:system_prompt | :user_message)

lib/ptc_runner/sub_agent/loop.ex

  • build_system_prompt/6: Skip mission log when mission_log_in: :user_message
  • maybe_inject_mission_log_in_messages/3: Inject mission log into the first user message each turn (after compaction, so state.messages stays clean and compaction can't discard it)

test/ptc_runner/sub_agent/journal_test.exs

  • Mission log appears in user message, not system prompt
  • System prompt is identical across turns (proves cacheability)
  • Mission log updates after task completion on turn 2
  • Empty journal produces no injection in either mode
  • Default :system_prompt mode preserves existing behavior

Test plan

  • All 5135 existing tests pass
  • 5 new tests covering :user_message mode
  • Existing journal/mission log tests pass unchanged
  • mix precommit passes (format + compile + credo + tests)

🤖 Generated with Claude Code

Fix Automation State

Fix attempts: 2/3

Allow configuring where the mission log is injected via
`format_options: [mission_log_in: :user_message]`. This keeps the
system prompt static across turns, enabling LLM prompt caching for
long-running multi-turn agents with journaling enabled.

Default remains `:system_prompt` (no breaking change). The mission log
is injected fresh into the first user message each turn, so it
reflects the current journal state without polluting state.messages.

Closes #785

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 25, 2026

PR Review: feat(sub_agent): add mission_log_in format option for prompt caching

Summary

Clean, well-scoped feature that adds format_options: [mission_log_in: :user_message] to control where the mission log is injected, enabling LLM prompt caching by keeping the system prompt static across turns. Follows the existing context_in_system pattern closely. One documentation gap to address.

What's Good

  • Follows the established context_in_system precedent exactly — same pattern for reading format_options, same conditional behavior in build_system_prompt, same call site placement
  • Smart injection point: maybe_inject_mission_log_in_messages is called after compaction (build_llm_messages) so the injected content can't be discarded by compaction, and state.messages stays clean
  • The with clause in maybe_inject_mission_log_in_messages cleanly handles all three bail-out conditions (journaling disabled, wrong mode, empty journal)
  • Thorough test coverage: cacheability proof (system prompt identical across turns), mission log updates on turn 2, empty journal in both modes, and default behavior preservation
  • inject_into_first_user_message correctly uses Enum.split_while to find the first user message, handling the edge case where no user message exists (returns messages unchanged)
  • Typespec updated in definition.ex — clean API surface

Issues (Must Fix)

  1. Missing documentationdocs/guides/subagent-advanced.md:124-131
    • Problem: The format_options table lists all options (feedback_limit, feedback_max_chars, history_max_bytes, result_limit, result_max_chars, max_print_length) but does not include the new mission_log_in option. The @doc in definition.ex is updated, but the user-facing guide is not.
    • Impact: Users won't discover this option from the guide. The typespec and @doc are only visible in HexDocs API docs, not in the conceptual guides where users learn about format_options.
    • Fix: Add a row to the table: | mission_log_in | :system_prompt | Where to inject the mission log: :system_prompt or :user_message (enables prompt caching) |. Also add a brief note or example under the table explaining the prompt caching benefit.
    • Complexity: Mechanical — one table row and a short paragraph.

Suggestions (Optional)

  1. File sizelib/ptc_runner/sub_agent/loop.ex is 1518 lines (exceeds 800-line threshold). This PR only adds 26 lines, so it's not the cause, but worth noting for future consideration. The prompt-building helpers (build_system_prompt, maybe_inject_mission_log_in_messages, inject_into_first_user_message, build_first_user_message) form a natural extraction boundary if the file is ever split.

Security

No concerns. The change only affects where existing content is placed in the LLM prompt (system prompt vs. user message). No new user input paths, no new external calls.

Documentation

  • definition.ex @doc and typespec: updated ✓
  • docs/guides/subagent-advanced.md format_options table: missing (see Issues above)
  • docs/guides/subagent-observability.md: already handles this correctly — line 95 states sections are parsed from "both system prompt and user messages", so Debug.print_trace(step, system: :mission_log) will find the mission log regardless of injection mode ✓

Verdict

Request Changes — The implementation is solid and well-tested, but the guide documentation gap should be closed before merge. It's a mechanical fix (one table row + short note).

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 25, 2026

@claude please fix:

  1. Missing mission_log_in in format_options table: In docs/guides/subagent-advanced.md, add a row to the format_options table (after the existing rows, before the closing row) for this option:
    | mission_log_in|:system_prompt| Where to inject the mission log::system_promptor:user_message(use:user_message to keep the system prompt static for prompt caching) |
    Also add a brief note or short example after the table explaining the prompt caching use case, for example: 'To enable prompt caching with providers that cache the system prompt, set mission_log_in: :user_message — this keeps the system prompt static across turns while the mission log updates in the first user message.'

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 25, 2026

Auto-Triage Summary

Issue Decision Action
Missing mission_log_in in subagent-advanced.md format_options table FIX_NOW In fix request above
loop.ex file size (1518 lines, exceeds 800-line threshold) IGNORE Optional suggestion not caused by this PR; noted for future refactor

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@andreasronge
Copy link
Copy Markdown
Owner Author

Fix Summary

Issue Fixed: Added mission_log_in to the format_options table in docs/guides/subagent-advanced.md

Added a new row to the format_options table documenting the mission_log_in option (default :system_prompt, supports :user_message), and added a brief note after the code example explaining the prompt caching use case: setting mission_log_in: :user_message keeps the system prompt static across turns while the mission log updates in the first user message.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 25, 2026

PR Review: feat(sub_agent): add mission_log_in format option for prompt caching

Summary

Well-executed feature that adds format_options: [mission_log_in: :user_message] to keep the system prompt static across turns for LLM prompt caching. Follows the existing context_in_system pattern, clean implementation, thorough tests, and documentation is complete (including the fix from the prior review round).

What's Good

  • Follows the established context_in_system precedent: same Keyword.get pattern, same conditional in build_system_prompt/6, same call-site placement
  • Smart injection point: maybe_inject_mission_log_in_messages/3 is called after build_llm_messages (compaction), so the injected content can't be discarded by compaction, and state.messages stays clean — the comment on line 1189-1191 correctly documents this non-obvious invariant
  • The with clause in maybe_inject_mission_log_in_messages/3 cleanly handles all three bail-out conditions (journaling disabled, wrong mode, empty journal) in a single expression
  • inject_into_first_user_message/2 correctly handles the edge case where no user message exists (returns messages unchanged)
  • Thorough test coverage: cacheability proof (system prompt identical across turns), mission log updates on turn 2 after task completion, empty journal in both modes, and default behavior preservation
  • Typespec, @doc, guide table row, and guide prose all updated — more thorough documentation than the existing context_in_system option (which is actually missing from both the typespec and the guide)

Issues (Must Fix)

None.

Suggestions (Optional)

  1. Silent mission log drop on invalid valueloop.ex:1173-1175, 1193-1196

    • Observation: If someone passes an invalid value like mission_log_in: :foo, the mission log is silently dropped: build_system_prompt skips it (:foo != :system_prompt) and maybe_inject_mission_log_in_messages skips it (:foo != :user_message). This is the same behavior as the existing context_in_system pattern, so it's consistent — but for a two-value enum, a typo like :user_messages (plural) could be confusing to debug.
    • Suggestion: Out of scope for this PR, but a future enhancement could validate format_options keys/values in the validator (validator.ex:331-337), covering both mission_log_in and the existing context_in_system.
  2. File sizeloop.ex is 1518 lines (exceeds 800-line threshold). This PR only adds 26 lines so it's not the cause. The prompt-building helpers (build_system_prompt, maybe_inject_mission_log_in_messages, inject_into_first_user_message, build_first_user_message) form a natural extraction boundary if the file is ever split.

Security

No concerns. The change only affects where existing content (the rendered mission log) is placed in the LLM prompt. No new user input paths, no new external calls.

Documentation

  • definition.ex @doc and typespec: updated ✓
  • docs/guides/subagent-advanced.md format_options table: updated ✓ (fixed in prior review round)
  • docs/guides/subagent-advanced.md prose note on prompt caching: added ✓

Verdict

Approve — Clean implementation that follows existing patterns, with thorough tests and complete documentation. The prior review's documentation gap has been addressed. Ready to merge.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 25, 2026

Auto-Triage Summary

Issue Decision Action
Silent mission log drop on invalid format_options value (e.g. typo :user_messages) DEFER_ISSUE Created #1023
loop.ex file size (1518 lines, 800-line threshold) IGNORE Optional note, not caused by this PR; no action needed

No FIX_NOW items — added ready-to-merge label.

@andreasronge andreasronge enabled auto-merge (squash) May 25, 2026 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge PR is ready to be merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider moving Mission Log out of system prompt for prompt caching

1 participant