Skip to content

refactor(pedagogy): split prompt_builder.rs into responsibility submodules#321

Merged
hherb merged 3 commits into
mainfrom
refactor/split-prompt-builder
Jul 2, 2026
Merged

refactor(pedagogy): split prompt_builder.rs into responsibility submodules#321
hherb merged 3 commits into
mainfrom
refactor/split-prompt-builder

Conversation

@hherb

@hherb hherb commented Jul 2, 2026

Copy link
Copy Markdown
Owner

Summary

Splits the 708-line primer-pedagogy/src/prompt_builder.rs into a directory module split by responsibility — behaviour-preserving, all files < 500 lines. Tests already lived in the prompt_builder/tests.rs sibling (untouched), so the 708 lines were pure production code.

file lines holds
prompt_builder/mod.rs 74 module doc + english_pack() (private, shared) + flat pub use façade
prompt_builder/system_prompt.rs 297 build_system_prompt_* family + assemble_system_prompt core + truncate_passages + days_since
prompt_builder/prompt.rs 151 build_messages + build_prompt_* family
prompt_builder/intent.rs 252 decide_intent* (the Socratic brain) + extract_active_concepts + opener/assertion classifiers

Visibility notes

  • english_pack() stays in mod.rs (all three submodules reach it via super::english_pack(); a test child of mod.rs sees it too).
  • The build_prompt_* functions call the sibling build_system_prompt_* via the super:: re-exports.
  • The (untouched) tests.rs reaches english_pack, the private is_factual_question / is_factual_question_with_pack intent helpers, and the Passage type by their bare names via use super::*. mod.rs carries three #[cfg(test)]-gated use re-imports — matching what the pre-split flat file leaked into module scope — so the test file needs zero edits.

Verification

  • Pub-surface diff empty (15 symbols, pub (struct|enum|fn|const|async fn|trait|type), old flat file vs new submodules).
  • cargo test -p primer-pedagogy216 passed / 0 failed (baseline match).
  • cargo clippy --workspace --all-targets -- -D warnings clean.
  • cargo fmt --all -- --check clean.
  • cargo test --workspace51 × test result: ok, 0 failed.

Internal refactor, no runtime behaviour change.

🤖 Generated with Claude Code

…dules

The 708-line prompt_builder.rs (production code only — tests already live
in the prompt_builder/tests.rs sibling) becomes a directory module, all
files < 500 lines and behaviour-preserving:

- mod.rs (74) — module doc + the process-wide cached english_pack()
  (private, shared by all submodules via super::english_pack()) + a flat
  `pub use` re-export facade (15-symbol pub surface unchanged).
- system_prompt.rs (297) — the build_system_prompt_* family + the shared
  assemble_system_prompt core + truncate_passages + days_since.
- prompt.rs (151) — build_messages + the build_prompt_* family (which
  call the sibling build_system_prompt_* via the super:: re-exports).
- intent.rs (252) — decide_intent* (the Socratic brain) +
  extract_active_concepts + the opener/assertion classifiers.

The pre-existing tests.rs is untouched: it reaches english_pack (kept in
mod.rs) plus the private is_factual_question / is_factual_question_with_pack
intent helpers and the Passage type via `use super::*`. mod.rs carries
three #[cfg(test)]-gated `use` re-imports (matching what the pre-split
flat file leaked into module scope) so the test file needs no edit.

Verification: pub-surface diff empty (15 symbols, incl. `type` in the
regex); primer-pedagogy 216/216 (baseline match); workspace clippy
-D warnings clean; fmt --check clean; full workspace test 51 ok / 0 failed.

CLAUDE.md: prompt_builder bullet rewritten for the directory-module split.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 2, 2026

Copy link
Copy Markdown

Deploying primer with  Cloudflare Pages  Cloudflare Pages

Latest commit: 585dc16
Status: ✅  Deploy successful!
Preview URL: https://c9fbda27.primer-10b.pages.dev
Branch Preview URL: https://refactor-split-prompt-builde.primer-10b.pages.dev

View logs

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@hherb

hherb commented Jul 2, 2026

Copy link
Copy Markdown
Owner Author

Code review

Found 1 issue:

  1. The CLAUDE.md prompt_builder bullet self-cites the wrong PR number: it says "(behaviour-preserving, PR refactor(core): split consts.rs into per-area submodules #322)" but this is PR refactor(pedagogy): split prompt_builder.rs into responsibility submodules #321. Every other reference in this PR's diff (NEXT_SESSION.md, the handoff doc) correctly says refactor(pedagogy): split prompt_builder.rs into responsibility submodules #321, and the sibling split PRs refactor(knowledge): split lib.rs into responsibility submodules #304/refactor(engine): split wiring.rs into responsibility submodules #305 self-cited their own number correctly in this same "(behaviour-preserving, PR #NNN)" pattern. A future bisector following the citation would land on the wrong PR.

primer/CLAUDE.md

Lines 81 to 83 in 7c1c16a

- **`primer-pedagogy`** — the Primer's "soul". Two modules:
- `prompt_builder` builds the system prompt (encoding the Socratic method as instructions, varying by age, engagement, and intent), assembles messages from session turns, and contains `decide_intent()` — the heuristic that picks the next `PedagogicalIntent`. `build_prompt` takes a `summary` (rolling pre-window summary) and a slice of `retrieved_older` turns; both are injected as system-prompt sections so the chat-message timeline (the last `context_window_turns` turns) stays linear. **`prompt_builder` is a directory module split by responsibility** (behaviour-preserving, PR #322): `mod.rs` owns the module doc, the process-wide cached `english_pack()` (private, shared by all three submodules via `super::english_pack()`), and a flat `pub use` re-export façade so every `prompt_builder::<name>` path is unchanged (15-symbol pub surface); `system_prompt.rs` the `build_system_prompt_*` family + the shared `assemble_system_prompt` core (base/intent/engagement/summary/retrieved/vocab/knowledge assembly, budgeted + unbudgeted) + `truncate_passages` + `days_since`; `prompt.rs` `build_messages` + the `build_prompt_*` family (which call the sibling `build_system_prompt_*` via the `super::` re-exports); `intent.rs` `decide_intent*` (the Socratic brain) + `extract_active_concepts` + the opener/assertion classifiers (`is_factual_question_with_pack`, `is_confusion_with_pack`, `is_probeable_assertion_with_pack`, `matches_opener`). The pre-existing `prompt_builder/tests.rs` is untouched: it reaches `english_pack` (in `mod.rs`, accessible to the test child) plus the private `is_factual_question`/`is_factual_question_with_pack` intent helpers and the `Passage` type via `use super::*` — `mod.rs` carries three `#[cfg(test)]`-gated `use` re-imports (matching what the pre-split flat file leaked into module scope) so the test file needs no edit.
- `dialogue_manager` orchestrates a session: record child turn → decide intent → retrieve knowledge → retrieve long-term memory (summary + FTS-relevant older turns) → build prompt → call inference → record Primer turn → refresh summary if due → update learner model. New entry point `resume_session(loaded)` replaces `open_session()` for resumed flows: it swaps in the loaded `Session`, clears `ended_at`, refreshes the summary **only if the loaded one is stale** (covers fewer pre-window turns than the current range; a current summary is preserved verbatim — no point burning an LLM call on identical work), and persists. Two summary-refresh cadences: active-conversation `refresh_summary_if_due` uses a K-threshold (re-summarize when `context_window_turns` new pre-window turns have accumulated, default K=20) so per-turn dialogue doesn't trigger an LLM call every time the boundary advances; resume `refresh_summary_if_stale` uses a simple "is the summary out of date?" check (any uncovered pre-window content triggers a refresh). Summarization goes through `InferenceBackend::summarize` — stub returns a canned string; cloud/ollama default-impl through `generate`.

Otherwise this is a clean, behavior-preserving split: the 15-symbol public surface, english_pack() placement, super:: re-exports, and the #[cfg(test)] re-imports for the untouched tests.rs all check out, and no pedagogical/intent logic changed.

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

The CLAUDE.md prompt_builder bullet cited PR #322, but this work is
PR #321. Every other reference in the branch (NEXT_SESSION.md, the
handoff doc) already says #321, matching the correct self-citation
pattern of sibling split PRs #304/#305.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@hherb hherb merged commit faf71ca into main Jul 2, 2026
11 checks passed
@hherb hherb deleted the refactor/split-prompt-builder branch July 2, 2026 23:15
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