From f16d3d18d64493af9e8081df2503934c7c8772d6 Mon Sep 17 00:00:00 2001 From: Chi Date: Sun, 6 Sep 2026 17:33:43 +0700 Subject: [PATCH 1/6] planning --- docs/plans/inference-layer-audit.md | 347 ++++++++++++++++++++++++++++ 1 file changed, 347 insertions(+) create mode 100644 docs/plans/inference-layer-audit.md diff --git a/docs/plans/inference-layer-audit.md b/docs/plans/inference-layer-audit.md new file mode 100644 index 00000000..09c36353 --- /dev/null +++ b/docs/plans/inference-layer-audit.md @@ -0,0 +1,347 @@ +# `backend/inference/` — layer audit and relocation plan + +Status: proposal, nothing implemented. +Audit date: 2026-09-06. Base commit: `91f7d64`. +Scope: `backend/inference/` (7,683 LOC across 25 modules). + +This document is a hand-off for independent review. It states a principle, applies +it, and lists what was checked and cleared so a reviewer does not repeat the work. +Every claim carries a `file:line` anchor. Section 8 records claims made during the +audit that were **withdrawn** on verification — read it to calibrate the rest. + +--- + +## 1. Why this audit exists + +The prose rewriter used to live entirely in `inference/`. It was moved out: the +shared llama.cpp server layer stayed (`inference/local_models/llama_server/`) and +the feature became `features/prose_rewriter/`. That move was correct, and it was +made for a reason that generalises — a feature does not belong in `inference/` +even if `inference/` is where it happened to be written. + +The question this audit answers: **what else is in there for the same reason?** + +The answer is three more tenants of the same kind, one weaker candidate, and one +root cause in the architecture documentation that licensed all of them. + +--- + +## 2. The organizing principle + +The existing rule in `AGENTS.md` is a *dependency* rule — it constrains which +layers may import which. `inference/` passes it cleanly (verified, §7). The rule +is silent on the question that actually matters here, which is placement, not +direction. + +Proposed test, by **axis of change**: + +> A module belongs in `inference/` if its reason to change is **a model, a +> provider, or the cache contract**. It does not belong if its reason to change +> is **a product decision**. + +Applied: + +| Reason to change | Modules | Verdict | +|---|---|---| +| A provider's API or quirks | `client.py`, `anthropic.py`, `endpoint_profiles.py`, `retry.py`, `errors.py`, `gemma_tool_format.py`, `text_completion.py` | stays | +| A local runtime | `local_ml.py` (runtime half), `local_models/**` | stays | +| The KV-cache contract | `cached_call.py`, `kv_tracker.py`, `build_prefix`, the `TOOLS` dict *ordering* | stays | +| What the Director is told | `prompt_builder.py` prose (~510 lines) | **moves** | +| Which lorebook entries activate | `lorebook.py` (selection half) | **moves** | +| What autocomplete drafts look like | `local_ml.build_prompt` | **moves** | +| Group-chat visibility policy | `group_context.py` | weak; defer | + +This test is worth reviewing on its own merits. If it is wrong, most of §4 is wrong. + +--- + +## 3. Root cause (the finding that matters most) + +`AGENTS.md:17` defines layer 5 as: + +> `inference/` and `analysis/` — model access, **prompt assembly**, and pure text analysis + +**"Prompt assembly" is the phrase that created the dump site.** It conflates two +unrelated things: + +- **prefix assembly** — `build_prefix`, attachment embedding, the tools blob. + Infrastructure. KV-critical. Correctly layer 5. +- **prompt authoring** — the actual words sent to the Director, Editor, and + Writer. Product content, owned by whoever owns the behaviour. + +Under the broad reading, every prompt string in the app is licensed to live in +`inference/`. That is how ~510 lines of pass prose ended up there. + +Newer code already ignores the broad reading, deliberately and with comments +explaining why. `features/cards/public_profile.py:46`: + +> `# Deliberately not registered in inference.tool_registry.TOOLS: that module` +> `# asserts PRE_WRITER_TOOLS | POST_WRITER_TOOLS == BUILTIN_TOOL_NAMES at import,` +> `# so registering here would force a turn-phase partition onto a tool that has` +> `# nothing to do with a turn.` + +Same again in `features/cards/sheet_update.py:30`. `workflows/image_gen/prompts.py` +and `pipeline/passes/editor/length_guard.py` also keep their prompts local. + +So the codebase already has two conventions in force, and the newer one is the +better one. **The primary recommendation is to narrow the `AGENTS.md` wording** +("prompt and tool-schema assembly for the shared cached prefix"). The file moves +in §4 are downstream of that decision. If a reviewer disagrees with narrowing the +rule, items 2–4 below should be rejected as a set, not individually. + +--- + +## 4. Findings + +### 4.1 `inference/lorebook.py` (413 lines) — same shape as the prose rewriter + +`features/lorebook/` already exists, with real local modules (`changesets.py`, +`proposals.py`). But **15 of the ~28 names in its public facade are re-exported +straight out of `inference/lorebook.py`** (`features/lorebook/__init__.py:5-21`), +and `features/lorebook/proposals.py:10` imports four more from the same place. + +Nothing in the module touches a client, a model, or a transport. It is keyword +scanning, entry-layer projection, and Markdown rendering. Even +`database/queries/worlds.py:273` documents the projection as +`inference.lorebook.select_effective_entries` — layer 6 pointing at layer 5 to +explain a lorebook concept. + +**The one real constraint.** `workflows/toolkit.py:178` calls +`compute_constant_lorebook_block` to feed `build_prefix` inside +`build_offturn_prefix`. `workflows/` (4) sits *below* `features/` (3), so it +cannot import the feature slice. + +This is structurally identical to the `workflows → passes.editor.audit` edge that +was previously resolved by extracting the `analysis/` layer — and that precedent +is live (`workflows/toolkit.py:7` imports `..analysis`). + +**Proposed cut — the same cut already made for the prose rewriter:** block +*rendering* stays in `inference/` as the shared layer; selection, gating, the +Director catalog, and Dynamic Worlds move to `features/lorebook/`. + +Note `build_offturn_prefix` exists to reproduce the turn prefix byte-for-byte for +off-turn workflow calls. Inverting the dependency (passing the block in) was +considered and rejected: it changes a KV-sensitive path and adds a parameter to a +workflow-author-facing API. + +### 4.2 `prompt_builder.py` + `tool_registry.py` — pass content in the model layer + +`prompt_builder.py` is 710 lines: roughly the first 198 are genuine inference +(`build_prefix`, `format_message_with_attachments`), and the remainder is +Director/Editor/Writer prompt prose — `DIRECTOR_PREAMBLE`, +`EDITOR_PATCH_INSTRUCTIONS`, `EDITOR_REWRITE_INSTRUCTIONS`, `WORLD_CHANGE_PREAMBLE`, +`build_editor_prompt`, `build_feedback_prompt`, `build_lorebook_select_prompt`, +and the style-injection block. + +Consumers (verified, all of them): + +| Symbol | Consumers | +|---|---| +| `build_director_tool_prompt`, `build_director_scene_step_prompt`, `render_direction_notes_block` | `pipeline/passes/director/director.py` | +| `build_lorebook_select_prompt` | `pipeline/passes/director/lorebook_select.py` | +| `build_direction_note_prompt` | `pipeline/passes/director/direction_note.py` | +| `build_editor_prompt`, `EDITOR_RENUMBER_NOTICE` | `pipeline/passes/editor/editor.py` | +| `build_feedback_prompt` | `pipeline/passes/editor/feedback.py` | +| `build_world_change_prompt` | `pipeline/passes/world_change.py`, `pipeline/world_proposal.py` | +| `compute_style_injection_block`, `resolve_mood_fragment_randoms` | `pipeline/passes/director/director.py`, `api/routes/conversations.py` | + +Every consumer is in `pipeline/`, except two symbols also used by one API route. +The move target is unambiguous. + +**`tool_registry.py` is a different case and must be treated separately — see §5.** +Its ~328 lines of concrete schemas look like the same problem, but the `TOOLS` +dict ordering is a cache contract. Recommendation: **move the prose, keep the +`TOOLS` dict literal centralized.** + +### 4.3 `local_ml.py` — a runtime with one feature glued on + +The llama-cpp-python machinery (load/lock/complete/score/classify) is genuine +inference and stays. Two things ride along that should not: + +- **`build_prompt()` (`local_ml.py:368-401`)** — the autocomplete/typeahead prompt + builder: character name, summary, recent messages, draft. Feature prompt + assembly inside a runtime module. Sole production consumer: + `api/routes/messages.py:499`. +- **The 39-line `if __name__ == "__main__":` block (`local_ml.py:404-442`)** — + self-tests for `build_prompt`, `pov_from_logits`, and `pov_input`. + `tests/unit/test_autocomplete.py` and `tests/unit/test_local_ml.py` already + cover all three. Production code carrying duplicate test surface. + +`GO_EMOTIONS` stays — the tuple order must match the GGUF head's logit order +(`local_ml.py:68`), which is a model fact. Worth noting that +`features/cards/expressions.py:14` imports it with an apologetic +`# dep-free tuple; no llama import`, which hints the label vocabulary would rather +be a datum than a runtime import. Low priority; not proposed here. + +### 4.4 `group_context.py` (213 lines) — weak candidate, defer + +Group-chat cast rendering and per-mode character-field visibility. Same domain as +`features/cards/public_profile.py`, and `docs/architecture/group-chats.md` treats +it as a feature. But there is no `features/group_chat/` slice to move into, and +`prompt_builder.py` consumes it directly. + +Recommendation: **leave it.** Revisit only if 4.1 and 4.2 land and the seam +becomes obvious. Creating a feature slice to house one module is over-engineering. + +### 4.5 Leftovers + +- **`backend/inference/prose_rewriter/` still exists**, containing only stale + `__pycache__` from the move. Untracked and gitignored, so it never appears in + `git status` — but `ls inference/` still prints `prose_rewriter`, which is + exactly the confusion the move was meant to end. +- Orphaned bytecode for deleted modules: `inference/__pycache__/isolated.pyc`, + `local_completion.pyc`. +- **`roster_names` is exported from `inference/__init__.py:107` with zero + consumers** — three internal call sites in `group_context.py`, no external + import, no test import. A dead export, not dead code. + +### 4.6 Structural note: the facade invites the dump + +`inference/__init__.py` re-exports **60 names from 11 modules** into one flat +namespace. `from ..inference import (...)` therefore makes transport primitives +and product prose equally reachable at equal cost. There is no friction against +adding one more name, which is a contributing cause rather than a symptom. + +Not proposing a facade rewrite — that is churn without behaviour change. But if +4.1–4.3 land, the facade should shrink with them rather than keep re-exporting +relocated symbols for compatibility. + +--- + +## 5. Hard constraint: tool array order is cached-prefix bytes + +**This is the constraint most likely to be missed by a reviewer, and violating it +would be a serious regression.** + +`enabled_schemas()` (`tool_registry.py:430`) iterates `TOOLS` in **dict insertion +order**. Its output goes directly into `tools=tuple(...)` on `CachedBase` +(`pipeline/config.py:84`, `:95`), which `cached_call.py:100` documents as "the +cached bottom (prefix + tools + model)". + +`kv_tracker._serialize_tools` uses `json.dumps(..., sort_keys=True)`, which sorts +object *keys* — **not** array order. Array order is therefore load-bearing for +prefix identity. + +**Consequence:** scattering the seven built-in schemas into their pass modules +would make the tools-blob byte order depend on module import order, silently +invalidating the KV prefix for every existing conversation. KV cache is the +project's stated first priority. Do not do this. + +Two further guards live in the same file and constrain any change: +`tool_registry.py:391` asserts `BUILTIN_TOOL_NAMES` matches `TOOLS.keys()`, and +`tool_registry.py:413` asserts the pre/post-writer sets partition it. The second +is the one `features/cards/public_profile.py:46` cites as its reason to stay out +of the registry. + +**Acceptance test for any change in §4.2:** the tools tuple and prefix must be +byte-identical before and after. `kv_tracker` already computes this; assert on it +rather than eyeballing. + +--- + +## 6. Recommended sequence + +Ordered by risk, lowest first. Each step is independently shippable. + +| # | Action | Risk | Notes | +|---|---|---|---| +| 0 | Narrow the `AGENTS.md:17` wording (§3) | none | **Decide this first.** Steps 2–4 are downstream of it. | +| 1 | `rm -rf backend/inference/prose_rewriter/`; drop `roster_names` from the facade | none | Independent of everything else. | +| 2 | Move `lorebook.py` selection/gating/catalog/Dynamic-Worlds → `features/lorebook/`; leave block rendering | low | Collapses the pass-through facade. Watch the `workflows/toolkit.py:178` edge. | +| 3 | Move `local_ml.build_prompt` → autocomplete feature; delete the `__main__` block | low | Smallest real win. | +| 4 | Move ~510 lines of pass prose out of `prompt_builder.py` into `pipeline/passes/` owners | **medium** | Largest diff. Prefix-adjacent — §5 acceptance test required. | +| — | ~~Scatter `TOOLS` schemas~~ | — | **Rejected**, §5. | +| — | ~~`pov_*` → `analysis/`~~ | — | **Rejected**, §8. | +| — | `group_context.py` | — | Deferred, §4.4. | + +Steps 2 and 4 both touch `inference/__init__.py`; sequence them, do not parallelise. + +--- + +## 7. Checked and cleared — do not redo + +| Question | Finding | +|---|---| +| Layer violations out of `inference/`? | **None.** Only `..core` leaves the package; all other relative imports are intra-package. (First attempt used a regex blind to relative imports; re-run corrected, conclusion unchanged.) | +| Two local runtimes = duplication? | **No.** `local_ml` (in-process llama-cpp-python) and `llama_server/` (subprocess) are a deliberate split behind a `RuntimeKind` discriminator on `ModelSpec` (`local_models/catalog.py:49`, `:115`), sharing one catalog. | +| `endpoint_profiles.PROFILES` vs the "derive or degrade, never hand-keep a capability table" rule? | **Consistent.** The whole table is 2 endpoint keys (`api.deepseek.com`, `nano-gpt.com`) with 1 per-model override (`deepseek-reasoner`); unmatched → `None` → pass-through unchanged; entries encode *observed* rejections, not guessed capabilities. `anthropic.py:21,26` learns rejections at runtime, which is the derive pattern. | +| `inference/errors.redact` duplicating `core/utils.scrub_log`? | **No.** Unrelated: API-key removal vs. CWE-117 newline stripping. | +| `build_style_injection` dead? | **No.** Called internally at `prompt_builder.py:669`, directly tested in `tests/unit/test_interactive_fragments.py`. | +| `strictify_schema` misplaced with the schemas? | **No.** Consumed by `client.py` and `anthropic.py` — transport concern, correctly in layer 5. | + +--- + +## 8. Withdrawn claims (calibration) + +Claims made during this audit and retracted on verification. Listed so a reviewer +can weigh the surviving ones. + +1. **`pov_input` / `pov_from_logits` → `analysis/`.** Withdrawn. Asserted their + consumer was `workflows/image_gen/pov.py:70`; that line calls `aclassify_pov`, + the async *runtime* wrapper. The pure helpers have no production consumer + outside `local_ml` itself. Moving them would relocate two test files so that + `local_ml` could import its own helpers back — over-engineering. +2. **Scattering `TOOLS` schemas into pass modules.** Withdrawn on discovering §5. +3. **`build_style_injection` is dead code.** False — the consumer grep excluded + the defining file, hiding an internal call. +4. **"Every pass-prompt function has exactly one consumer."** Overstated; see the + table in §4.2 for the two exceptions. +5. **`tool_registry.py:392` is the partition assert.** Wrong line; it is `:413`. +6. **`features/lorebook/__init__.py` is a "pass-through shell."** Overstated — + 15 of ~28 names, roughly half. +7. **"`PROFILES` has 3 per-model overrides"** (in a draft of §7). Miscounted: the + regex conflated endpoint keys with model keys. Actual shape is 2 endpoints, + 1 model override. Corrected in place; the verdict was unaffected. + +--- + +## 9. Open questions for review + +1. **Is the §2 axis-of-change test the right principle?** It is the load-bearing + assumption. Most of §4 collapses if it is wrong. +2. **Is step 4 worth its risk?** ~510 lines moved, prefix-adjacent, for a purely + structural gain. A reasonable reviewer could accept steps 0–3 and reject 4. + What would change the answer is whether the pass prose is expected to churn: + if it is edited often, the move pays for itself; if it is stable, it may not. +3. **Should the shared lorebook rendering layer live in `inference/` or in + `analysis/`?** §4.1 proposes `inference/` by analogy with the llama_server + precedent. `analysis/` is arguably a better fit for pure rendering, and the + `workflows → analysis` edge already exists. +4. **`kv_tracker._prev_turn_entries` grows without bound.** Module-level dict + (`kv_tracker.py:13`), written at `:220` keyed by `conversation_id`, read at + `:97`, with no `pop`, `clear`, or size cap anywhere in the module — verified. + One retained entry per conversation for the process lifetime. This is a + diagnostics cache, so the leak is slow and bounded by conversation count, but + it is real. Out of scope for a placement audit; raised because it was found + during it. Placement itself is fine — it is a cache-contract concern, layer 5. +5. **Is there a fifth tenant this audit missed?** Coverage was driven by the §2 + test applied to all 25 modules, but the test itself may have blind spots — + particularly around `cached_call.py` and `kv_tracker.py`, which were accepted + as infrastructure without deep reading. + +--- + +## 10. How to re-verify + +```sh +# §7 layer check (relative-import aware — the naive regex misses these) +grep -rnE "^\s*from\s+\.+(api|pipeline|features|workflows|database)\b" backend/inference/ --include="*.py" +grep -rhoE "^\s*from\s+\.[.a-z_]*" backend/inference/ --include="*.py" | sort | uniq -c | sort -rn + +# §4.2 consumer table +for n in build_editor_prompt build_feedback_prompt build_world_change_prompt \ + compute_style_injection_block resolve_mood_fragment_randoms; do + printf "%-32s " "$n" + grep -rn "\b$n\b" --include="*.py" backend/ | grep -v __pycache__ \ + | grep -v "inference/prompt_builder.py\|inference/__init__.py" \ + | sed 's|backend/||;s|:.*||' | sort -u | tr '\n' ' '; echo +done + +# §5 cache contract +grep -n "tools" backend/inference/cached_call.py +sed -n '78,96p' backend/pipeline/config.py +grep -n "^assert" backend/inference/tool_registry.py +``` + +Note: consumer greps must **not** exclude the defining file — that error produced +withdrawn claim 3. From 80d524178e7ff66ed430a3d436b07365b7769d25 Mon Sep 17 00:00:00 2001 From: Chi Date: Sun, 6 Sep 2026 18:10:30 +0700 Subject: [PATCH 2/6] refine plan --- docs/plans/inference-layer-audit.md | 687 ++++++++++++++-------------- 1 file changed, 355 insertions(+), 332 deletions(-) diff --git a/docs/plans/inference-layer-audit.md b/docs/plans/inference-layer-audit.md index 09c36353..219d7445 100644 --- a/docs/plans/inference-layer-audit.md +++ b/docs/plans/inference-layer-audit.md @@ -1,347 +1,370 @@ -# `backend/inference/` — layer audit and relocation plan - -Status: proposal, nothing implemented. -Audit date: 2026-09-06. Base commit: `91f7d64`. -Scope: `backend/inference/` (7,683 LOC across 25 modules). - -This document is a hand-off for independent review. It states a principle, applies -it, and lists what was checked and cleared so a reviewer does not repeat the work. -Every claim carries a `file:line` anchor. Section 8 records claims made during the -audit that were **withdrawn** on verification — read it to calibrate the rest. - ---- - -## 1. Why this audit exists - -The prose rewriter used to live entirely in `inference/`. It was moved out: the -shared llama.cpp server layer stayed (`inference/local_models/llama_server/`) and -the feature became `features/prose_rewriter/`. That move was correct, and it was -made for a reason that generalises — a feature does not belong in `inference/` -even if `inference/` is where it happened to be written. - -The question this audit answers: **what else is in there for the same reason?** - -The answer is three more tenants of the same kind, one weaker candidate, and one -root cause in the architecture documentation that licensed all of them. - ---- - -## 2. The organizing principle - -The existing rule in `AGENTS.md` is a *dependency* rule — it constrains which -layers may import which. `inference/` passes it cleanly (verified, §7). The rule -is silent on the question that actually matters here, which is placement, not -direction. - -Proposed test, by **axis of change**: - -> A module belongs in `inference/` if its reason to change is **a model, a -> provider, or the cache contract**. It does not belong if its reason to change -> is **a product decision**. - -Applied: - -| Reason to change | Modules | Verdict | -|---|---|---| -| A provider's API or quirks | `client.py`, `anthropic.py`, `endpoint_profiles.py`, `retry.py`, `errors.py`, `gemma_tool_format.py`, `text_completion.py` | stays | -| A local runtime | `local_ml.py` (runtime half), `local_models/**` | stays | -| The KV-cache contract | `cached_call.py`, `kv_tracker.py`, `build_prefix`, the `TOOLS` dict *ordering* | stays | -| What the Director is told | `prompt_builder.py` prose (~510 lines) | **moves** | -| Which lorebook entries activate | `lorebook.py` (selection half) | **moves** | -| What autocomplete drafts look like | `local_ml.build_prompt` | **moves** | -| Group-chat visibility policy | `group_context.py` | weak; defer | - -This test is worth reviewing on its own merits. If it is wrong, most of §4 is wrong. - ---- - -## 3. Root cause (the finding that matters most) - -`AGENTS.md:17` defines layer 5 as: - -> `inference/` and `analysis/` — model access, **prompt assembly**, and pure text analysis - -**"Prompt assembly" is the phrase that created the dump site.** It conflates two -unrelated things: - -- **prefix assembly** — `build_prefix`, attachment embedding, the tools blob. - Infrastructure. KV-critical. Correctly layer 5. -- **prompt authoring** — the actual words sent to the Director, Editor, and - Writer. Product content, owned by whoever owns the behaviour. - -Under the broad reading, every prompt string in the app is licensed to live in -`inference/`. That is how ~510 lines of pass prose ended up there. - -Newer code already ignores the broad reading, deliberately and with comments -explaining why. `features/cards/public_profile.py:46`: - -> `# Deliberately not registered in inference.tool_registry.TOOLS: that module` -> `# asserts PRE_WRITER_TOOLS | POST_WRITER_TOOLS == BUILTIN_TOOL_NAMES at import,` -> `# so registering here would force a turn-phase partition onto a tool that has` -> `# nothing to do with a turn.` - -Same again in `features/cards/sheet_update.py:30`. `workflows/image_gen/prompts.py` -and `pipeline/passes/editor/length_guard.py` also keep their prompts local. - -So the codebase already has two conventions in force, and the newer one is the -better one. **The primary recommendation is to narrow the `AGENTS.md` wording** -("prompt and tool-schema assembly for the shared cached prefix"). The file moves -in §4 are downstream of that decision. If a reviewer disagrees with narrowing the -rule, items 2–4 below should be rejected as a set, not individually. - ---- - -## 4. Findings - -### 4.1 `inference/lorebook.py` (413 lines) — same shape as the prose rewriter - -`features/lorebook/` already exists, with real local modules (`changesets.py`, -`proposals.py`). But **15 of the ~28 names in its public facade are re-exported -straight out of `inference/lorebook.py`** (`features/lorebook/__init__.py:5-21`), -and `features/lorebook/proposals.py:10` imports four more from the same place. - -Nothing in the module touches a client, a model, or a transport. It is keyword -scanning, entry-layer projection, and Markdown rendering. Even -`database/queries/worlds.py:273` documents the projection as -`inference.lorebook.select_effective_entries` — layer 6 pointing at layer 5 to -explain a lorebook concept. - -**The one real constraint.** `workflows/toolkit.py:178` calls -`compute_constant_lorebook_block` to feed `build_prefix` inside -`build_offturn_prefix`. `workflows/` (4) sits *below* `features/` (3), so it -cannot import the feature slice. - -This is structurally identical to the `workflows → passes.editor.audit` edge that -was previously resolved by extracting the `analysis/` layer — and that precedent -is live (`workflows/toolkit.py:7` imports `..analysis`). - -**Proposed cut — the same cut already made for the prose rewriter:** block -*rendering* stays in `inference/` as the shared layer; selection, gating, the -Director catalog, and Dynamic Worlds move to `features/lorebook/`. - -Note `build_offturn_prefix` exists to reproduce the turn prefix byte-for-byte for -off-turn workflow calls. Inverting the dependency (passing the block in) was -considered and rejected: it changes a KV-sensitive path and adds a parameter to a -workflow-author-facing API. - -### 4.2 `prompt_builder.py` + `tool_registry.py` — pass content in the model layer +# Inference boundary refactor implementation plan + +Status: ready for implementation. This document update changes no production +code. + +## 1. Outcome + +Create a lower shared `backend/prompting/` package for deterministic, +provider-independent model-facing construction, then move product policy out of +`backend/inference/`. + +The resulting ownership is: + +- `inference/`: model execution, provider adaptation, retries and errors, + structured-output normalization, cache-call mechanics, cache diagnostics, and + local model runtimes; +- `prompting/`: shared prompt-context projections, message construction, tool + schemas, and deterministic ordered tool composition; +- `pipeline/passes/`: instruction prose and result contracts owned by a + pipeline pass; +- `features/`: feature enablement and feature-specific adapters; +- `analysis/`: text interpretation and measurement. + +A module belongs in `prompting/` only when it: + +1. deterministically transforms caller-supplied data; +2. constructs model-facing content or a canonical projection required by that + content; +3. is shared by multiple upper-layer consumers or by the stable workflow + toolkit; and +4. performs no I/O, model call, persistence, or feature/pass enablement decision. + +### Enforced dependency graph + +```text +api -> pipeline, features, workflows, prompting, inference, analysis, database, core +pipeline -> features, workflows, prompting, inference, analysis, database, core +features -> prompting, inference, analysis, database, core +workflows -> prompting, inference, analysis, database, core +prompting -> core +inference -> core +analysis -> database, core +database -> core +core -> (nothing) +``` -`prompt_builder.py` is 710 lines: roughly the first 198 are genuine inference -(`build_prefix`, `format_message_with_attachments`), and the remainder is -Director/Editor/Writer prompt prose — `DIRECTOR_PREAMBLE`, -`EDITOR_PATCH_INSTRUCTIONS`, `EDITOR_REWRITE_INSTRUCTIONS`, `WORLD_CHANGE_PREAMBLE`, -`build_editor_prompt`, `build_feedback_prompt`, `build_lorebook_select_prompt`, -and the style-injection block. +`features/` and `workflows/` are sibling composition inputs and do not import +each other. Feature slices remain isolated from peer feature slices. -Consumers (verified, all of them): +## 2. Planned source moves -| Symbol | Consumers | +| Current surface | Implemented destination | |---|---| -| `build_director_tool_prompt`, `build_director_scene_step_prompt`, `render_direction_notes_block` | `pipeline/passes/director/director.py` | -| `build_lorebook_select_prompt` | `pipeline/passes/director/lorebook_select.py` | -| `build_direction_note_prompt` | `pipeline/passes/director/direction_note.py` | -| `build_editor_prompt`, `EDITOR_RENUMBER_NOTICE` | `pipeline/passes/editor/editor.py` | -| `build_feedback_prompt` | `pipeline/passes/editor/feedback.py` | -| `build_world_change_prompt` | `pipeline/passes/world_change.py`, `pipeline/world_proposal.py` | -| `compute_style_injection_block`, `resolve_mood_fragment_randoms` | `pipeline/passes/director/director.py`, `api/routes/conversations.py` | - -Every consumer is in `pipeline/`, except two symbols also used by one API route. -The move target is unambiguous. - -**`tool_registry.py` is a different case and must be treated separately — see §5.** -Its ~328 lines of concrete schemas look like the same problem, but the `TOOLS` -dict ordering is a cache contract. Recommendation: **move the prose, keep the -`TOOLS` dict literal centralized.** - -### 4.3 `local_ml.py` — a runtime with one feature glued on - -The llama-cpp-python machinery (load/lock/complete/score/classify) is genuine -inference and stays. Two things ride along that should not: - -- **`build_prompt()` (`local_ml.py:368-401`)** — the autocomplete/typeahead prompt - builder: character name, summary, recent messages, draft. Feature prompt - assembly inside a runtime module. Sole production consumer: - `api/routes/messages.py:499`. -- **The 39-line `if __name__ == "__main__":` block (`local_ml.py:404-442`)** — - self-tests for `build_prompt`, `pov_from_logits`, and `pov_input`. - `tests/unit/test_autocomplete.py` and `tests/unit/test_local_ml.py` already - cover all three. Production code carrying duplicate test surface. - -`GO_EMOTIONS` stays — the tuple order must match the GGUF head's logit order -(`local_ml.py:68`), which is a model fact. Worth noting that -`features/cards/expressions.py:14` imports it with an apologetic -`# dep-free tuple; no llama import`, which hints the label vocabulary would rather -be a datum than a runtime import. Low priority; not proposed here. - -### 4.4 `group_context.py` (213 lines) — weak candidate, defer - -Group-chat cast rendering and per-mode character-field visibility. Same domain as -`features/cards/public_profile.py`, and `docs/architecture/group-chats.md` treats -it as a feature. But there is no `features/group_chat/` slice to move into, and -`prompt_builder.py` consumes it directly. - -Recommendation: **leave it.** Revisit only if 4.1 and 4.2 land and the seam -becomes obvious. Creating a feature slice to house one module is over-engineering. - -### 4.5 Leftovers - -- **`backend/inference/prose_rewriter/` still exists**, containing only stale - `__pycache__` from the move. Untracked and gitignored, so it never appears in - `git status` — but `ls inference/` still prints `prose_rewriter`, which is - exactly the confusion the move was meant to end. -- Orphaned bytecode for deleted modules: `inference/__pycache__/isolated.pyc`, - `local_completion.pyc`. -- **`roster_names` is exported from `inference/__init__.py:107` with zero - consumers** — three internal call sites in `group_context.py`, no external - import, no test import. A dead export, not dead code. - -### 4.6 Structural note: the facade invites the dump - -`inference/__init__.py` re-exports **60 names from 11 modules** into one flat -namespace. `from ..inference import (...)` therefore makes transport primitives -and product prose equally reachable at equal cost. There is no friction against -adding one more name, which is a contributing cause rather than a symptom. - -Not proposing a facade rewrite — that is churn without behaviour change. But if -4.1–4.3 land, the facade should shrink with them rather than keep re-exporting -relocated symbols for compatibility. - ---- - -## 5. Hard constraint: tool array order is cached-prefix bytes - -**This is the constraint most likely to be missed by a reviewer, and violating it -would be a serious regression.** - -`enabled_schemas()` (`tool_registry.py:430`) iterates `TOOLS` in **dict insertion -order**. Its output goes directly into `tools=tuple(...)` on `CachedBase` -(`pipeline/config.py:84`, `:95`), which `cached_call.py:100` documents as "the -cached bottom (prefix + tools + model)". - -`kv_tracker._serialize_tools` uses `json.dumps(..., sort_keys=True)`, which sorts -object *keys* — **not** array order. Array order is therefore load-bearing for -prefix identity. - -**Consequence:** scattering the seven built-in schemas into their pass modules -would make the tools-blob byte order depend on module import order, silently -invalidating the KV prefix for every existing conversation. KV cache is the -project's stated first priority. Do not do this. - -Two further guards live in the same file and constrain any change: -`tool_registry.py:391` asserts `BUILTIN_TOOL_NAMES` matches `TOOLS.keys()`, and -`tool_registry.py:413` asserts the pre/post-writer sets partition it. The second -is the one `features/cards/public_profile.py:46` cites as its reason to stay out -of the registry. - -**Acceptance test for any change in §4.2:** the tools tuple and prefix must be -byte-identical before and after. `kv_tracker` already computes this; assert on it -rather than eyeballing. - ---- +| autocomplete `local_ml.build_prompt` and the autocomplete-specific `complete` whitespace adapter | `features/autocomplete/` | +| `format_message_with_attachments`, `group_speaker_label`, and `build_prefix` | `prompting/base.py` | +| all of `inference/group_context.py` | `prompting/group_context.py` | +| `resolve_mood_fragment_randoms`, `compute_style_injection_block`, and `build_style_injection` | `prompting/scene_direction.py` | +| lorebook effective projection, catalog, selection, rendering, scan-depth contracts, and block builders | `prompting/lorebook.py` | +| `agentic_lorebook_active` | `features/lorebook/` | +| Director and Agentic Lorebook instruction builders | `pipeline/passes/director/prompts.py` | +| direction-note instruction and block rendering | `pipeline/passes/director/direction_note_prompts.py` | +| Editor and feedback instruction builders | `pipeline/passes/editor/prompts.py` | +| Dynamic Worlds instruction builder | `pipeline/passes/world_change_prompt.py` | +| shared tool-call instruction formatting and reasoning guidance | `pipeline/passes/_prompting.py` | +| concrete built-in schemas, choices, and dynamic schema builders | `prompting/tool_schemas.py` | +| ordered tool lookup, filtering, registration, and removal | `prompting/tool_catalog.py` | +| `strictify_schema` | `inference/schema.py` | +| Director-loop tool membership | `pipeline/tools.py` as `DIRECTOR_LOOP_TOOL_NAMES` | + +`features/lorebook/__init__.py` remains the feature facade. Feature callers use +that facade; lower consumers such as `workflows/toolkit.py` import +`prompting.lorebook` directly. + +The context-size route imports shared group and Scene Direction projections from +`prompting/`. It does not depend on private pass modules or expand the +`pipeline` facade for read-only formatting helpers. + +## 3. Preserved contracts + +Every relocation preserves: + +- HTTP request and response bodies; +- SSE event names, ordering, and payload shapes; +- database schema and stored JSON shapes; +- prompt text, whitespace, delimiters, message order, and parameter order; +- macro resolution timing, seeds, and stored random choices; +- tool names, descriptions, schemas, property order, choice objects, and final + array order; +- the names and signatures exported by `backend.workflows.toolkit`. + +Repository consumers migrate atomically with each move. Old +`backend.inference` re-exports are removed in the same phase so the final +architecture contains no `inference -> prompting` dependency. + +### Ordered tool contract + +The built-in order becomes an explicit tuple: + +```python +BUILTIN_TOOL_ORDER = ( + "direct_scene", + "editor_apply_patch", + "editor_rewrite", + "give_feedback", + "record_direction_note", + "select_lorebook", + "propose_world_changes", +) +``` -## 6. Recommended sequence +`BUILTIN_TOOL_NAMES` is derived from that tuple. Enabled built-ins are emitted +in tuple order. Workflow tools append in workflow registration order. +Re-registering an existing workflow tool preserves its position. -Ordered by risk, lowest first. Each step is independently shippable. +`prompting.tool_catalog` owns mutable catalog state. Production consumers use +catalog operations for lookup, registration, and removal; workflow replacement +does not mutate catalog dictionaries or standalone sets directly. +`workflows.toolkit` retains `TOOLS` and `STANDALONE_TOOLS` as read-only live +views for compatibility. Test fixtures restore catalog state through +catalog-owned operations instead of calling `clear()`, `update()`, `pop()`, +or `discard()` on those views. -| # | Action | Risk | Notes | -|---|---|---|---| -| 0 | Narrow the `AGENTS.md:17` wording (§3) | none | **Decide this first.** Steps 2–4 are downstream of it. | -| 1 | `rm -rf backend/inference/prose_rewriter/`; drop `roster_names` from the facade | none | Independent of everything else. | -| 2 | Move `lorebook.py` selection/gating/catalog/Dynamic-Worlds → `features/lorebook/`; leave block rendering | low | Collapses the pass-through facade. Watch the `workflows/toolkit.py:178` edge. | -| 3 | Move `local_ml.build_prompt` → autocomplete feature; delete the `__main__` block | low | Smallest real win. | -| 4 | Move ~510 lines of pass prose out of `prompt_builder.py` into `pipeline/passes/` owners | **medium** | Largest diff. Prefix-adjacent — §5 acceptance test required. | -| — | ~~Scatter `TOOLS` schemas~~ | — | **Rejected**, §5. | -| — | ~~`pov_*` → `analysis/`~~ | — | **Rejected**, §8. | -| — | `group_context.py` | — | Deferred, §4.4. | +Tool-order verification uses the raw request body captured at the HTTP transport +boundary for both OpenAI-compatible and Anthropic requests. Cached-base +verification also compares insertion-order-preserving serialization: -Steps 2 and 4 both touch `inference/__init__.py`; sequence them, do not parallelise. +```python +json.dumps(tools, separators=(",", ":"), ensure_ascii=False) +``` ---- +## 4. Implementation sequence + +Each phase is one reviewable commit and must satisfy its acceptance checks +before the next phase starts. + +### Phase 0 — make the target architecture executable + +1. Create `docs/architecture/prompting.md` with the ownership test, allowed + dependency graph, tool-order contract, and prompt-byte contract. +2. Update `AGENTS.md` with the same layer order and link responsibilities to + the architecture note. +3. Replace numeric ranks in `scripts/check_backend_layers.py` with one explicit + allowed-edge matrix containing `prompting`. +4. Make the checker fail when a Python-bearing top-level `backend/` package is + absent from the matrix. +5. Preserve the feature peer-slice rule in the shared checker. +6. Replace the duplicate policy in `tests/unit/test_import_layering.py` with + calls and focused negative cases against the shared checker. +7. Keep `tests/unit/test_backend_layers.py` as the repository-wide execution + test for that checker. + +Acceptance: + +- the current backend passes the shared AST checker; +- fixtures prove rejection of `inference -> prompting`, + `prompting -> inference`, `features -> workflows`, + `workflows -> features`, peer feature-slice imports, and unclassified + Python packages; +- the script and both test modules enforce the same matrix. + +### Phase 1 — pin behavior before relocation + +Add or strengthen exact-output tests for: + +- solo prefixes and all group-context modes; +- user attachments and workflow attachment annotations; +- constant, depth, keyword, Agentic, replacement, and suppression lore paths; +- combined and per-fragment Director prompts; +- lorebook selection, feedback, direction-note, world-change, and Editor + prompts; +- Scene Direction rendering, progressive fields, disabled Director state, and + stored-random mutation; +- autocomplete prompts with blank summaries, named group messages, truncation, + whitespace-ending drafts, and non-whitespace-ending drafts; +- the complete built-in tools blob; +- dynamic schema overrides, workflow registration order, workflow replacement, + orphan-tool removal, standalone tools, and forced `offer_tools` calls. + +Capture the actual request body for representative OpenAI-compatible and +Anthropic tool calls. Assert byte equality, including object-key and array +order. Keep the runtime KV tracker assertions as a second witness rather than +the byte oracle. + +Acceptance: + +- every moved behavior listed in Section 2 has characterization coverage at its + current location; +- the tool tests fail on either schema-key reordering or tool-array reordering; +- existing expected prompt and tool fixtures remain unchanged. + +### Phase 2 — extract autocomplete as one feature + +1. Add `backend/features/autocomplete/__init__.py` and its implementation + module. +2. Move `local_ml.build_prompt` into the feature. +3. Move `local_ml.complete` into the feature, preserving its trailing + `rstrip`/conditional `lstrip` behavior. +4. Call `inference.local_ml.acomplete("autocomplete", ...)` as the lower + runtime primitive. +5. Update `api/routes/messages.py`, package imports, and autocomplete tests. +6. Remove the `local_ml.py` executable self-check block after autocomplete and + POV assertions are covered by unit tests. + +Acceptance: + +- the API returns the same completion for whitespace and non-whitespace tails; +- `inference/local_ml.py` contains only generic load, completion, scoring, and + classification runtime contracts. + +### Phase 3 — add shared prompt projections + +1. Add `backend/prompting/__init__.py`, `base.py`, + `group_context.py`, and `scene_direction.py`. +2. Move prefix/message construction to `prompting/base.py`. +3. Move the complete group-context projection to + `prompting/group_context.py`. +4. Move Scene Direction projection and stored-random resolution to + `prompting/scene_direction.py`. +5. Update pipeline context construction, Writer/Director composition, + summarization, context-size reporting, workflow toolkit imports, tests, and + package facades. +6. Remove `roster_names` from the public package facade; keep it private to + `prompting.group_context`. +7. Update `docs/architecture/group-chats.md`, `kv-cache.md`, and + `secondary-workflow.md`. +8. Delete `inference/group_context.py` after all imports migrate. + +Acceptance: + +- prefix, attachment, group, Scene Direction, and context-size fixtures are + unchanged; +- `prompting/` imports only `core/`; +- `backend.workflows.toolkit` retains its existing public names and + signatures. + +### Phase 4 — move lorebook projection as one unit + +1. Add `prompting/lorebook.py`. +2. Move the effective-layer projection, keyword and Director selection, + catalog construction, rendering, scan-depth constants, and every lorebook + block builder into it. +3. Move `agentic_lorebook_active` into `features/lorebook/`. +4. Update the lorebook feature facade, proposals, pipeline context and passes, + API effective views, workflow toolkit, tests, and package exports. +5. Update database docstrings and + `docs/architecture/dynamic-worlds.md` to the new canonical module. +6. Delete `inference/lorebook.py` after all imports migrate. + +Acceptance: + +- lore selection and rendering have one implementation in + `prompting.lorebook`; +- constant, depth, keyword, Agentic, replacement, suppression, and off-turn + outputs match Phase 1; +- the feature gate is reached through `features.lorebook`; +- lower consumers import `prompting.lorebook`, not the feature facade. + +### Phase 5 — co-locate pass instruction contracts + +1. Add `pipeline/passes/_prompting.py` for shared tool-call instruction + formatting and reasoning guidance. +2. Add `pipeline/passes/director/prompts.py` for combined/per-fragment + Director prompts and Agentic Lorebook selection prompts. +3. Add `pipeline/passes/director/direction_note_prompts.py` for direction-note + instructions and block rendering. +4. Add `pipeline/passes/editor/prompts.py` for Editor and feedback + instructions. +5. Add `pipeline/passes/world_change_prompt.py` for the Dynamic Worlds + instruction. +6. Update each pass to import its local contract and update focused tests. +7. Remove moved names from `inference/__init__.py`. +8. Delete `inference/prompt_builder.py` after all repository consumers migrate. + +Acceptance: + +- all Phase 1 prompt strings remain unchanged; +- each instruction builder is imported only by its owning pass and tests; +- `inference/` contains no Orb-domain instruction prose. + +### Phase 6 — separate tool contracts from transport + +1. Add `inference/schema.py`, move `strictify_schema` into it, and update both + provider adapters and tests. +2. Add `prompting/tool_schemas.py` and move all built-in schema/choice objects + and dynamic schema builders into it. +3. Add `prompting/tool_catalog.py` with: + - `BUILTIN_TOOL_ORDER` and derived `BUILTIN_TOOL_NAMES`; + - catalog lookup and membership operations; + - `enabled_schemas` with its existing signature; + - workflow tool registration and removal operations; + - read-only live compatibility views for `TOOLS` and + `STANDALONE_TOOLS`. +4. Build the initial catalog from `BUILTIN_TOOL_ORDER`; built-in population + uses no import-time registration side effects. +5. Update workflow replacement to remove orphaned tools through the catalog + operation. +6. Update workflow test fixtures to restore registration state through catalog + operations. +7. Add `pipeline/tools.py` with + `DIRECTOR_LOOP_TOOL_NAMES = frozenset({"direct_scene"})`, replace + `PRE_WRITER_TOOLS` consumers, and remove `POST_WRITER_TOOLS` and its + partition assertion. +8. Update pipeline configuration, pass prompt lookups, settings filtering, + forced-call helpers, document audit, workflow toolkit, tests, docstrings, and + comments to the new owners and terminology. +9. Delete `inference/tool_registry.py` after all imports migrate. + +Acceptance: + +- built-in tool bytes equal the Phase 1 baseline at the cached-base and raw + transport boundaries; +- dynamic override, registration, replacement, orphan-removal, standalone, and + forced-offer paths preserve their Phase 1 order and bytes; +- production code mutates tool-catalog state only through catalog operations; +- toolkit compatibility views reject direct mutation; +- provider strictification output is unchanged. + +### Phase 7 — finish facades and documentation + +1. Restrict `inference/__init__.py` to model/runtime contracts. +2. Give `prompting/__init__.py` a small facade for genuinely shared contracts; + specialized consumers import canonical submodules. +3. Remove all repository imports of: + - `inference.prompt_builder`; + - `inference.group_context`; + - `inference.lorebook`; + - `inference.tool_registry`; + - moved names from the flat `inference` facade. +4. Update architecture documents, test descriptions, code comments, and + docstrings that identify the previous owners. +5. Run the dependency checker and the full validation suite. + +Acceptance: + +- `inference/` has no dependency on `prompting/` or an upper layer; +- `prompting/` has no dependency outside `core/`; +- one AST checker is the source of truth for backend dependency edges; +- every moved source module has been deleted; +- no compatibility re-export recreates the old dependency boundary. + +## 5. Validation + +Run focused checks after each phase: + +| Phase | Required focused checks | +|---|---| +| 0 | backend layer checker, `test_backend_layers.py`, `test_import_layering.py` | +| 1 | all characterization tests added or strengthened in the phase | +| 2 | autocomplete and local-ML unit/integration tests | +| 3 | group context, prefix, attachment, Scene Direction, context-size, off-turn prefix, and KV entry-point tests | +| 4 | lore matching, Agentic Lorebook, Dynamic Worlds, constant/depth lore, off-turn prefix, and API effective-view tests | +| 5 | interactive fragments, Director, lore selection, feedback, direction notes, Editor, world proposal, and KV invariant tests | +| 6 | tool catalog, provider structured calls, workflow registry, forced calls, offer-tools blob, KV invariant, and real-stack tests | +| 7 | all affected tests above plus import and layer checks | -## 7. Checked and cleared — do not redo +For Phases 3–6, compare the Phase 1 prompt/tool fixtures before accepting the +commit. A relocation commit produces no fixture change. -| Question | Finding | -|---|---| -| Layer violations out of `inference/`? | **None.** Only `..core` leaves the package; all other relative imports are intra-package. (First attempt used a regex blind to relative imports; re-run corrected, conclusion unchanged.) | -| Two local runtimes = duplication? | **No.** `local_ml` (in-process llama-cpp-python) and `llama_server/` (subprocess) are a deliberate split behind a `RuntimeKind` discriminator on `ModelSpec` (`local_models/catalog.py:49`, `:115`), sharing one catalog. | -| `endpoint_profiles.PROFILES` vs the "derive or degrade, never hand-keep a capability table" rule? | **Consistent.** The whole table is 2 endpoint keys (`api.deepseek.com`, `nano-gpt.com`) with 1 per-model override (`deepseek-reasoner`); unmatched → `None` → pass-through unchanged; entries encode *observed* rejections, not guessed capabilities. `anthropic.py:21,26` learns rejections at runtime, which is the derive pattern. | -| `inference/errors.redact` duplicating `core/utils.scrub_log`? | **No.** Unrelated: API-key removal vs. CWE-117 newline stripping. | -| `build_style_injection` dead? | **No.** Called internally at `prompt_builder.py:669`, directly tested in `tests/unit/test_interactive_fragments.py`. | -| `strictify_schema` misplaced with the schemas? | **No.** Consumed by `client.py` and `anthropic.py` — transport concern, correctly in layer 5. | - ---- - -## 8. Withdrawn claims (calibration) - -Claims made during this audit and retracted on verification. Listed so a reviewer -can weigh the surviving ones. - -1. **`pov_input` / `pov_from_logits` → `analysis/`.** Withdrawn. Asserted their - consumer was `workflows/image_gen/pov.py:70`; that line calls `aclassify_pov`, - the async *runtime* wrapper. The pure helpers have no production consumer - outside `local_ml` itself. Moving them would relocate two test files so that - `local_ml` could import its own helpers back — over-engineering. -2. **Scattering `TOOLS` schemas into pass modules.** Withdrawn on discovering §5. -3. **`build_style_injection` is dead code.** False — the consumer grep excluded - the defining file, hiding an internal call. -4. **"Every pass-prompt function has exactly one consumer."** Overstated; see the - table in §4.2 for the two exceptions. -5. **`tool_registry.py:392` is the partition assert.** Wrong line; it is `:413`. -6. **`features/lorebook/__init__.py` is a "pass-through shell."** Overstated — - 15 of ~28 names, roughly half. -7. **"`PROFILES` has 3 per-model overrides"** (in a draft of §7). Miscounted: the - regex conflated endpoint keys with model keys. Actual shape is 2 endpoints, - 1 model override. Corrected in place; the verdict was unaffected. - ---- - -## 9. Open questions for review - -1. **Is the §2 axis-of-change test the right principle?** It is the load-bearing - assumption. Most of §4 collapses if it is wrong. -2. **Is step 4 worth its risk?** ~510 lines moved, prefix-adjacent, for a purely - structural gain. A reasonable reviewer could accept steps 0–3 and reject 4. - What would change the answer is whether the pass prose is expected to churn: - if it is edited often, the move pays for itself; if it is stable, it may not. -3. **Should the shared lorebook rendering layer live in `inference/` or in - `analysis/`?** §4.1 proposes `inference/` by analogy with the llama_server - precedent. `analysis/` is arguably a better fit for pure rendering, and the - `workflows → analysis` edge already exists. -4. **`kv_tracker._prev_turn_entries` grows without bound.** Module-level dict - (`kv_tracker.py:13`), written at `:220` keyed by `conversation_id`, read at - `:97`, with no `pop`, `clear`, or size cap anywhere in the module — verified. - One retained entry per conversation for the process lifetime. This is a - diagnostics cache, so the leak is slow and bounded by conversation count, but - it is real. Out of scope for a placement audit; raised because it was found - during it. Placement itself is fine — it is a cache-contract concern, layer 5. -5. **Is there a fifth tenant this audit missed?** Coverage was driven by the §2 - test applied to all 25 modules, but the test itself may have blind spots — - particularly around `cached_call.py` and `kv_tracker.py`, which were accepted - as infrastructure without deep reading. - ---- - -## 10. How to re-verify +Final validation from the repository root: ```sh -# §7 layer check (relative-import aware — the naive regex misses these) -grep -rnE "^\s*from\s+\.+(api|pipeline|features|workflows|database)\b" backend/inference/ --include="*.py" -grep -rhoE "^\s*from\s+\.[.a-z_]*" backend/inference/ --include="*.py" | sort | uniq -c | sort -rn - -# §4.2 consumer table -for n in build_editor_prompt build_feedback_prompt build_world_change_prompt \ - compute_style_injection_block resolve_mood_fragment_randoms; do - printf "%-32s " "$n" - grep -rn "\b$n\b" --include="*.py" backend/ | grep -v __pycache__ \ - | grep -v "inference/prompt_builder.py\|inference/__init__.py" \ - | sed 's|backend/||;s|:.*||' | sort -u | tr '\n' ' '; echo -done - -# §5 cache contract -grep -n "tools" backend/inference/cached_call.py -sed -n '78,96p' backend/pipeline/config.py -grep -n "^assert" backend/inference/tool_registry.py +./scripts/format_backend.sh +./scripts/format_frontend.sh +./scripts/lint.sh +./scripts/tests.sh all ``` -Note: consumer greps must **not** exclude the defining file — that error produced -withdrawn claim 3. +Review the final diff after formatting. The refactor is complete when all +acceptance checks pass and the public HTTP, SSE, persistence, frontend, and +workflow-toolkit contracts remain unchanged. From 84b9f531398e5ab1728d7ffb236c2902705749b9 Mon Sep 17 00:00:00 2001 From: Chi Date: Sun, 6 Sep 2026 18:36:08 +0700 Subject: [PATCH 3/6] agent lane --- backend/core/__init__.py | 2 + backend/core/utils.py | 73 ++++++++++-- backend/features/documents/audit.py | 6 +- backend/features/summarization/summarizer.py | 15 +-- .../passes/director/direction_note.py | 2 +- backend/pipeline/passes/director/director.py | 4 +- .../passes/director/lorebook_select.py | 2 +- backend/pipeline/passes/editor/editor.py | 2 +- backend/pipeline/passes/editor/feedback.py | 2 +- backend/pipeline/passes/world_change.py | 2 +- backend/workflows/_forced_call.py | 18 ++- backend/workflows/image_gen/composer.py | 17 ++- docs/architecture/endpoints.md | 14 +++ docs/architecture/secondary-workflow.md | 4 +- docs/multimedia/image-generation.md | 5 + frontend/settings_models.js | 8 +- .../test_agent_lane_hyperparams.py | 110 ++++++++++++++++++ tests/unit/test_extract_hyperparams.py | 74 ++++++++++++ tests/unit/workflows/test_forced_call.py | 37 ++++++ 19 files changed, 355 insertions(+), 42 deletions(-) create mode 100644 tests/integration/test_agent_lane_hyperparams.py create mode 100644 tests/unit/test_extract_hyperparams.py diff --git a/backend/core/__init__.py b/backend/core/__init__.py index 8be66030..ef85cb2e 100644 --- a/backend/core/__init__.py +++ b/backend/core/__init__.py @@ -32,6 +32,7 @@ split_sentences, ) from .utils import ( + agent_lane_max_tokens, build_multimodal_content, estimate_tokens, extract_hyperparams, @@ -71,6 +72,7 @@ "remove_quoted_spans", "split_sentences", # utils — token/log/multimodal helpers + "agent_lane_max_tokens", "build_multimodal_content", "estimate_tokens", "extract_hyperparams", diff --git a/backend/core/utils.py b/backend/core/utils.py index 41e4451f..0ec19472 100644 --- a/backend/core/utils.py +++ b/backend/core/utils.py @@ -5,6 +5,7 @@ from collections.abc import Mapping, Sequence from typing import Any +from .domain_types import AgentLane from .llm_types import ContentPart #: Heuristic characters-per-token ratio used for rough context-size estimates. @@ -31,27 +32,75 @@ def scrub_log(value: object) -> str: return str(value).replace("\r", "").replace("\n", "") -def extract_hyperparams(settings: Mapping[str, Any], *, defaults: Mapping[str, Any] | None = None) -> dict: - """Extract LLM hyperparameters from a settings dict. +#: The sampler/budget fields a settings row carries for a lane, in the order the +#: endpoint editor shows them. +_HYPERPARAM_KEYS = ( + "temperature", + "max_tokens", + "top_p", + "min_p", + "top_k", + "repetition_penalty", +) - Optionally fills in *defaults* for any keys not present in settings. + +def extract_hyperparams( + settings: Mapping[str, Any], + *, + lane: AgentLane = "writer", + token_floor: int | None = None, + defaults: Mapping[str, Any] | None = None, +) -> dict: + """Extract LLM hyperparameters from a settings dict for the lane making the call. + + The agent lane reads each key's ``agent_`` twin. ``get_settings`` overlays those + from the agent endpoint's own model config, and only when a separate lane + resolves, so single-model mode falls through to the writer's values -- which is + the same endpoint it is calling. Passing the writer's lane to an agent call is + not a harmless default: it sends one endpoint's preset to another. The fallback + is per key rather than whole-row only as a guard for partial mappings; the six + columns behind these keys are all NOT NULL, so a resolved agent lane carries + every twin and an unresolved one carries none. + + ``token_floor`` is what the call needs to answer in full. The configured budget + may only *raise* it: a budget is a reply-length preference, and a short-reply + preset is a normal setting, while the floor is what the call needs to answer at + all -- so honoring a smaller one would truncate the reply mid-answer and turn a + sampling preference into a silent failure. Every call whose whole answer has to + fit in one reply (a forced tool call, a constrained-decoding call) passes one; + passes that stream prose take the setting as-is and leave it unset. + + Optionally fills in *defaults* for any keys not present in settings. Note that + both ``settings`` and ``model_configs`` declare all six columns NOT NULL, so + *defaults* only ever fires for a partial mapping, never for a real row -- it is + not a way to spell a minimum, which is what ``token_floor`` is for. """ - keys = [ - "temperature", - "max_tokens", - "top_p", - "min_p", - "top_k", - "repetition_penalty", - ] - params = {k: v for k in keys if (v := settings.get(k)) is not None} + prefix = "agent_" if lane == "agent" else "" + params: dict[str, Any] = {} + for key in _HYPERPARAM_KEYS: + value = settings.get(f"{prefix}{key}") if prefix else None + if value is None: + value = settings.get(key) + if value is not None: + params[key] = value if defaults: for k, v in defaults.items(): if k not in params: params[k] = v + if token_floor is not None: + params["max_tokens"] = max(token_floor, int(params.get("max_tokens") or 0)) return params +def agent_lane_max_tokens(settings: Mapping[str, Any], *, floor: int) -> int: + """The agent lane's reply budget for a call that needs at least *floor* tokens. + + The lane cascade and the floor rule are ``extract_hyperparams``'; this is the + spelling for a caller that sets its own samplers and wants only the budget. + """ + return int(extract_hyperparams(settings, lane="agent", token_floor=floor)["max_tokens"]) + + def build_multimodal_content(text: str, attachments: Sequence[Mapping[str, Any]] | None = None) -> str | list[ContentPart]: """Wrap *text* (and optional image attachments) into a multimodal content list. diff --git a/backend/features/documents/audit.py b/backend/features/documents/audit.py index bb3010f5..8d923d64 100644 --- a/backend/features/documents/audit.py +++ b/backend/features/documents/audit.py @@ -265,7 +265,11 @@ async def patch_document( # generation prompt is what keeps the KV prefix warm); only the scanners # see the cleaned/capped ctx above. report_text = format_numbered_report(targets) - params = extract_hyperparams(settings, defaults={"temperature": 0.25, "max_tokens": 8192}) + # Writer lane on purpose (the route serves this call from the writer endpoint, + # for byte parity with the prompt that generated the draft), but floored like + # the agent-lane forced calls: the whole patch set has to fit in one reply, and + # a document preset kept short for brief continuations would truncate it. + params = extract_hyperparams(settings, token_floor=8192, defaults={"temperature": 0.25}) schema = TOOLS["editor_apply_patch"]["schema"] if client.completion_mode == "text": # Both text shapes byte-extend the generation prompt as a raw diff --git a/backend/features/summarization/summarizer.py b/backend/features/summarization/summarizer.py index 8db552b9..3a6e6a57 100644 --- a/backend/features/summarization/summarizer.py +++ b/backend/features/summarization/summarizer.py @@ -3,7 +3,7 @@ from collections.abc import AsyncGenerator, Mapping, Sequence from typing import Any -from ...core import ChatMessage, Macros, TurnCast +from ...core import ChatMessage, Macros, TurnCast, extract_hyperparams from ...inference import LLMClient, prompt_builder DEFAULT_SUMMARY_INSTRUCTIONS = ( @@ -13,15 +13,6 @@ "Be thorough — this will be the sole context for the story's continuation.]" ) -_LLM_PARAMS = ( - "temperature", - "max_tokens", - "top_p", - "min_p", - "top_k", - "repetition_penalty", -) - class ConversationSummarizer: def __init__(self, client: LLMClient, settings: Mapping[str, Any]): @@ -61,7 +52,9 @@ def build_messages( return prefix + [{"role": "user", "content": instructions}] async def stream(self, llm_messages: Sequence[Mapping[str, Any]], model: str) -> AsyncGenerator[str, None]: - params = {k: v for k in _LLM_PARAMS if (v := self.settings.get(k)) is not None} + # Writer lane: the summary is prose in the user's own preset, and it is + # written with the model that writes the story. + params = extract_hyperparams(self.settings) async for chunk in self.client.complete(llm_messages, model, **params): if chunk["type"] == "content": yield chunk["delta"] diff --git a/backend/pipeline/passes/director/direction_note.py b/backend/pipeline/passes/director/direction_note.py index 6e88b6c3..fa578367 100644 --- a/backend/pipeline/passes/director/direction_note.py +++ b/backend/pipeline/passes/director/direction_note.py @@ -95,7 +95,7 @@ async def direction_note_step( per_fragment_on = bool(settings.get("director_individual_fragments", 0)) groups = [[df] for df in direction_note_fragments] if per_fragment_on else [list(direction_note_fragments)] - hyperparams = extract_hyperparams(settings, defaults={"temperature": 0.4, "max_tokens": 2048}) + hyperparams = extract_hyperparams(settings, lane="agent", token_floor=2048, defaults={"temperature": 0.4}) notes: list[dict] = [] raws: list[str] = [] diff --git a/backend/pipeline/passes/director/director.py b/backend/pipeline/passes/director/director.py index 3b10a9a4..76a8c705 100644 --- a/backend/pipeline/passes/director/director.py +++ b/backend/pipeline/passes/director/director.py @@ -263,7 +263,7 @@ async def director_pass( plans_speakers = SPEAKING_PLAN_FIELD in scene_fields if name == "direct_scene" and per_fragment_on and (interactive_fragments or plans_speakers): reasoning_params = reasoning_cfg(reasoning_on, reasoning_prefill) - hyperparams = extract_hyperparams(settings, defaults={"temperature": 0.25, "max_tokens": 8192}) + hyperparams = extract_hyperparams(settings, lane="agent", token_floor=8192, defaults={"temperature": 0.25}) # One forced call per fragment, each shown the values already chosen # this turn so later fragments build on earlier ones. Moods are @@ -362,7 +362,7 @@ async def director_pass( # direction-note steps. Aborting the turn here would also skip # persisting the finished reply. reasoning_params = reasoning_cfg(reasoning_on, reasoning_prefill) - hyperparams = extract_hyperparams(settings, defaults={"temperature": 0.25, "max_tokens": 8192}) + hyperparams = extract_hyperparams(settings, lane="agent", token_floor=8192, defaults={"temperature": 0.25}) try: async for event in base.complete_into( client, diff --git a/backend/pipeline/passes/director/lorebook_select.py b/backend/pipeline/passes/director/lorebook_select.py index e1e2f5ce..0dee9156 100644 --- a/backend/pipeline/passes/director/lorebook_select.py +++ b/backend/pipeline/passes/director/lorebook_select.py @@ -58,7 +58,7 @@ async def lorebook_select_step( request = build_lorebook_select_prompt(catalog, user_message, reasoning_on=reasoning_on) trailing = [{"role": "user", "content": request}] - hyperparams = extract_hyperparams(settings, defaults={"temperature": 0.25, "max_tokens": 2048}) + hyperparams = extract_hyperparams(settings, lane="agent", token_floor=2048, defaults={"temperature": 0.25}) resp: dict = {} try: diff --git a/backend/pipeline/passes/editor/editor.py b/backend/pipeline/passes/editor/editor.py index e599330b..ef6826f7 100644 --- a/backend/pipeline/passes/editor/editor.py +++ b/backend/pipeline/passes/editor/editor.py @@ -539,7 +539,7 @@ async def _run_edit_loop( report.total_issues, ) try: - hyperparams = extract_hyperparams(settings, defaults={"temperature": 0.25, "max_tokens": 8192}) + hyperparams = extract_hyperparams(settings, lane="agent", token_floor=8192, defaults={"temperature": 0.25}) reasoning_params = reasoning_cfg(reasoning_on, reasoning_prefill) if not reasoning_params["reasoning"].get("enabled", True): logger.info("Editor iteration %d: reasoning disabled", iteration + 1) diff --git a/backend/pipeline/passes/editor/feedback.py b/backend/pipeline/passes/editor/feedback.py index 5ce9dc0e..ce0dd9f1 100644 --- a/backend/pipeline/passes/editor/feedback.py +++ b/backend/pipeline/passes/editor/feedback.py @@ -87,7 +87,7 @@ async def feedback_step( {"role": "user", "content": request}, ] - hyperparams = extract_hyperparams(settings, defaults={"temperature": 0.4, "max_tokens": 2048}) + hyperparams = extract_hyperparams(settings, lane="agent", token_floor=2048, defaults={"temperature": 0.4}) resp: dict = {} # Errors propagate out like the director/writer/editor passes. diff --git a/backend/pipeline/passes/world_change.py b/backend/pipeline/passes/world_change.py index c3f9171e..0c25229a 100644 --- a/backend/pipeline/passes/world_change.py +++ b/backend/pipeline/passes/world_change.py @@ -87,7 +87,7 @@ async def world_change_step( {"role": "assistant", "content": reply_text}, {"role": "user", "content": request}, ] - hyperparams = extract_hyperparams(settings, defaults={"temperature": 0.3, "max_tokens": 2048}) + hyperparams = extract_hyperparams(settings, lane="agent", token_floor=2048, defaults={"temperature": 0.3}) resp: dict = {} try: diff --git a/backend/workflows/_forced_call.py b/backend/workflows/_forced_call.py index e4400bfc..54b91409 100644 --- a/backend/workflows/_forced_call.py +++ b/backend/workflows/_forced_call.py @@ -7,7 +7,7 @@ from types import MappingProxyType from typing import Any -from ..core import ReasoningChannel, mark_call_start +from ..core import ReasoningChannel, agent_lane_max_tokens, mark_call_start from ..inference import ( STANDALONE_TOOLS, TOOLS, @@ -55,10 +55,18 @@ async def forced_tool_call( model_name: str | None = None, reasoning_on: bool = True, temperature: float = 0.25, - max_tokens: int = 8192, + token_floor: int = 8192, tools_in_prompt: bool = True, ) -> AsyncIterator[dict]: - """Run one forced tool call and yield its parsed arguments.""" + """Run one forced tool call and yield its parsed arguments. + + ``token_floor`` is what this call needs to answer in full; the agent lane's + configured ``max_tokens`` raises it when the user has given that endpoint more + room (see :func:`~backend.core.agent_lane_max_tokens`), the same floor the + Director and Editor forced calls apply. ``temperature`` stays a caller + constant: a forced call fills a schema, so a roleplay preset would only add + flourish to it -- the same split ``features.cards._drafting`` documents. + """ schema = TOOLS[tool_name]["schema"] resolved_model = model_name or settings["model_name"] reasoning_params = reasoning_cfg(reasoning_on) @@ -137,7 +145,7 @@ async def _attempt(tool_array: list[dict]) -> AsyncIterator[dict]: tools=tool_array, tool_choice=TOOLS[tool_name]["choice"], temperature=temperature, - max_tokens=max_tokens, + max_tokens=agent_lane_max_tokens(settings, floor=token_floor), tools_in_prompt=tools_in_prompt, **reasoning_params, ) @@ -159,7 +167,7 @@ def _parse() -> tuple[dict, bool]: The second flag is the only sound evidence that tool selection was left to the model: a reply with no call at all proves nothing (truncated at - max_tokens mid-reasoning, a content-only answer, a provider-side + the token budget mid-reasoning, a content-only answer, a provider-side finish_reason=error), and treating it as evidence would drop the shared blob for the whole session over one flaky reply. """ diff --git a/backend/workflows/image_gen/composer.py b/backend/workflows/image_gen/composer.py index e88cf5ad..2c8bc10b 100644 --- a/backend/workflows/image_gen/composer.py +++ b/backend/workflows/image_gen/composer.py @@ -28,7 +28,16 @@ logger = logging.getLogger(__name__) -async def _forced_args(*, client, model_name, prefix, tail, tool_name, settings, max_tokens, reasoning_on) -> dict: +# What each call needs to answer in full: a compact JSON argument object, plus room +# for the reasoning that precedes it when `prompter_reasoning` is on. Composition +# writes the prompt itself, so it gets the larger of the two. The agent endpoint's +# configured max_tokens raises these when it is higher; a writer preset kept short +# for brief replies never lowers them (see `agent_lane_max_tokens`). +_ANALYZE_TOKENS = 2_048 +_COMPOSE_TOKENS = 4_096 + + +async def _forced_args(*, client, model_name, prefix, tail, tool_name, settings, token_floor, reasoning_on) -> dict: logger.info("[image_gen] %s tail:\n%s", tool_name, "\n--\n".join(m["content"] for m in tail)) args: dict = {} async for event in forced_tool_call( @@ -41,7 +50,7 @@ async def _forced_args(*, client, model_name, prefix, tail, tool_name, settings, # One workflow-owned mode for both calls, so they share a reasoning-forked lane. reasoning_on=reasoning_on, temperature=0.2, - max_tokens=max_tokens, + token_floor=token_floor, offer_tools=OFFER_TOOLS, ): if event.get("type") == "result" and isinstance(event.get("args"), dict): @@ -238,7 +247,7 @@ async def analyze_scene( tail=[{"role": "user", "content": analyze_ooc(pov, supports_negative, _sheets(subjects))}], tool_name="analyze_scene", settings=settings, - max_tokens=2_048, + token_floor=_ANALYZE_TOKENS, reasoning_on=reasoning_on, ) # First-person view is the user looking at the subject: keep only the subject @@ -308,7 +317,7 @@ async def compose_scene( tail=tail, tool_name="compose_image_prompt", settings=settings, - max_tokens=4_096, + token_floor=_COMPOSE_TOKENS, reasoning_on=reasoning_on, ) diff --git a/docs/architecture/endpoints.md b/docs/architecture/endpoints.md index b778bd64..55e3353c 100644 --- a/docs/architecture/endpoints.md +++ b/docs/architecture/endpoints.md @@ -69,6 +69,20 @@ route or an ambiguous route. An explicit OpenAI resource only retries when the response names the native `x-api-key` header. This retry is independently bounded and never changes hosts; provider and model names are not evidence. +## Lane presets + +Each lane sends the sampler preset of the endpoint it is calling. Director, +Editor, and workflow tool calls read the Agent model config's temperature, +budget, and samplers; they fall back to the Writer's whenever the Agent lane does +not resolve, which is what one endpoint serving both lanes means. + +Agent-lane forced tool calls raise the configured budget to what their answer +needs and never lower it, because the whole reply has to fit inside one call and +a truncated one reaches the user as the pass doing nothing. A short-reply budget +therefore shapes prose without breaking a tool call. The document Output Auditor +takes the same floor from the other lane: it patches on the Writer endpoint, to +keep byte parity with the prompt that generated the draft. + ## Provider request behavior Native Anthropic requests are built from an allowlist. System messages are diff --git a/docs/architecture/secondary-workflow.md b/docs/architecture/secondary-workflow.md index c78b4b5b..e1c5ac83 100644 --- a/docs/architecture/secondary-workflow.md +++ b/docs/architecture/secondary-workflow.md @@ -177,7 +177,9 @@ the main reply and other workflows can continue. Use `forced_tool_call` for a one-shot tool call. Pass the context's prefix, enabled tools, schema overrides, client, and cache tracker so the call follows -the same prompt and cache rules as the main turn. +the same prompt and cache rules as the main turn. Its `token_floor` is what the +call needs to answer in full; the Agent lane's configured `max_tokens` raises it +when that endpoint has more room, and never lowers it below the floor. Public hook events pass through to SSE. Core events and names beginning with `_` are reserved. A useful custom event is `phase_status` with a channel that diff --git a/docs/multimedia/image-generation.md b/docs/multimedia/image-generation.md index 9af3630f..9a989a43 100644 --- a/docs/multimedia/image-generation.md +++ b/docs/multimedia/image-generation.md @@ -248,6 +248,11 @@ complex scene or writes the image prompt. It applies to both prompt steps and ca increase token use. Stable thinking settings generally give better prompt-cache reuse. +Each prompt step has its own reply budget, and the Agent endpoint's configured +**Max Tokens** raises it when that setting is higher — room a thinking model can +spend before it answers. A lower setting is left to the writing it was chosen for: +it never shrinks a prompt step below what the step needs to finish. + ## Troubleshooting | Problem | Try this | diff --git a/frontend/settings_models.js b/frontend/settings_models.js index 6e3c4bd3..3726f208 100644 --- a/frontend/settings_models.js +++ b/frontend/settings_models.js @@ -44,7 +44,7 @@ const SETTING_FIELDS = [ { k: "shared_system_prompt", l: "System Prompt (global)", t: "textarea" }, { k: "system_prompt", l: "System Prompt (model)", t: "textarea" }, { k: "temperature", l: "Temperature", t: "number", s: "0.05", mn: "0", mx: "2" }, - { k: "max_tokens", l: "Max Tokens", t: "number", s: "64", mn: "64", mx: "8192" }, + { k: "max_tokens", l: "Max Tokens", t: "number", s: "64", mn: "64", mx: "32768" }, { k: "top_p", l: "Top P", t: "number", s: "0.05", mn: "0", mx: "1" }, { k: "min_p", l: "Min P", t: "number", s: "0.01", mn: "0", mx: "1" }, { k: "top_k", l: "Top K", t: "number", s: "1", mn: "0", mx: "200" }, @@ -68,7 +68,10 @@ const FIELD_GROUPS = [ const AGENT_MODEL_HYPERPARAM_KEYS = [ "agent_shared_system_prompt", "agent_temperature", + "agent_max_tokens", "agent_top_p", + "agent_min_p", + "agent_top_k", "agent_repetition_penalty", "agent_reasoning_effort", "agent_reasoning_effort_param", @@ -93,7 +96,10 @@ const AGENT_SETTING_FIELDS = [ { k: "agent_proxy", l: "Agent Proxy", t: "text", ph: "socks5://127.0.0.1:1080" }, { k: "agent_shared_system_prompt", l: "Agent System Prompt (global)", t: "textarea" }, { k: "agent_temperature", l: "Agent Temperature", t: "number", s: "0.05", mn: "0", mx: "2" }, + { k: "agent_max_tokens", l: "Agent Max Tokens", t: "number", s: "64", mn: "64", mx: "32768" }, { k: "agent_top_p", l: "Agent Top P", t: "number", s: "0.05", mn: "0", mx: "1" }, + { k: "agent_min_p", l: "Agent Min P", t: "number", s: "0.01", mn: "0", mx: "1" }, + { k: "agent_top_k", l: "Agent Top K", t: "number", s: "1", mn: "0", mx: "200" }, { k: "agent_repetition_penalty", l: "Agent Rep. Penalty", t: "number", s: "0.05", mn: "1", mx: "2" }, { k: "agent_reasoning_effort", l: "Agent Reasoning Effort", t: "reasoning_effort" }, { k: "agent_extra_headers", l: "Agent Extra Request Headers", t: "textarea", ph: "X-Provider: deepinfra" }, diff --git a/tests/integration/test_agent_lane_hyperparams.py b/tests/integration/test_agent_lane_hyperparams.py new file mode 100644 index 00000000..90039a78 --- /dev/null +++ b/tests/integration/test_agent_lane_hyperparams.py @@ -0,0 +1,110 @@ +"""Each lane sends the sampler preset of the endpoint it is calling. + +The Agent endpoint carries its own model config -- temperature, budget, samplers -- +and the Agent passes are the ones dialing that endpoint. Sending the Writer's preset +there is the bug this file guards: it reads as the Agent ignoring its own settings. + +The budget is the one key that does not pass straight through. A forced tool call +has to fit its whole answer in one reply, so the configured `max_tokens` may raise +that call's floor but never lower it. +""" + +from __future__ import annotations + +from typing import Any + +import backend.database as dbmod +from backend.pipeline import handle_turn + +# Deliberately far apart, and each key different from the other lane's, so a mixed +# spread fails on the key that leaked rather than passing on a shared default. The +# Agent budget sits above the Director's 8192 floor so it survives verbatim; the +# floored case gets its own test below. +_SAMPLERS = ("temperature", "top_k", "min_p") +_WRITER_PRESET = {"temperature": 1.15, "max_tokens": 700, "top_k": 80, "min_p": 0.02} +_AGENT_PRESET = {"temperature": 0.4, "max_tokens": 16384, "top_k": 20, "min_p": 0.1} +_DIRECTOR_FLOOR = 8192 + + +async def _drain(agen) -> list[dict]: + return [ev async for ev in agen] + + +async def _config_id(client, endpoint_id: int, role: str) -> int: + models = (await client.get(f"/api/endpoints/{endpoint_id}/models")).json() + return next(m["id"] for m in models if m["role"] == role) + + +def _params(captured: list[dict], pass_name: str) -> dict[str, Any]: + return next(c["params"] for c in captured if c["pass"] == pass_name) + + +def _samplers(captured: list[dict], pass_name: str) -> dict[str, Any]: + params = _params(captured, pass_name) + return {k: params[k] for k in _SAMPLERS if k in params} + + +async def _two_lane_setup(client, agent_preset: dict) -> None: + writer_endpoint = (await client.get("/api/endpoints")).json()[0]["id"] + await client.put(f"/api/models/{await _config_id(client, writer_endpoint, 'writer')}", json=_WRITER_PRESET) + + # A new endpoint auto-provisions a writer and an agent model config; the agent + # lane reads the latter once `agent_same_as_writer` is off. + agent_endpoint = (await client.post("/api/endpoints", json={"url": "http://agent.local", "api_key": "k"})).json()["id"] + await client.put(f"/api/models/{await _config_id(client, agent_endpoint, 'agent')}", json=agent_preset) + await client.put( + "/api/settings", + json={ + "agent_same_as_writer": False, + "agent_endpoint_id": agent_endpoint, + "enable_agent": True, + "enabled_tools": {"direct_scene": True}, + }, + ) + + +async def _run_turn(cid: str, llm_mock) -> None: + await dbmod.create_conversation(cid, "presets", "Bot", "a scenario") + llm_mock.enqueue_director([{"type": "function", "function": {"name": "direct_scene", "arguments": {"moods": []}}}]) + llm_mock.enqueue_writer("She nods slowly.") + await _drain(handle_turn(cid, "hello")) + + +async def test_each_lane_sends_the_preset_of_the_endpoint_it_calls(client, db, llm_mock): + await _two_lane_setup(client, _AGENT_PRESET) + await _run_turn("conv-lane-presets", llm_mock) + + writer_params = _params(llm_mock.captured, "writer") + assert {k: writer_params[k] for k in _WRITER_PRESET} == _WRITER_PRESET + assert _samplers(llm_mock.captured, "director") == {k: _AGENT_PRESET[k] for k in _SAMPLERS} + # Above the floor, so the Agent endpoint's own budget is what goes out. + assert _params(llm_mock.captured, "director")["max_tokens"] == _AGENT_PRESET["max_tokens"] + + +async def test_a_short_agent_budget_never_shrinks_a_forced_call(client, db, llm_mock): + """1024 tokens is a normal setting for an endpoint kept to brief replies. + + Honoring it for `direct_scene` truncates the tool call mid-arguments, which + degrades to empty arguments and reaches the user as the Director silently doing + nothing -- so the floor wins, while every sampler still comes from the Agent. + """ + await _two_lane_setup(client, {**_AGENT_PRESET, "max_tokens": 1024}) + await _run_turn("conv-lane-floored", llm_mock) + + assert _samplers(llm_mock.captured, "director") == {k: _AGENT_PRESET[k] for k in _SAMPLERS} + assert _params(llm_mock.captured, "director")["max_tokens"] == _DIRECTOR_FLOOR + + +async def test_one_endpoint_for_both_lanes_keeps_sending_its_preset(client, db, llm_mock): + """Single-model mode has no `agent_*` overlay: the agent passes read the same + row the writer does, because it is the same endpoint they are calling.""" + writer_endpoint = (await client.get("/api/endpoints")).json()[0]["id"] + await client.put(f"/api/models/{await _config_id(client, writer_endpoint, 'writer')}", json=_WRITER_PRESET) + await client.put("/api/settings", json={"enable_agent": True, "enabled_tools": {"direct_scene": True}}) + + await _run_turn("conv-single-lane-presets", llm_mock) + + assert _samplers(llm_mock.captured, "director") == {k: _WRITER_PRESET[k] for k in _SAMPLERS} + # The writer's 700-token preset is honored for prose and floored for the call. + assert _params(llm_mock.captured, "writer")["max_tokens"] == _WRITER_PRESET["max_tokens"] + assert _params(llm_mock.captured, "director")["max_tokens"] == _DIRECTOR_FLOOR diff --git a/tests/unit/test_extract_hyperparams.py b/tests/unit/test_extract_hyperparams.py new file mode 100644 index 00000000..32e61aeb --- /dev/null +++ b/tests/unit/test_extract_hyperparams.py @@ -0,0 +1,74 @@ +"""The lane cascade and token floor `extract_hyperparams` applies to a settings row.""" + +from __future__ import annotations + +import pytest + +from backend.core import agent_lane_max_tokens, extract_hyperparams + +_WRITER = {"temperature": 0.8, "max_tokens": 4096, "top_p": 0.95, "min_p": 0.0, "top_k": 40, "repetition_penalty": 1.0} + + +def test_the_writer_lane_never_reads_the_agent_overlay(): + params = extract_hyperparams({**_WRITER, "agent_temperature": 0.2, "agent_max_tokens": 512}) + assert params == _WRITER + + +def test_the_agent_lane_prefers_its_own_values(): + settings = {**_WRITER, "agent_temperature": 0.2, "agent_max_tokens": 512} + params = extract_hyperparams(settings, lane="agent") + assert params["temperature"] == 0.2 + assert params["max_tokens"] == 512 + + +def test_a_partial_mapping_falls_back_per_key(): + # A real settings row is all-or-nothing -- every `agent_` twin comes from the + # same overlay of six NOT NULL columns -- so this guards callers that hand in a + # hand-built mapping rather than a state the database can reach. + params = extract_hyperparams({**_WRITER, "agent_temperature": 0.2}, lane="agent") + assert params == {**_WRITER, "temperature": 0.2} + + +def test_defaults_only_fill_keys_no_lane_supplied(): + params = extract_hyperparams({"temperature": 0.8}, lane="agent", defaults={"temperature": 0.25, "max_tokens": 2048}) + assert params == {"temperature": 0.8, "max_tokens": 2048} + + +class TestTokenFloor: + """The configured budget may raise the floor; it may never lower it.""" + + def test_a_roomier_budget_is_kept(self): + assert extract_hyperparams({"max_tokens": 16384}, token_floor=8192)["max_tokens"] == 16384 + + def test_a_short_reply_budget_is_raised_to_the_floor(self): + # 600 tokens is a normal setting for brief prose. Sending it to a call whose + # whole answer must fit truncates the answer, which reaches the user as the + # pass doing nothing rather than as the shorter reply they asked for. + assert extract_hyperparams({"max_tokens": 600}, token_floor=8192)["max_tokens"] == 8192 + + def test_a_missing_budget_becomes_the_floor(self): + assert extract_hyperparams({}, token_floor=2048)["max_tokens"] == 2048 + + def test_the_floor_outranks_a_default(self): + params = extract_hyperparams({}, token_floor=8192, defaults={"max_tokens": 512}) + assert params["max_tokens"] == 8192 + + def test_no_floor_leaves_the_budget_alone(self): + # Prose passes stream to a stop token, so a short preset is honored there. + assert extract_hyperparams({"max_tokens": 600}) == {"max_tokens": 600} + + def test_the_floor_reads_the_agent_lane(self): + settings = {"max_tokens": 600, "agent_max_tokens": 32768} + assert extract_hyperparams(settings, lane="agent", token_floor=8192)["max_tokens"] == 32768 + + +@pytest.mark.parametrize( + ("settings", "floor", "expected"), + [ + ({"agent_max_tokens": 32768, "max_tokens": 600}, 8192, 32768), + ({"max_tokens": 600}, 8192, 8192), + ({}, 4096, 4096), + ], +) +def test_agent_lane_max_tokens_is_the_budget_alone(settings, floor, expected): + assert agent_lane_max_tokens(settings, floor=floor) == expected diff --git a/tests/unit/workflows/test_forced_call.py b/tests/unit/workflows/test_forced_call.py index 248c867c..dd154201 100644 --- a/tests/unit/workflows/test_forced_call.py +++ b/tests/unit/workflows/test_forced_call.py @@ -166,6 +166,43 @@ async def test_pass_id_none_suppresses_reasoning_deltas(self): assert out == [{"type": "result", "args": {"rewritten_text": "x"}}] +class TestTokenBudget: + """`token_floor` is the call's requirement; the endpoint's config may raise it.""" + + async def _sent_budget(self, settings: dict, floor: int = 4096) -> int: + client = _FakeClient([_done_event_with_tool_call(_TOOL_NAME, {"rewritten_text": "x"})]) + await _collect( + forced_tool_call( + client=client, + prefix=[], + tail_messages=[], + tool_name=_TOOL_NAME, + settings=settings, + token_floor=floor, + ) + ) + assert client.complete_kwargs is not None + return client.complete_kwargs["max_tokens"] + + async def test_a_roomier_endpoint_budget_is_the_one_sent(self): + assert await self._sent_budget({**_SETTINGS, "max_tokens": 16384}) == 16384 + + async def test_a_short_reply_preset_never_shrinks_the_call(self): + # 600 tokens is a normal setting for brief prose replies. Honoring it here + # would truncate the tool call mid-arguments, which reaches the user as the + # workflow failing rather than as the shorter reply they asked for. + assert await self._sent_budget({**_SETTINGS, "max_tokens": 600}) == 4096 + + async def test_the_agent_lanes_own_budget_wins_when_it_resolves(self): + # Present only when a separate agent endpoint overlaid its model config; + # the forced call runs on that lane, so its budget outranks the writer's. + settings = {**_SETTINGS, "max_tokens": 600, "agent_max_tokens": 32768} + assert await self._sent_budget(settings) == 32768 + + async def test_settings_without_a_budget_fall_back_to_the_floor(self): + assert await self._sent_budget(_SETTINGS) == 4096 + + class TestToolsAssembly: async def test_enabled_tools_none_single_schema(self): client = _FakeClient([_done_event_with_tool_call(_TOOL_NAME, {})]) From f31b9d4c4eb964bba631c3c7c025c296e39d3a10 Mon Sep 17 00:00:00 2001 From: Chi Date: Sun, 6 Sep 2026 19:39:28 +0700 Subject: [PATCH 4/6] refactor inference layer, new prompting layer --- AGENTS.md | 10 +- backend/api/routes/conversations.py | 16 +- backend/api/routes/messages.py | 5 +- backend/api/routes/settings.py | 4 +- backend/core/domain_types.py | 2 +- backend/database/queries/worlds.py | 4 +- backend/features/autocomplete/__init__.py | 5 + backend/features/autocomplete/service.py | 55 ++ backend/features/cards/public_profile.py | 8 +- backend/features/cards/sheet_update.py | 7 +- backend/features/documents/audit.py | 8 +- backend/features/lorebook/__init__.py | 4 +- backend/features/lorebook/enablement.py | 20 + backend/features/lorebook/proposals.py | 2 +- backend/features/summarization/summarizer.py | 5 +- backend/inference/__init__.py | 94 +-- backend/inference/anthropic.py | 2 +- backend/inference/client.py | 2 +- backend/inference/local_ml.py | 102 +-- backend/inference/prompt_builder.py | 710 ------------------ backend/inference/schema.py | 28 + backend/pipeline/config.py | 4 +- backend/pipeline/context.py | 13 +- backend/pipeline/entrypoints.py | 3 +- backend/pipeline/passes/_prompting.py | 28 + .../passes/director/direction_note.py | 8 +- .../passes/director/direction_note_prompts.py | 59 ++ backend/pipeline/passes/director/director.py | 25 +- .../passes/director/lorebook_select.py | 30 +- backend/pipeline/passes/director/prompts.py | 119 +++ backend/pipeline/passes/editor/editor.py | 9 +- backend/pipeline/passes/editor/feedback.py | 5 +- backend/pipeline/passes/editor/prompts.py | 100 +++ backend/pipeline/passes/world_change.py | 8 +- .../pipeline/passes/world_change_prompt.py | 52 ++ backend/pipeline/passes/writer.py | 3 +- backend/pipeline/state.py | 6 +- backend/pipeline/tools.py | 3 + backend/pipeline/workflow_bridge.py | 5 +- backend/pipeline/world_proposal.py | 2 +- backend/prompting/__init__.py | 31 + backend/prompting/base.py | 144 ++++ .../{inference => prompting}/group_context.py | 0 backend/{inference => prompting}/lorebook.py | 63 +- backend/prompting/scene_direction.py | 96 +++ backend/prompting/tool_catalog.py | 173 +++++ .../tool_schemas.py} | 154 +--- backend/workflows/_forced_call.py | 13 +- backend/workflows/attachment_cache.py | 2 +- backend/workflows/contracts.py | 2 +- backend/workflows/registry.py | 8 +- backend/workflows/toolkit.py | 10 +- docs/architecture/dynamic-worlds.md | 2 +- docs/architecture/group-chats.md | 2 +- docs/architecture/kv-cache.md | 10 + docs/architecture/prompting.md | 58 ++ docs/architecture/secondary-workflow.md | 6 + docs/plans/inference-layer-audit.md | 370 --------- scripts/check_backend_layers.py | 122 +-- tests/integration/_llm_mock.py | 2 +- tests/integration/test_autocomplete.py | 8 +- tests/integration/test_characters.py | 2 +- .../test_endpoint_transport_passes.py | 57 ++ tests/integration/test_group_chats.py | 4 +- tests/integration/test_settings.py | 2 +- tests/integration/workflows/_fixtures.py | 27 +- tests/unit/test_abort_pipeline.py | 8 +- tests/unit/test_agentic_lorebook.py | 25 +- tests/unit/test_autocomplete.py | 4 +- tests/unit/test_card_v3.py | 2 +- tests/unit/test_director_per_fragment.py | 8 +- tests/unit/test_document_audit.py | 2 +- tests/unit/test_dynamic_worlds.py | 6 +- tests/unit/test_editor_abort.py | 3 +- tests/unit/test_editor_draft_update.py | 4 +- tests/unit/test_group_context_modes.py | 6 +- tests/unit/test_group_prompt.py | 2 +- tests/unit/test_import_layering.py | 191 ++--- tests/unit/test_interactive_fragments.py | 19 +- tests/unit/test_kv_cache_invariants.py | 15 +- tests/unit/test_lorebook_matching.py | 2 +- tests/unit/test_offer_tools_blob.py | 4 +- tests/unit/test_prompt_builder_extras.py | 2 +- tests/unit/test_public_profile_draft.py | 2 +- tests/unit/test_reasoning_breaks.py | 3 +- tests/unit/test_structured_tool_calls.py | 3 +- tests/unit/test_super_regen_audit_context.py | 3 +- tests/unit/test_tool_catalog.py | 156 ++++ tests/unit/test_tool_registry.py | 137 ---- tests/unit/workflows/test_forced_call.py | 7 +- tests/unit/workflows/test_registry.py | 17 +- 91 files changed, 1600 insertions(+), 1974 deletions(-) create mode 100644 backend/features/autocomplete/__init__.py create mode 100644 backend/features/autocomplete/service.py create mode 100644 backend/features/lorebook/enablement.py delete mode 100644 backend/inference/prompt_builder.py create mode 100644 backend/inference/schema.py create mode 100644 backend/pipeline/passes/_prompting.py create mode 100644 backend/pipeline/passes/director/direction_note_prompts.py create mode 100644 backend/pipeline/passes/director/prompts.py create mode 100644 backend/pipeline/passes/editor/prompts.py create mode 100644 backend/pipeline/passes/world_change_prompt.py create mode 100644 backend/pipeline/tools.py create mode 100644 backend/prompting/__init__.py create mode 100644 backend/prompting/base.py rename backend/{inference => prompting}/group_context.py (100%) rename backend/{inference => prompting}/lorebook.py (90%) create mode 100644 backend/prompting/scene_direction.py create mode 100644 backend/prompting/tool_catalog.py rename backend/{inference/tool_registry.py => prompting/tool_schemas.py} (66%) create mode 100644 docs/architecture/prompting.md delete mode 100644 docs/plans/inference-layer-audit.md create mode 100644 tests/unit/test_tool_catalog.py delete mode 100644 tests/unit/test_tool_registry.py diff --git a/AGENTS.md b/AGENTS.md index ca62ab97..30bddc8b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -8,18 +8,20 @@ Orb is an agentic roleplay and writing application with a Python/FastAPI backend ## Backend layout -The backend is split into layers. Dependencies point downward only: +The backend is split into layers with explicit allowed dependency edges: 1. `api/` — HTTP routes and schemas 2. `pipeline/` — conversation turn orchestration -3. `features/` — self-contained application features -4. `workflows/` — pluggable secondary workflows -5. `inference/` and `analysis/` — model access, prompt assembly, and pure text analysis +3. `features/` and `workflows/` — sibling product composition inputs +4. `prompting/` — deterministic, provider-independent model-facing construction +5. `inference/` and `analysis/` — model execution and pure text analysis 6. `database/` — schema, migrations, queries, and row models 7. `core/` — dependency-free shared utilities and types Lower layers must not import higher layers or peer feature slices. Use dependency inversion when a lower layer needs higher-layer behavior. Keep pure logic separate from integration code and persistence. +See `docs/architecture/prompting.md` for the exact allowed-edge matrix, the prompting ownership test, and prompt/tool ordering contracts. `scripts/check_backend_layers.py` is the executable source of truth for dependency edges. + Feature slices should expose a small facade, keep local contracts near their logic, and persist through the database layer rather than reaching into unrelated features. Before changing prompt assembly, pass ordering, tool schemas, or streaming behavior, read the relevant document in `docs/architecture/`. diff --git a/backend/api/routes/conversations.py b/backend/api/routes/conversations.py index db32de47..d4d4817d 100644 --- a/backend/api/routes/conversations.py +++ b/backend/api/routes/conversations.py @@ -76,9 +76,6 @@ AbortToken, agent_lane_from_settings, client_from_settings, - group_context, - macro_identity, - prompt_builder, ) from ...pipeline import ( agent_enabled, @@ -86,6 +83,13 @@ persona_macros, resolve_card_and_persona, ) +from ...prompting import ( + compute_style_injection_block, + group_context, + group_speaker_label, + macro_identity, + resolve_mood_fragment_randoms, +) from ..deps import ( _active_aborts, _CleanupStreamingResponse, @@ -729,7 +733,7 @@ async def api_get_context_size(cid: str, conv: ConversationRow = Depends(require for message in messages: if message.get("role") != "assistant": continue - label = prompt_builder.group_speaker_label(names, message.get("speaker_member_id")) + label = group_speaker_label(names, message.get("speaker_member_id")) msg_chars += len(f"{label}: ") # Director injection — fragment {{random}} resolves against a throwaway @@ -737,8 +741,8 @@ async def api_get_context_size(cid: str, conv: ConversationRow = Depends(require # real turn would inject, without recording new picks. active_moods = director.get("active_moods", []) if director else [] est_choices = dict(director.get("macro_choices", {}) if director else {}) - est_mood_frags = prompt_builder.resolve_mood_fragment_randoms(mood_frags, active_moods, est_choices) - inj_block = prompt_builder.compute_style_injection_block( + est_mood_frags = resolve_mood_fragment_randoms(mood_frags, active_moods, est_choices) + inj_block = compute_style_injection_block( active_moods, active_moods, est_mood_frags, diff --git a/backend/api/routes/messages.py b/backend/api/routes/messages.py index 323789ad..4d87df19 100644 --- a/backend/api/routes/messages.py +++ b/backend/api/routes/messages.py @@ -32,6 +32,7 @@ update_message_content, ) from ...database.models import ConversationRow +from ...features import autocomplete from ...features.prose_rewriter import ( ProseRewriteConfig, resolve_config, @@ -496,7 +497,7 @@ async def api_autocomplete( for m in messages[-4:] ] summary = macros.resolve_prompt(summary_source) - prompt = local_ml.build_prompt(char_name, user_name, summary, recent, macros.resolve_prompt(data.draft)) + prompt = autocomplete.build_prompt(char_name, user_name, summary, recent, macros.resolve_prompt(data.draft)) - completion = await local_ml.complete(prompt) + completion = await autocomplete.complete(prompt) return {"completion": completion} diff --git a/backend/api/routes/settings.py b/backend/api/routes/settings.py index 9f021966..fef519b5 100644 --- a/backend/api/routes/settings.py +++ b/backend/api/routes/settings.py @@ -5,7 +5,7 @@ from fastapi import APIRouter, HTTPException from ...database import get_settings, reset_to_defaults, update_settings -from ...inference import TOOLS +from ...prompting.tool_catalog import has_tool from ..schemas import ResetConfirm, SettingsUpdate router = APIRouter() @@ -22,7 +22,7 @@ async def api_update_settings(data: SettingsUpdate): # enabled_tools holds only model-callable tools. Drop any key that is not a # registered tool so non-tool feature flags can never be persisted into it. if isinstance(payload.get("enabled_tools"), dict): - payload["enabled_tools"] = {k: v for k, v in payload["enabled_tools"].items() if k in TOOLS} + payload["enabled_tools"] = {k: v for k, v in payload["enabled_tools"].items() if has_tool(k)} return await update_settings(payload) diff --git a/backend/core/domain_types.py b/backend/core/domain_types.py index d62cf6c3..14ed0c34 100644 --- a/backend/core/domain_types.py +++ b/backend/core/domain_types.py @@ -10,7 +10,7 @@ # Which character information every group generation carries. Stored on # ``conversations.group_context_mode``; the projection each value selects lives -# in ``inference/group_context.py`` and nowhere else. +# in ``prompting/group_context.py`` and nowhere else. GroupContextMode: TypeAlias = Literal["private", "shared", "swap"] diff --git a/backend/database/queries/worlds.py b/backend/database/queries/worlds.py index f26afeef..bb9a730d 100644 --- a/backend/database/queries/worlds.py +++ b/backend/database/queries/worlds.py @@ -270,7 +270,7 @@ async def get_lorebook_entries(world_id: str) -> list[LorebookEntryRow]: """Every row in the World, both layers, archived included. The drawer labels the layers and shows archived overlay rows, and the - projection (``inference.lorebook.select_effective_entries``) filters from + projection (``prompting.lorebook.select_effective_entries``) filters from this same superset -- so this reader stays deliberately unfiltered. """ async with get_db() as db: @@ -395,7 +395,7 @@ async def get_active_lorebook_entries() -> list[ActiveLorebookEntryRow]: This is the raw overlay pool, not the effective lore: an authored entry hidden by a replacement is still in here, and so is the suppression marker - that hides it. Resolving that is ``inference.lorebook`` -- the projection + that hides it. Resolving that is ``prompting.lorebook`` -- the projection lives in the lore layer, which ``database/`` sits below and may not import. """ async with get_db() as db: diff --git a/backend/features/autocomplete/__init__.py b/backend/features/autocomplete/__init__.py new file mode 100644 index 00000000..adf077a4 --- /dev/null +++ b/backend/features/autocomplete/__init__.py @@ -0,0 +1,5 @@ +"""Autocomplete prompt construction and completion adaptation.""" + +from .service import build_prompt, complete + +__all__ = ["build_prompt", "complete"] diff --git a/backend/features/autocomplete/service.py b/backend/features/autocomplete/service.py new file mode 100644 index 00000000..f774d29c --- /dev/null +++ b/backend/features/autocomplete/service.py @@ -0,0 +1,55 @@ +"""Feature-specific prompt and output handling for local autocomplete.""" + +from __future__ import annotations + +from collections.abc import Mapping, Sequence + +from ...inference import local_ml + + +async def complete( + prompt: str, + n_predict: int = 12, + stop: Sequence[str] = ("\n",), + temperature: float = 0.25, +) -> str: + """Autocomplete continuation over the generic local-model runtime. + + The typeahead model produces garbage for a whitespace-ending prompt, so + trim that tail before inference. If whitespace was removed, also remove the + model's re-emitted leading separator because the frontend appends the result + to the original, untrimmed draft. + """ + trimmed = prompt.rstrip() + completion = await local_ml.acomplete("autocomplete", trimmed, n_predict, stop, temperature) + return completion.lstrip() if trimmed != prompt else completion + + +def build_prompt( + char_name: str, + user_name: str, + char_summary: str, + recent: Sequence[Mapping[str, str]], + draft: str, + *, + max_msg_chars: int = 500, + max_summary_chars: int = 400, +) -> str: + """Assemble a short raw-continuation prompt ending at the user's draft. + + *recent* is oldest-to-newest and may carry a ``name`` that labels a group + speaker instead of using *char_name*. The lightweight prompt deliberately + excludes Director and pipeline injection content. + """ + lines: list[str] = [] + summary = (char_summary or "").strip() + if summary: + lines.append(summary[:max_summary_chars]) + lines.append("***Roleplay chat below***") + for message in recent: + name = (message.get("name") or "").strip() or (user_name if message.get("role") == "user" else char_name) + content = (message.get("content") or "").strip()[-max_msg_chars:] + if content: + lines.append(f"{name}: {content}") + lines.append(f"{user_name}: {draft}") + return "\n".join(lines) diff --git a/backend/features/cards/public_profile.py b/backend/features/cards/public_profile.py index 2731ef0b..dbdc6f0d 100644 --- a/backend/features/cards/public_profile.py +++ b/backend/features/cards/public_profile.py @@ -15,7 +15,7 @@ # output contract below has something to be the enforcement of. # # Braces are on the list because a profile is macro-resolved at turn time -# (``inference/group_context._render_public_cast``): a generated ``{{user}}`` +# (``prompting/group_context._render_public_cast``): a generated ``{{user}}`` # would quietly substitute months later, in a string the user already reviewed # and approved. PROFILE_FLOOR = ( @@ -43,10 +43,8 @@ "a character's card fields or display name. Call the requested tool." ) -# Deliberately not registered in ``inference.tool_registry.TOOLS``: that module -# asserts ``PRE_WRITER_TOOLS | POST_WRITER_TOOLS == BUILTIN_TOOL_NAMES`` at -# import, so registering here would force a turn-phase partition onto a tool that -# has nothing to do with a turn. +# Deliberately not registered in ``prompting.tool_catalog``: this card-only +# schema is a one-shot contract, not part of the stable pipeline tool blob. DRAFT_PROFILE_TOOL = { "type": "function", "function": { diff --git a/backend/features/cards/sheet_update.py b/backend/features/cards/sheet_update.py index 5d13fcf3..3b7a1f64 100644 --- a/backend/features/cards/sheet_update.py +++ b/backend/features/cards/sheet_update.py @@ -27,10 +27,9 @@ "character's sheet or display name. Call the requested tool, reporting no change when there is none." ) -# Deliberately not registered in ``inference.tool_registry.TOOLS``, for the same -# reason ``DRAFT_PROFILE_TOOL`` is not: that module partitions its tools by turn -# phase, and this call is bookkeeping about a finished exchange rather than a phase -# of one. +# Deliberately not registered in ``prompting.tool_catalog``, for the same reason +# ``DRAFT_PROFILE_TOOL`` is not: this call is bookkeeping about a finished +# exchange, not part of the stable pipeline tool blob. UPDATE_SHEET_TOOL = { "type": "function", "function": { diff --git a/backend/features/documents/audit.py b/backend/features/documents/audit.py index 8d923d64..573c631e 100644 --- a/backend/features/documents/audit.py +++ b/backend/features/documents/audit.py @@ -23,7 +23,8 @@ sentence_boundary_ends, ) from ...core import ChatMessage, extract_hyperparams -from ...inference import TOOLS, LLMClient, parse_tool_calls, reasoning_cfg +from ...inference import LLMClient, parse_tool_calls, reasoning_cfg +from ...prompting.tool_catalog import require_tool from .continuation import _MACRO_RE, build_generation_messages if TYPE_CHECKING: @@ -270,7 +271,8 @@ async def patch_document( # the agent-lane forced calls: the whole patch set has to fit in one reply, and # a document preset kept short for brief continuations would truncate it. params = extract_hyperparams(settings, token_floor=8192, defaults={"temperature": 0.25}) - schema = TOOLS["editor_apply_patch"]["schema"] + editor_patch = require_tool("editor_apply_patch") + schema = editor_patch["schema"] if client.completion_mode == "text": # Both text shapes byte-extend the generation prompt as a raw # continuation: verbatim document (raw) or the re-run /apply-template @@ -295,7 +297,7 @@ async def patch_document( messages, model, tools=[schema], - tool_choice=TOOLS["editor_apply_patch"]["choice"], + tool_choice=editor_patch["choice"], tools_in_prompt=False, **params, **reasoning_cfg(False), diff --git a/backend/features/lorebook/__init__.py b/backend/features/lorebook/__init__.py index 74a2ef96..58c92e9f 100644 --- a/backend/features/lorebook/__init__.py +++ b/backend/features/lorebook/__init__.py @@ -2,11 +2,10 @@ from __future__ import annotations -from ...inference.lorebook import ( +from ...prompting.lorebook import ( AGENTIC_LOREBOOK_SCAN_DEPTH, DYNAMIC_SECTION_TITLE, LOREBOOK_SCAN_DEPTH, - agentic_lorebook_active, build_lorebook_catalog, compute_agentic_lorebook_block, compute_constant_lorebook_block, @@ -29,6 +28,7 @@ stage_proposal, undo_changeset, ) +from .enablement import agentic_lorebook_active from .proposals import ( ValidatedProposal, build_world_change_catalog, diff --git a/backend/features/lorebook/enablement.py b/backend/features/lorebook/enablement.py new file mode 100644 index 00000000..00ecf1b2 --- /dev/null +++ b/backend/features/lorebook/enablement.py @@ -0,0 +1,20 @@ +"""Lorebook feature enablement decisions.""" + +from __future__ import annotations + +from collections.abc import Mapping, Sequence +from typing import Any + + +def agentic_lorebook_active( + settings: Mapping[str, Any], + lorebook_entries: Sequence[Mapping[str, Any]], + *, + agent_on: bool, +) -> bool: + """Return whether the Director should pick lorebook entries this turn.""" + if not bool(settings.get("agentic_lorebook_enabled", 0)): + return False + if not agent_on: + return False + return any(not entry.get("constant") for entry in lorebook_entries) diff --git a/backend/features/lorebook/proposals.py b/backend/features/lorebook/proposals.py index db2ae1ba..24ffb3f3 100644 --- a/backend/features/lorebook/proposals.py +++ b/backend/features/lorebook/proposals.py @@ -7,7 +7,7 @@ from dataclasses import dataclass, field from typing import Any, NamedTuple -from ...inference.lorebook import ( +from ...prompting.lorebook import ( DYNAMIC_SECTION_TITLE, is_dynamic, select_effective_entries, diff --git a/backend/features/summarization/summarizer.py b/backend/features/summarization/summarizer.py index 3a6e6a57..95fe3c3a 100644 --- a/backend/features/summarization/summarizer.py +++ b/backend/features/summarization/summarizer.py @@ -4,7 +4,8 @@ from typing import Any from ...core import ChatMessage, Macros, TurnCast, extract_hyperparams -from ...inference import LLMClient, prompt_builder +from ...inference import LLMClient +from ...prompting import build_prefix DEFAULT_SUMMARY_INSTRUCTIONS = ( "[OOC: Write a rich prose narrative summary of the story so far. " @@ -34,7 +35,7 @@ def build_messages( cast: TurnCast | None = None, speaker_names: Mapping[str, str] | None = None, ) -> list[ChatMessage]: - prefix = prompt_builder.build_prefix( + prefix = build_prefix( system_prompt, char_persona, char_scenario, diff --git a/backend/inference/__init__.py b/backend/inference/__init__.py index a34f8af4..4ac3b5f1 100644 --- a/backend/inference/__init__.py +++ b/backend/inference/__init__.py @@ -1,4 +1,4 @@ -"""LLM transport and prompt/tool assembly.""" +"""Model execution, provider adaptation, retries, and cache mechanics.""" from __future__ import annotations @@ -21,57 +21,9 @@ profile_for, ) from .errors import LLMCallError, provider_sentence, redact -from .group_context import ( - context_size_components, - macro_identity, - member_macros, - prefix_is_speaker_scoped, - render_cast_section, - roster_names, - tail_carries_identity, -) from .kv_tracker import _KVCacheTracker -from .lorebook import ( - DYNAMIC_SECTION_TITLE, - compute_constant_lorebook_block, - compute_depth_lorebook_block, - is_dynamic, - select_effective_entries, -) -from .prompt_builder import ( - EDITOR_RENUMBER_NOTICE, - build_direction_note_prompt, - build_director_scene_step_prompt, - build_director_tool_prompt, - build_editor_prompt, - build_feedback_prompt, - build_lorebook_select_prompt, - build_prefix, - build_style_injection, - build_world_change_prompt, - compute_style_injection_block, - format_message_with_attachments, - render_direction_notes_block, - resolve_mood_fragment_randoms, -) from .retry import RetryPolicy from .text_completion import has_image_parts -from .tool_registry import ( - BUILTIN_TOOL_NAMES, - GIVE_FEEDBACK_CHOICE, - POST_WRITER_TOOLS, - PRE_WRITER_TOOLS, - PROPOSE_WORLD_CHANGES_CHOICE, - RECORD_DIRECTION_NOTE_CHOICE, - SELECT_LOREBOOK_CHOICE, - STANDALONE_TOOLS, - TOOLS, - build_direct_scene_tool, - build_direction_note_tool, - build_feedback_tool, - enabled_schemas, - register_tool, -) __all__ = [ # client — LLM transport @@ -98,50 +50,6 @@ # cached_call / kv_tracker "CachedBase", "_KVCacheTracker", - # group_context — the one owner of per-mode character-field visibility - "context_size_components", - "macro_identity", - "member_macros", - "prefix_is_speaker_scoped", - "render_cast_section", - "roster_names", - "tail_carries_identity", # text_completion "has_image_parts", - # lorebook — full surface via .lorebook / features.lorebook facade - "DYNAMIC_SECTION_TITLE", - "compute_constant_lorebook_block", - "compute_depth_lorebook_block", - "is_dynamic", - "select_effective_entries", - # prompt_builder - "build_director_scene_step_prompt", - "build_director_tool_prompt", - "EDITOR_RENUMBER_NOTICE", - "build_editor_prompt", - "build_feedback_prompt", - "build_lorebook_select_prompt", - "build_direction_note_prompt", - "build_prefix", - "build_style_injection", - "build_world_change_prompt", - "compute_style_injection_block", - "format_message_with_attachments", - "render_direction_notes_block", - "resolve_mood_fragment_randoms", - # tool_registry - "BUILTIN_TOOL_NAMES", - "GIVE_FEEDBACK_CHOICE", - "POST_WRITER_TOOLS", - "PRE_WRITER_TOOLS", - "RECORD_DIRECTION_NOTE_CHOICE", - "PROPOSE_WORLD_CHANGES_CHOICE", - "SELECT_LOREBOOK_CHOICE", - "STANDALONE_TOOLS", - "TOOLS", - "build_direct_scene_tool", - "build_feedback_tool", - "build_direction_note_tool", - "enabled_schemas", - "register_tool", ] diff --git a/backend/inference/anthropic.py b/backend/inference/anthropic.py index c2f54c5e..86e47a24 100644 --- a/backend/inference/anthropic.py +++ b/backend/inference/anthropic.py @@ -7,7 +7,7 @@ from collections.abc import Mapping, Sequence from typing import Any -from .tool_registry import strictify_schema +from .schema import strictify_schema # Anthropic rejects unknown top-level fields. These are the only user-provided # extra_body keys accepted on a native Messages route; OpenAI-shaped escape diff --git a/backend/inference/client.py b/backend/inference/client.py index a19c7864..01bf7ccd 100644 --- a/backend/inference/client.py +++ b/backend/inference/client.py @@ -13,7 +13,7 @@ from .errors import LLMCallError, llm_call_error, llm_stream_error from .gemma_tool_format import parse_gemma_tool_calls from .retry import RetryPolicy -from .tool_registry import strictify_schema +from .schema import strictify_schema logger = logging.getLogger(__name__) diff --git a/backend/inference/local_ml.py b/backend/inference/local_ml.py index edcfcaf3..609469c4 100644 --- a/backend/inference/local_ml.py +++ b/backend/inference/local_ml.py @@ -6,7 +6,7 @@ import atexit import math import os -from collections.abc import Mapping, Sequence +from collections.abc import Sequence from typing import Any from ..core.text_segmentation import remove_quoted_spans, split_sentences @@ -46,8 +46,6 @@ "aclassify_pov", "ascore", "available", - "build_prompt", - "complete", "delete_model", "deps_ok", "download", @@ -192,27 +190,6 @@ async def acomplete( return await asyncio.to_thread(_complete_blocking, feature, prompt, n_predict, stop, temperature) -async def complete( - prompt: str, - n_predict: int = 12, - stop: Sequence[str] = ("\n",), - temperature: float = 0.25, -) -> str: - """Autocomplete continuation over ``acomplete('autocomplete', ...)``. - - Works around a tokenization quirk of the typeahead model: a prompt ending in - whitespace generates garbage ("I hold up both " fails where "I hold up both" - works). rstrip the prompt before generating; build_prompt guarantees the only - trailing whitespace is the user's draft tail, so this trims exactly the draft. - When we did trim, the user already typed the word separator, so lstrip the - model's re-emitted leading space back off — the frontend appends the completion - to the untrimmed draft, and "...both " + " hands" would double the space. - """ - trimmed = prompt.rstrip() - completion = await acomplete("autocomplete", trimmed, n_predict, stop, temperature) - return completion.lstrip() if trimmed != prompt else completion - - # A separate Llama mode from generation: the GGUF carries a 2-class head, scored # with RANK pooling. `embed()` then returns a buffer whose first two floats are # the class logits (rest is uninitialized) — softmax them, class 1 is "slop". @@ -363,80 +340,3 @@ async def aclassify_pov(text: str) -> str: """ async with _lock("pov_classifier"): return await asyncio.to_thread(_classify_pov_blocking, "pov_classifier", text) - - -def build_prompt( - char_name: str, - user_name: str, - char_summary: str, - recent: Sequence[Mapping[str, str]], - draft: str, - *, - max_msg_chars: int = 500, - max_summary_chars: int = 400, -) -> str: - """Assemble a short raw-continuation prompt ending at the user's draft. - - *recent* is oldest→newest ``{"role": "user"|"assistant", "content": str}``, - each entry optionally carrying a ``"name"`` that labels that line instead of - *char_name* — how a group scene names the member who actually spoke, since - there every reply would otherwise be attributed to the scene itself. - Deliberately excludes the Director/pipeline injection block — this is a - lightweight typeahead, not a full turn. The model continues the final line. - """ - lines: list[str] = [] - summary = (char_summary or "").strip() - if summary: - lines.append(summary[:max_summary_chars]) - lines.append("***Roleplay chat below***") - for m in recent: - name = (m.get("name") or "").strip() or (user_name if m.get("role") == "user" else char_name) - content = (m.get("content") or "").strip()[ - -max_msg_chars: - ] # keep the tail: typeahead reacts to the latest action, which is at the END of the message - if content: - lines.append(f"{name}: {content}") - # No trailing newline: the model continues this exact line. - lines.append(f"{user_name}: {draft}") - return "\n".join(lines) - - -if __name__ == "__main__": - # Self-check for the pure trimmer (no model needed). - p = build_prompt( - "Aria", - "Sam", - "Aria is a wry tavern keeper.", - [{"role": "assistant", "content": "You look lost."}, {"role": "user", "content": "Maybe I am."}], - "I walk into the", - ) - assert p.endswith("Sam: I walk into the"), p - assert "Aria: You look lost." in p - assert "Aria is a wry tavern keeper." in p - assert "Director" not in p and "Scene Direction" not in p - print("build_prompt OK") - - # Self-check for the povtense grid layout (no model needed). One hot cell per - # case, placed row-major: index = row * 3 + tense column. - for row, label in enumerate(POV_ROWS): - for col in range(_POV_TENSES): - grid = [0.0] * (len(POV_ROWS) * _POV_TENSES) - grid[row * _POV_TENSES + col] = 9.0 - assert pov_from_logits(grid) == label, (row, col, label) - # A POV spread across all three tenses still beats a single taller cell in another row. - spread = [0.0] * 12 - spread[6] = spread[7] = spread[8] = 2.0 # "third", split across tenses - spread[0] = 3.0 # "first", one tense only - assert pov_from_logits(spread) == "third", spread - print("pov_from_logits OK") - - # Self-check for what the POV model is actually shown (no model needed). - reply = 'She turned. "I will go," she said. He waited by the door. Rain hit the glass.' - shaped = pov_input(reply) - assert "I will go" not in shaped, shaped # dialogue is not narration - assert shaped.endswith("Rain hit the glass."), shaped # tail-anchored - assert len(split_sentences(shaped)) <= _POV_SENTENCES, shaped - assert pov_input('"All of it." "Every word."') == "" # all dialogue -> caller walks back - assert pov_input("") == "" and pov_input(" ") == "" - assert pov_input("no terminal punctuation here") == "no terminal punctuation here" - print("pov_input OK") diff --git a/backend/inference/prompt_builder.py b/backend/inference/prompt_builder.py deleted file mode 100644 index cb6c2ed8..00000000 --- a/backend/inference/prompt_builder.py +++ /dev/null @@ -1,710 +0,0 @@ -"""Build prompt blocks and tool-call messages for pipeline passes.""" - -from __future__ import annotations - -from collections.abc import Collection, Mapping, MutableMapping, Sequence -from typing import Any - -from ..core import ChatMessage, ContentPart, Macros, TurnCast, resolve_stored_random -from .group_context import render_cast_section -from .tool_registry import TOOLS - - -def format_message_with_attachments(message: Mapping[str, Any], macros: Macros | None) -> ChatMessage: - """Convert a message dict to OpenAI chat format, embedding attachments. - - Two attachment lists on the message dict are handled differently: - - ``user_attachments``: embedded as multimodal ``image_url`` parts in - the message content. - - ``workflow_attachments``: their raw bytes never enter the prefix; only - the ``annotation`` of root rows (``parent_attachment_id IS NULL``) is - appended as text. Sibling variants and blank annotations contribute nothing. - - Returns ``{"role": ..., "content": str | list}``. - """ - role = message["role"] - raw = message.get("content", "") - text = macros.resolve_prompt(raw) if macros else raw - - user_atts: list[dict] = list(message.get("user_attachments") or []) - workflow_annotations: list[str] = [] - for att in message.get("workflow_attachments") or []: - if att.get("parent_attachment_id") is not None: - continue - annot = att.get("annotation") - if isinstance(annot, str) and annot.strip(): - workflow_annotations.append(annot) - - text_parts = [text] if text else [] - text_parts.extend(workflow_annotations) - combined_text = "\n\n".join(text_parts) - - if not user_atts: - return {"role": role, "content": combined_text} - - parts: list[ContentPart] = [] - if combined_text: - parts.append({"type": "text", "text": combined_text}) - for att in user_atts: - mime = att["mime_type"] - b64 = att["data_b64"] - url = f"data:{mime};base64,{b64}" - parts.append({"type": "image_url", "image_url": {"url": url}}) - return {"role": role, "content": parts} - - -def group_speaker_label(speaker_names: Mapping[str, str], speaker_member_id: object) -> str: - """The name a group prefix attributes one assistant row to. - - An assistant row with no speaker is a summary the compressor wrote, not a - member's line; a speaker whose member row is gone entirely is still named - rather than silently merged into the reply above it. The context-size - estimator bills the same string, so the two read it from here. - """ - if not speaker_member_id: - return "Summary" - return speaker_names.get(str(speaker_member_id), "Unknown speaker") - - -# ── System-prompt prefix - - -def build_prefix( - system_prompt: str, - char_persona: str, - char_scenario: str, - mes_example: str = "", - post_history_instructions: str = "", - messages: Sequence[Mapping[str, Any]] | None = None, - macros: Macros | None = None, - user_description: str = "", - *, - constant_lorebook_block: str = "", - extra_system_blocks: list[str] | None = None, - cast: TurnCast | None = None, - speaker_names: Mapping[str, str] | None = None, -) -> list[ChatMessage]: - resolve = macros.resolve_message if macros else (lambda t: t) - resolved = { - key: resolve(val) - for key, val in { - "persona": char_persona, - "scenario": char_scenario, - "mes_example": mes_example, - "post_history": post_history_instructions, - "user_desc": user_description, - }.items() - } - - parts = [system_prompt] - if cast and cast.grouped: - # Which character fields land here is the context mode's call alone — - # public cast, shared dossiers, or the active card. See - # ``inference/group_context.py``; no pass decides this for itself. - parts.append(render_cast_section(cast, macros)) - elif macros and macros.char: - parts.append(f"\n\n## Character: {macros.char}") - if resolved["persona"] and not (cast and cast.grouped): - parts.append(f"\n{resolved['persona']}") - # Opaque, pre-rendered (header + macros already resolved by the caller) — - # not passed through resolve, to avoid double macro expansion. - if constant_lorebook_block: - parts.append(f"\n\n{constant_lorebook_block}") - if resolved["scenario"]: - parts.append(f"\n\n## Scenario\n{resolved['scenario']}") - if resolved["mes_example"] and not (cast and cast.grouped): - mes = resolved["mes_example"] - if "" in mes: - processed_example = mes.replace("", "## Example Dialogue") - parts.append(f"\n\n{processed_example}") - else: - parts.append(f"\n\n## Example Dialogue\n{mes}") - # Kept for a group as well: this is the *scene's* single directive - # (``conversations.post_history_instructions``), not a card's. There is - # exactly one per scene, it is identical for every speaker, and it is - # therefore cacheable here. A member's own card directive is active-only - # and rides the trailing Writer message instead (``passes/writer.py``). - if resolved["post_history"]: - parts.append(f"\n\n## Additional Instructions\n{resolved['post_history']}") - if resolved["user_desc"].strip(): - user_label = macros.user if macros else "User" - parts.append(f"\n\n## User: {user_label}\n{resolved['user_desc']}") - - if extra_system_blocks: - for block in extra_system_blocks: - parts.append(f"\n\n{block}") - - processed_messages = [format_message_with_attachments(m, macros) for m in (messages or [])] - if cast and cast.grouped: - labelled: list[ChatMessage] = [] - names = dict(speaker_names or {}) - names.update({m.member_id: m.name for m in cast.members}) - for original, rendered in zip(messages or [], processed_messages, strict=True): - if rendered["role"] != "assistant": - labelled.append(rendered) - continue - label = group_speaker_label(names, original.get("speaker_member_id")) - content = rendered["content"] - if isinstance(content, str): - text = f"{label}: {content}" - if labelled and labelled[-1]["role"] == "assistant" and isinstance(labelled[-1]["content"], str): - labelled[-1] = {"role": "assistant", "content": str(labelled[-1]["content"]) + "\n\n" + text} - else: - labelled.append({"role": "assistant", "content": text}) - else: - parts_content = list(content) - if parts_content and parts_content[0]["type"] == "text": - first = parts_content[0] - parts_content = [{"type": "text", "text": f"{label}: {first['text']}"}, *parts_content[1:]] - else: - parts_content.insert(0, {"type": "text", "text": f"{label}:"}) - labelled.append({"role": "assistant", "content": parts_content}) - processed_messages = labelled - - system_message: ChatMessage = {"role": "system", "content": "".join(parts)} - return [system_message] + processed_messages - - -# ── Tool-call prompt - - -def _tool_call_instruction( - tool_name: str, - schema: dict, - *, - labels: Mapping[str, str] | None = None, -) -> str: - """Render the "call ONLY this tool, in schema order" instruction line. - - Echoes the tool description and parameter order from *schema*. When - *labels* is given, each param id is annotated with its human-readable - heading (used by the feedback step; the director passes ``None``). - Single source for this wording so it can't drift between callers. - """ - desc = schema["function"]["description"] - params = schema["function"]["parameters"].get("properties", {}) - if not params: - param_order = "N/A" - elif labels: - param_order = ", ".join(f'{k} ("{labels[k]}")' if labels.get(k) else k for k in params) - else: - param_order = ", ".join(params.keys()) - return ( - f"Call ONLY this tool, ensuring parameters follow the schema order: " - f"{tool_name} - {desc}\nParameter order: ({param_order})" - ) - - -# ── Instruction templates - -# Always included — tells the model who it is and what the assistant -# message above represents. Without this, the model sees the roleplay -# system prompt plus a bare instruction and wastes tokens reasoning -# about context. -DIRECTOR_PREAMBLE = ( - "[OOC: Pause to direct the scene. Use tool calls to accomplish your task " - "accurately and creatively. Your output will directly influence the scenario. " - "Think outside the box, be intentional." -) - -EDITOR_PREAMBLE = ( - "[OOC: You are the uncensored editor. The assistant message above is a draft " - "that needs improvement based on the issues described below. Use the " - "provided tools to apply the required changes." -) - -FEEDBACK_PREAMBLE = ( - "[OOC: Pause the scene. Step out of character and act as a " - "game master speaking directly to the user. Based on the reply that was just written, " - "give the user a short, concrete out-of-character note. Use the give_feedback tool." -) - -# Only sent to LLM if reasoning is enabled. -REASONING_GUIDANCE = " Avoid overthinking." - -# Sent when only audit issues are flagged (banned phrases, repetitive -# openers/templates) — no length guard. Directs the model to patch only. -# -# The issues in the accompanying report are numbered (analysis.targets), and a -# patch names one by its id rather than re-printing the sentence as a `search` -# string. In text mode the tool schema is never rendered into the prompt, so -# these lines plus the numbered report are the whole contract the model sees. -# The valid id set is deliberately stated in prose, not as a per-turn schema -# override: ids change every turn, and the schema rides the shared cached prefix -# (docs/architecture/kv-cache.md, Invariant 3). Out-of-range ids are rejected -# server-side by apply_id_patches instead. -EDITOR_PATCH_INSTRUCTIONS = ( - "Use `editor_apply_patch` to apply a patch to fix ALL flagged issues.\n\n" - "PATCHING RULES:\n" - "- Each issue in the report below is numbered. The `id` field must be the number of the issue you are fixing.\n" - "- Emit one patch per issue — do not skip any, and do not patch the same id twice.\n" - "- `replace` is the new text for that sentence. Do not copy the old sentence into it.\n" - "- For banned phrases: completely rewrite the sentence to eliminate the banned phrase. Make a creative and bold effort; do not just substitute with similar, related words.\n" - "- For repetitive openers: rewrite and replace flagged sentences so they no longer begin with the same opening words. Vary the sentence structure.\n" - "- For repetitive templates: restructure flagged sentences so they no longer follow the same POS pattern. Change clause order, combine sentences, or vary syntax.\n" - "- For repetitive phrases: rewrite and replace flagged phrases.\n" - "- For contrastive negation ('not X, but Y'): rewrite sentences that use this cliché construction. Consider alternative phrasing that avoids this rhetorical formula.\n" - "- For interrogative dialogue: replace the dialogue AND its related narration with something entirely different." -) - -# Sent when only the length guard is triggered — no audit issues. -# Directs the model to rewrite only. -EDITOR_REWRITE_INSTRUCTIONS = ( - "Use `editor_rewrite` to produce a rewrite within the specified limits.\n\n" - "REWRITING RULES:\n" - "- Preserve the author's vocabulary and creative word choices and all key story beats. Sentence starters should be varied.\n" - "- First priority is to get rid of repetitiveness and condense comma-separated adjectives into stronger, more precise words (e.g. old, ruined building -> decrepit building).\n" - "- Be more concise but maintain coherence and narrative flow." -) - -# Sent when both audit issues AND length guard are triggered. -# The model already receives the full audit report and length-guard -# instruction with concrete word/paragraph limits. -EDITOR_BOTH_INSTRUCTIONS = "Call `editor_rewrite` to address both concerns in a single rewrite. Address all audit issues while also respecting length constraints." - -# Prepended to the tool-result text on the structured-replay path, where the -# model's previous call is replayed verbatim beside a freshly numbered report. -# Ids are rebuilt from scratch on every re-audit (the draft moved, so the old -# offsets are meaningless), and the structured replay is the one place the model -# can see both numberings at once — so the rule has to be stated, not inferred. -EDITOR_RENUMBER_NOTICE = ( - "The draft has changed and the issues below have been renumbered. Ignore the ids from your previous " - "call and patch only the ids listed in this report." -) - -STRUCTURAL_REWRITE_INSTRUCTIONS = ( - "STRUCTURAL REPETITION: This response follows the same paragraph layout as recent " - "previous messages. Call `editor_rewrite` with an entirely different structure — " - "change the order and balance of narration, dialogue, and internal thought so the " - "response is laid out distinctly from the previous ones." -) - - -def build_director_tool_prompt( - tool_name: str, - user_message: str, - active_moods: list[str], - mood_fragments: Sequence[Mapping[str, Any]], - reasoning_on: bool = False, - interactive_fragments: Sequence[Mapping[str, Any]] | None = None, - progressive_state: dict | None = None, - tool_schema: dict | None = None, - cast_instruction: str = "", -) -> str: - """Build the combined director request for one tool. - - *cast_instruction* is the group speaking-plan line (``director.speaking_plan_ - instruction``): the roster is volatile and rides this per-call tail rather than - the shared tool blob, exactly like the editor's numbered issue ids. Empty on - a solo turn. - """ - tool = TOOLS.get(tool_name) - if not tool: - return "" - schema = tool_schema if tool_schema is not None else tool["schema"] - preamble = DIRECTOR_PREAMBLE + (REASONING_GUIDANCE if reasoning_on else "") - parts = [ - preamble, - _tool_call_instruction(tool_name, schema), - ] - if tool_name == "direct_scene": - if cast_instruction: - parts.append(cast_instruction) - # Scene context (progressive/interactive) before the mood options, mirroring - # the per-fragment builder: settle the scene, then pick moods that fit it. - progressive_lines = [ - f"* [{df['id']}] ({df['description']}): {(progressive_state or {}).get(df['id'])}" - for df in (interactive_fragments or []) - if df.get("field_type") == "progressive" and (progressive_state or {}).get(df["id"]) - ] - if progressive_lines: - parts.append("Previous progressive fields - dynamically update these:\n" + "\n".join(progressive_lines)) - parts.append(_moods_options_block(active_moods, mood_fragments)) - parts.append(f'User\'s next message (for context, take this into account when directing):\n"""{user_message}"""') - # Close the [OOC: aside opened in DIRECTOR_PREAMBLE; the whole instruction is the aside. - return "\n\n".join(parts) + "]" - - -def _render_decided(value: Any) -> str: - return ", ".join(str(x) for x in value) if isinstance(value, list) else str(value) - - -def _moods_options_block(active_moods: Sequence[str], mood_fragments: Sequence[Mapping[str, Any]]) -> str: - """The "previously active + available moods" block shared by both director prompts.""" - moods = ", ".join(active_moods) or "none" - frags = "\n".join(f"* [{f['id']}] - use in case: {f['description']}" for f in mood_fragments) - return f"Previously active moods: {moods}\n\nAvailable writing moods:\n{frags}" - - -def build_director_scene_step_prompt( - user_message: str, - active_moods: list[str], - mood_fragments: Sequence[Mapping[str, Any]], - *, - tool_schema: dict | None = None, - reasoning_on: bool = False, - target_fragment: Mapping[str, Any] | None = None, - decided_fields: Sequence[tuple[str, Any]] = (), - progressive_prior: Any = None, - cast_instruction: str = "", -) -> str: - """Build one ``direct_scene`` request that targets a single output. - - With ``target_fragment`` None the model is asked only for ``moods``; otherwise - only for the named fragment, with the values already chosen this turn - (``decided_fields``) shown so it can build on them. - - *cast_instruction* is passed only on the speaking-plan stage, and is the only - place that stage's roster appears: a step prompt echoes the stage's own - ``description``, never the schema property's, so before this the per-fragment - path cast the exchange without ever being told the valid keys. - """ - schema = tool_schema if tool_schema is not None else TOOLS["direct_scene"]["schema"] - desc = schema["function"]["description"] - parts = [DIRECTOR_PREAMBLE + (REASONING_GUIDANCE if reasoning_on else "")] - - if target_fragment is None: - parts.append(f"Call ONLY direct_scene - {desc}\nFill ONLY: moods.") - # Moods run last this turn, so show the scene already directed and let the - # model pick moods that fit it (distinct heading from the interactive - # branch's "build on / do not contradict" — moods only need to match). - scene = [f"- {label}: {_render_decided(value)}" for label, value in decided_fields if value] - if scene: - parts.append("Scene direction decided this turn (pick moods that fit it):\n" + "\n".join(scene)) - parts.append(_moods_options_block(active_moods, mood_fragments)) - else: - fid = target_fragment["id"] - hint = {"array": "list of strings", "progressive": "single value, evolves across turns"}.get( - target_fragment["field_type"], "single value" - ) - parts.append( - f"Call ONLY direct_scene - {desc}\nFill ONLY the '{fid}' parameter. Leave moods and all other fields empty." - ) - parts.append(f"Field '{fid}' ({hint}): {target_fragment['description']}") - if cast_instruction: - parts.append(cast_instruction) - prior = [f"- {label}: {_render_decided(value)}" for label, value in decided_fields if value] - if prior: - parts.append("Decided so far this turn (build on these, do not contradict):\n" + "\n".join(prior)) - if target_fragment["field_type"] == "progressive" and progressive_prior: - parts.append(f"Previous value (update it): {progressive_prior}") - - parts.append(f'User\'s next message (context):\n"""{user_message}"""') - # Close the [OOC: aside opened in DIRECTOR_PREAMBLE; the whole instruction is the aside. - return "\n\n".join(parts) + "]" - - -def build_lorebook_select_prompt(catalog: str, user_message: str, *, reasoning_on: bool = False) -> str: - """Build the request for the standalone agentic-lorebook ``select_lorebook`` step. - - The catalog of selectable entries rides this OOC trailing (not the system prompt - or the tools blob), so the call reuses the shared history KV the other passes warm. - The pending *user_message* trails the catalog: during the director pass it is not - yet in the shared history, so without it the model can't judge relevance (it would - only see the prior turn). Placed last so the stable preamble+catalog prefix caches. - """ - parts = [ - DIRECTOR_PREAMBLE + (REASONING_GUIDANCE if reasoning_on else ""), - ( - "Call ONLY select_lorebook. From the catalog below, choose ONLY the entries relevant to the " - "current scene and the user's next message (quoted after the catalog); leave the selection " - "empty if none apply." - ), - catalog, - f'User\'s next message:\n"""{user_message}"""', - ] - # Close the [OOC: aside opened in DIRECTOR_PREAMBLE; the whole instruction is the aside. - return "\n\n".join(parts) + "]" - - -def build_feedback_prompt( - feedback_fragments: Sequence[Mapping[str, Any]], - reasoning_on: bool = False, - tool_schema: dict | None = None, -) -> str: - """Build the request message for the post-writer feedback step. - - The just-written reply is already in the message history as an assistant - message, so it is not quoted here. *tool_schema* is the dynamic - ``give_feedback`` schema; its parameter order is echoed via - :func:`_tool_call_instruction` so the model fills fields in schema order. - Each param id is paired with its ``injection_label`` (the heading the user - sees) so the model understands what each opaque id means. Labels live in - the per-turn request, not the tools blob, so the shared KV cache is untouched. - """ - preamble = FEEDBACK_PREAMBLE + (REASONING_GUIDANCE if reasoning_on else "") - parts = [preamble] - if tool_schema is not None: - labels = {df["id"]: (df.get("injection_label") or "").strip() for df in feedback_fragments} - parts.append(_tool_call_instruction("give_feedback", tool_schema, labels=labels)) - # Close the [OOC: aside opened in FEEDBACK_PREAMBLE; the whole instruction is the aside. - return "\n\n".join(parts) + "]" - - -DIRECTION_NOTE_PREAMBLE = ( - "[OOC: Pause the roleplay and step out of character. The categories below are standing records " - "of lasting direction for this roleplay, and your task now is to update them. Work through each " - "one and record into it anything from what just happened that must hold for the rest of the " - "roleplay. Whatever you record is permanent: it returns on every later reply and steers the " - "rest of the story, so a category takes only what genuinely must constrain what follows -- if " - "nothing this turn belongs in a category, leave it empty. Record only the bare fact in each " - "category -- no leading label, category name, or turn number. Those are attached automatically; " - "where earlier entries appear tagged that way, the tag is for your reference only." -) - - -def _direction_notes_lines(notes: Sequence[Mapping[str, Any]]) -> str: - """One line per note in the given order (oldest-first, i.e. turn order), each tagged - with its authoring fragment's label and the turn it was recorded on.""" - lines = [] - for n in notes: - turn = n.get("turn_index") - tag = f"{n['interactive_fragment_label']}, turn {turn}" if turn is not None else n["interactive_fragment_label"] - lines.append(f"- ({tag}) {n['content']}") - return "\n".join(lines) - - -def render_direction_notes_block(notes: Sequence[Mapping[str, Any]]) -> str: - """Render the active direction notes as a Scene Direction sub-block, or '' when empty. - - Notes are listed in turn order, each prefixed with the label of the fragment that - authored it so the writer can tell which directive a note belongs to. - """ - if not notes: - return "" - return f"**Direction Notes**\n{_direction_notes_lines(notes)}" - - -def build_direction_note_prompt( - active_notes: Sequence[Mapping[str, Any]], - direction_note_fragments: Sequence[Mapping[str, Any]], - *, - inj_block: str | None = None, - reasoning_on: bool = False, - tool_schema: dict | None = None, -) -> str: - """Build the request message for the direction-note step. - - *active_notes* are already in effect on this branch; they are listed in turn order - (each labelled with its fragment) so the model evolves them rather than restating - them. *inj_block* is this turn's scene direction, passed for the pre-writer placement; - the post-turn placement omits it because the finished reply is already replayed in the - message history. Each parameter id is paired with its fragment's label so the model - knows what each opaque category id means. - """ - preamble = DIRECTION_NOTE_PREAMBLE + (REASONING_GUIDANCE if reasoning_on else "") - parts = [preamble] - if active_notes: - parts.append("Already recorded (do not repeat these):\n" + _direction_notes_lines(active_notes)) - if inj_block: - parts.append(inj_block) - if tool_schema is not None: - labels = {df["id"]: (df.get("injection_label") or df.get("label") or "").strip() for df in direction_note_fragments} - parts.append(_tool_call_instruction("record_direction_note", tool_schema, labels=labels)) - # Close the [OOC: aside opened in DIRECTION_NOTE_PREAMBLE; the whole request is the aside, - # so the bracket closes at the very end, not inside the preamble. - return "\n\n".join(parts) + "]" - - -WORLD_CHANGE_PREAMBLE = ( - "[OOC: Pause the roleplay and step out of character. Review the exchange above and decide whether it " - "added anything to a World's long-term memory. Use the World catalog below. Leave operations empty " - "when nothing durable was established." -) - -# The exclusions carry the weight: without them a model files every gesture and -# intention, and the review queue becomes noise. -# -# Two of them answer observed failures rather than theory. The reply is the -# step's own prose, and a model reads its own prior turn as settled fact: left -# unsaid, it files a suggestion the user has not answered yet -- and, on an -# assistant-style book, cannot have answered, since the confirming turn is the -# one after this step runs. And an entry is worth more the less it churns, so -# "already covered" has to resolve to silence, not to a reworded revision. -WORLD_CHANGE_RULES = ( - "Record only durable facts established by the exchange for long-term memory. Most turns add nothing; " - "leave operations empty when nothing qualifies.\n" - "- Do not record plans, guesses, possibilities, or facts introduced only in the assistant's reply " - "until the user takes them up.\n" - "- Preserve uncertainty and attribution: record rumors, beliefs, and disputed claims as such.\n" - "- Write concise factual notes, not narrative prose.\n" - "- Create only new information. Revise or retract only when an existing entry is no longer accurate; " - "never duplicate, reword, or add detail to a correct entry." -) - -WORLD_CHANGE_CATALOG_HEADER = ( - "**Current World memory** -- each `##` heading is a World and its current entries. Headings use " - "`## [world_id: ]`; for a `create` with more than one World, copy that id into " - "`target_world`. Each entry's stable numeric id appears in brackets, `[id]`; use it as " - "`target_entry_id` for `revise` or `retract`. `Authored` is user-written memory; `Dynamic World " - "State` is accepted Agent-managed memory." -) - - -def build_world_change_prompt( - catalog: str, - *, - original_user_message: str = "", - reasoning_on: bool = False, - tool_schema: dict | None = None, -) -> str: - """Build the post-turn Dynamic Worlds proposal request.""" - preamble = WORLD_CHANGE_PREAMBLE + (REASONING_GUIDANCE if reasoning_on else "") - parts = [preamble, WORLD_CHANGE_RULES] - if original_user_message: - parts.append( - "The user turn above is Orb's own instruction to the writer, not something the user said. " - f'Judge this as the user\'s message instead:\n"""{original_user_message}"""' - ) - if catalog: - parts.append(f"{WORLD_CHANGE_CATALOG_HEADER}\n{catalog}") - if tool_schema is not None: - parts.append(_tool_call_instruction("propose_world_changes", tool_schema)) - # Close the [OOC: aside opened in WORLD_CHANGE_PREAMBLE; the whole request is the aside. - return "\n\n".join(parts) + "]" - - -def build_editor_prompt( - has_audit_issues: bool, - report_text: str, - length_guard_triggered: bool, - length_guard_instruction: str, - structural_rewrite: bool = False, - reasoning_on: bool = False, - patchable: bool = True, -) -> str: - """Assemble the editor's request message. - - *patchable* says whether the issues resolved to numbered targets. They - normally do; when they do not — structural repetition has no span, and a - issue the detectors segmented differently from the draft cannot be - located — there is nothing to address by id, so the request falls through - to the rewrite path rather than shipping a report the model cannot act on. - Callers must render *report_text* to match: the numbered report only when - this ends up on the patch branch, the sectioned one otherwise. - """ - preamble = EDITOR_PREAMBLE + (REASONING_GUIDANCE if reasoning_on else "") - parts = [preamble] - rewrite_triggered = length_guard_triggered or structural_rewrite or (has_audit_issues and not patchable) - - if rewrite_triggered: - parts.append(EDITOR_REWRITE_INSTRUCTIONS) - if has_audit_issues: - parts.append(report_text) - if structural_rewrite: - parts.append(STRUCTURAL_REWRITE_INSTRUCTIONS) - if length_guard_triggered: - parts.append(length_guard_instruction) - if has_audit_issues and length_guard_triggered: - parts.append(EDITOR_BOTH_INSTRUCTIONS) - elif has_audit_issues: - parts.append(EDITOR_PATCH_INSTRUCTIONS) - parts.append(report_text) - - # Close the [OOC: aside opened in EDITOR_PREAMBLE; the whole instruction is the aside. - return "\n\n".join(parts) + "]" - - -# ── Style injection block - - -def resolve_mood_fragment_randoms( - mood_fragments: Sequence[Mapping[str, Any]], - renderable_ids: Collection[str], - choices: MutableMapping[str, str], -) -> list[Mapping[str, Any]]: - """Resolve {{random}} in the renderable mood fragments' prompt fields. - - Picks come from / are recorded into *choices*, the per-conversation map - (``director_state.macro_choices`` — see :func:`core.macros.resolve_stored_random`), - so a fragment's macros roll once per conversation and stay fixed. Fragments - not in *renderable_ids* pass through untouched, keeping the map free of - picks for moods that were never activated. - """ - resolved: list[Mapping[str, Any]] = [] - for f in mood_fragments: - if f["id"] in renderable_ids: - prompt_text, negative_prompt = resolve_stored_random( - [f.get("prompt_text", ""), f.get("negative_prompt", "")], choices, f"mood:{f['id']}" - ) - f = {**f, "prompt_text": prompt_text, "negative_prompt": negative_prompt} - resolved.append(f) - return resolved - - -def compute_style_injection_block( - active_moods: list[str], - prior_moods: list[str], - mood_fragments: Sequence[Mapping[str, Any]], - interactive_fragments: Sequence[Mapping[str, Any]], - direct_scene_enabled: bool, - extra_fields: dict | None = None, - prior_progressive_state: dict | None = None, -) -> str: - """Compute the Scene Direction injection block from director pass outputs. - - When *direct_scene_enabled* is ``False``, mood signals and extra fields are - cleared so the previous turn's director state cannot bleed into the writer. - """ - if extra_fields is None: - extra_fields = {} - - if direct_scene_enabled: - inj_active_moods = active_moods - inj_extra = extra_fields - else: - inj_active_moods = [] - inj_extra = {} - - deactivated = ( - [f for f in mood_fragments if f["id"] in (set(prior_moods) - set(inj_active_moods))] - if direct_scene_enabled and inj_active_moods - else [] - ) - active = [f for f in mood_fragments if f["id"] in inj_active_moods] - - if not (active or deactivated or inj_extra): - return "" - - return build_style_injection(active, deactivated, interactive_fragments, inj_extra, prior_progressive_state) - - -def build_style_injection( - active: Sequence[Mapping[str, Any]], - deactivated: Sequence[Mapping[str, Any]] | None = None, - interactive_fragments: Sequence[Mapping[str, Any]] | None = None, - extra_fields: dict | None = None, - prior_progressive_state: dict | None = None, -) -> str: - """Render the Scene Direction block for the writer pass. - - Interactive fragment values are rendered in ``sort_order`` using each - fragment's ``injection_label``. Array fields become bullet lists. - """ - parts = ["**Scene Direction**"] - - # Moods first, interactive fragments last: the writer reads this block, and - # the concrete scene steer (interactive) belongs closest to the end for - # recency/attention. (The director's *processing* order is the opposite — - # it decides interactive first, then moods; see build_director_scene_step_prompt.) - for f in active: - parts.append(f["prompt_text"]) - for f in deactivated or []: - if neg := f.get("negative_prompt", "").strip(): - parts.append(neg) - - for df in sorted(interactive_fragments or [], key=lambda x: x.get("sort_order", 0)): - val = (extra_fields or {}).get(df["id"]) - if not val: - continue - label = df["injection_label"] - if df["field_type"] == "array" and isinstance(val, list): - parts.append(label + ":\n" + "\n".join(f"- {item}" for item in val)) - elif df["field_type"] == "progressive": - old_val = (prior_progressive_state or {}).get(df["id"]) - transition = f"{old_val} -> {val}" if old_val and old_val != val else str(val) - parts.append(f"{label} ({df['description']}): {transition}") - else: - parts.append(f"{label}: {val}") - - return "\n\n".join(parts) diff --git a/backend/inference/schema.py b/backend/inference/schema.py new file mode 100644 index 00000000..d8629e8c --- /dev/null +++ b/backend/inference/schema.py @@ -0,0 +1,28 @@ +"""Provider-facing structured-output schema normalization.""" + +from __future__ import annotations + + +def strictify_schema(schema: dict) -> dict: + """Copy *schema* into OpenAI strict-mode shape recursively.""" + node = dict(schema) + properties = node.get("properties") + if isinstance(properties, dict): + required = set(node.get("required") or []) + output_properties: dict = {} + for key, property_schema in properties.items(): + sub = strictify_schema(property_schema) if isinstance(property_schema, dict) else property_schema + if key not in required and isinstance(sub, dict) and "type" in sub: + property_type = sub["type"] + if isinstance(property_type, list): + property_type = property_type if "null" in property_type else [*property_type, "null"] + elif property_type != "null": + property_type = [property_type, "null"] + sub = {**sub, "type": property_type} + output_properties[key] = sub + node["properties"] = output_properties + node["required"] = list(properties.keys()) + node["additionalProperties"] = False + if isinstance(node.get("items"), dict): + node["items"] = strictify_schema(node["items"]) + return node diff --git a/backend/pipeline/config.py b/backend/pipeline/config.py index 9e01c96a..9a99eb09 100644 --- a/backend/pipeline/config.py +++ b/backend/pipeline/config.py @@ -12,9 +12,9 @@ from ..inference import ( CachedBase, LLMClient, - build_direction_note_tool, - enabled_schemas, ) +from ..prompting.tool_catalog import enabled_schemas +from ..prompting.tool_schemas import build_direction_note_tool from ..workflows.enablement import disabled_workflow_tool_names from .passes.director import build_direct_scene_override from .passes.editor import _feedback_active, build_feedback_override diff --git a/backend/pipeline/context.py b/backend/pipeline/context.py index d314483c..bade5d22 100644 --- a/backend/pipeline/context.py +++ b/backend/pipeline/context.py @@ -22,21 +22,22 @@ ) from ..features.lorebook import ( agentic_lorebook_active, - build_lorebook_catalog, - compute_constant_lorebook_block, - compute_depth_lorebook_block, - compute_lorebook_injection_block, ) from ..inference import ( AbortToken, LLMClient, _KVCacheTracker, agent_client_from_settings, - build_prefix, client_from_settings, - macro_identity, separate_agent_lane_configured, ) +from ..prompting import build_prefix, macro_identity +from ..prompting.lorebook import ( + build_lorebook_catalog, + compute_constant_lorebook_block, + compute_depth_lorebook_block, + compute_lorebook_injection_block, +) from .config import _build_writer_tools_blob from .predicates import agent_enabled, resolve_persona_id, world_proposal_active from .state import LorebookTurn, WorldProposalTurn diff --git a/backend/pipeline/entrypoints.py b/backend/pipeline/entrypoints.py index bd8c99c7..153289ed 100644 --- a/backend/pipeline/entrypoints.py +++ b/backend/pipeline/entrypoints.py @@ -9,7 +9,8 @@ from .. import database as db from ..core import resolve_inline -from ..inference import AbortToken, prefix_is_speaker_scoped, tail_carries_identity +from ..inference import AbortToken +from ..prompting import prefix_is_speaker_scoped, tail_carries_identity from .cast import parse_speaking_plan, plan_cue, round_robin_member from .config import _resolve_pipeline_config, _split_interactive_fragments from .context import ( diff --git a/backend/pipeline/passes/_prompting.py b/backend/pipeline/passes/_prompting.py new file mode 100644 index 00000000..4747803e --- /dev/null +++ b/backend/pipeline/passes/_prompting.py @@ -0,0 +1,28 @@ +"""Shared instruction fragments for tool-calling pipeline passes.""" + +from __future__ import annotations + +from collections.abc import Mapping + +REASONING_GUIDANCE = " Avoid overthinking." + + +def tool_call_instruction( + tool_name: str, + schema: dict, + *, + labels: Mapping[str, str] | None = None, +) -> str: + """Render the ordered single-tool instruction used by pipeline passes.""" + description = schema["function"]["description"] + parameters = schema["function"]["parameters"].get("properties", {}) + if not parameters: + parameter_order = "N/A" + elif labels: + parameter_order = ", ".join(f'{key} ("{labels[key]}")' if labels.get(key) else key for key in parameters) + else: + parameter_order = ", ".join(parameters.keys()) + return ( + "Call ONLY this tool, ensuring parameters follow the schema order: " + f"{tool_name} - {description}\nParameter order: ({parameter_order})" + ) diff --git a/backend/pipeline/passes/director/direction_note.py b/backend/pipeline/passes/director/direction_note.py index fa578367..7da936e5 100644 --- a/backend/pipeline/passes/director/direction_note.py +++ b/backend/pipeline/passes/director/direction_note.py @@ -10,14 +10,16 @@ from ....core import ChatMessage, ContentPart, extract_hyperparams from ....inference import ( - RECORD_DIRECTION_NOTE_CHOICE, CachedBase, LLMClient, - build_direction_note_prompt, - build_direction_note_tool, parse_tool_calls, reasoning_cfg, ) +from ....prompting.tool_schemas import ( + RECORD_DIRECTION_NOTE_CHOICE, + build_direction_note_tool, +) +from .direction_note_prompts import build_direction_note_prompt logger = logging.getLogger(__name__) diff --git a/backend/pipeline/passes/director/direction_note_prompts.py b/backend/pipeline/passes/director/direction_note_prompts.py new file mode 100644 index 00000000..0074ddac --- /dev/null +++ b/backend/pipeline/passes/director/direction_note_prompts.py @@ -0,0 +1,59 @@ +"""Instruction and rendering contracts for direction notes.""" + +from __future__ import annotations + +from collections.abc import Mapping, Sequence +from typing import Any + +from .._prompting import REASONING_GUIDANCE, tool_call_instruction + +DIRECTION_NOTE_PREAMBLE = ( + "[OOC: Pause the roleplay and step out of character. The categories below are standing records " + "of lasting direction for this roleplay, and your task now is to update them. Work through each " + "one and record into it anything from what just happened that must hold for the rest of the " + "roleplay. Whatever you record is permanent: it returns on every later reply and steers the " + "rest of the story, so a category takes only what genuinely must constrain what follows -- if " + "nothing this turn belongs in a category, leave it empty. Record only the bare fact in each " + "category -- no leading label, category name, or turn number. Those are attached automatically; " + "where earlier entries appear tagged that way, the tag is for your reference only." +) + + +def _direction_notes_lines(notes: Sequence[Mapping[str, Any]]) -> str: + lines = [] + for note in notes: + turn = note.get("turn_index") + tag = f"{note['interactive_fragment_label']}, turn {turn}" if turn is not None else note["interactive_fragment_label"] + lines.append(f"- ({tag}) {note['content']}") + return "\n".join(lines) + + +def render_direction_notes_block(notes: Sequence[Mapping[str, Any]]) -> str: + """Render active direction notes as a Scene Direction sub-block.""" + if not notes: + return "" + return f"**Direction Notes**\n{_direction_notes_lines(notes)}" + + +def build_direction_note_prompt( + active_notes: Sequence[Mapping[str, Any]], + direction_note_fragments: Sequence[Mapping[str, Any]], + *, + inj_block: str | None = None, + reasoning_on: bool = False, + tool_schema: dict | None = None, +) -> str: + """Build the direction-note recording request.""" + preamble = DIRECTION_NOTE_PREAMBLE + (REASONING_GUIDANCE if reasoning_on else "") + parts = [preamble] + if active_notes: + parts.append("Already recorded (do not repeat these):\n" + _direction_notes_lines(active_notes)) + if inj_block: + parts.append(inj_block) + if tool_schema is not None: + labels = { + fragment["id"]: (fragment.get("injection_label") or fragment.get("label") or "").strip() + for fragment in direction_note_fragments + } + parts.append(tool_call_instruction("record_direction_note", tool_schema, labels=labels)) + return "\n\n".join(parts) + "]" diff --git a/backend/pipeline/passes/director/director.py b/backend/pipeline/passes/director/director.py index 76a8c705..345e8196 100644 --- a/backend/pipeline/passes/director/director.py +++ b/backend/pipeline/passes/director/director.py @@ -16,23 +16,21 @@ resolve_inline, ) from ....inference import ( - PRE_WRITER_TOOLS, - TOOLS, CachedBase, LLMClient, _KVCacheTracker, - build_direct_scene_tool, - build_director_scene_step_prompt, - build_director_tool_prompt, - compute_style_injection_block, parse_tool_calls, reasoning_cfg, - render_direction_notes_block, - resolve_mood_fragment_randoms, ) +from ....prompting import compute_style_injection_block, resolve_mood_fragment_randoms +from ....prompting.tool_catalog import require_tool +from ....prompting.tool_schemas import build_direct_scene_tool from ...predicates import direction_note_to_director, direction_note_to_writer +from ...tools import DIRECTOR_LOOP_TOOL_NAMES from . import progressive +from .direction_note_prompts import render_direction_notes_block from .lorebook_select import LorebookSelectResult, lorebook_select_step +from .prompts import build_director_scene_step_prompt, build_director_tool_prompt if TYPE_CHECKING: from ....core import Macros @@ -226,7 +224,7 @@ async def director_pass( all_calls: list[dict] = [] last_raw = "" - tool_names = [n for n, on in enabled_tools.items() if on and n in PRE_WRITER_TOOLS] + tool_names = [n for n, on in enabled_tools.items() if on and n in DIRECTOR_LOOP_TOOL_NAMES] if not tool_names: yield { @@ -300,7 +298,7 @@ async def director_pass( resp, label="director:direct_scene", trailing=trailing, - tool_choice=TOOLS["direct_scene"]["choice"], + tool_choice=require_tool("direct_scene")["choice"], kv_tracker=kv_tracker, json_schema=_step_schema(tool_schema, target) if tool_schema else None, **hyperparams, @@ -369,7 +367,7 @@ async def director_pass( resp, label=f"director:{name}", trailing=trailing, - tool_choice=TOOLS[name]["choice"], + tool_choice=require_tool(name)["choice"], kv_tracker=kv_tracker, **hyperparams, **reasoning_params, @@ -450,8 +448,8 @@ async def director_stage( direction_notes = director.get("direction_notes") or [] notes_block = macros.resolve_message(render_direction_notes_block(direction_notes)) if direction_notes else "" - has_pre_writer_tools = any(cfg.enabled_tools.get(n, False) for n in PRE_WRITER_TOOLS) - if cfg.agent_on and has_pre_writer_tools: + has_director_loop_tools = any(cfg.enabled_tools.get(n, False) for n in DIRECTOR_LOOP_TOOL_NAMES) + if cfg.agent_on and has_director_loop_tools: yield {"event": "director_start"} async for event in director_pass( cfg.agent_lane.client, @@ -503,6 +501,7 @@ async def director_stage( settings=settings, catalog=lorebook.catalog, user_message=state.user_message, + entries=lorebook.entries, kv_tracker=kv_tracker, reasoning_on=cfg.director_reasoning_on, reasoning_prefill=cfg.director_reasoning_prefill, diff --git a/backend/pipeline/passes/director/lorebook_select.py b/backend/pipeline/passes/director/lorebook_select.py index 0dee9156..107b9cf0 100644 --- a/backend/pipeline/passes/director/lorebook_select.py +++ b/backend/pipeline/passes/director/lorebook_select.py @@ -4,22 +4,42 @@ import json import logging -from collections.abc import AsyncIterator, Mapping +from collections.abc import AsyncIterator, Mapping, Sequence from dataclasses import dataclass, field from ....core import extract_hyperparams from ....inference import ( - SELECT_LOREBOOK_CHOICE, CachedBase, LLMClient, - build_lorebook_select_prompt, parse_tool_calls, reasoning_cfg, ) +from ....prompting.lorebook import director_pick_diagnostics +from ....prompting.tool_schemas import SELECT_LOREBOOK_CHOICE +from .prompts import build_lorebook_select_prompt logger = logging.getLogger(__name__) +def _log_director_pick_diagnostics( + entries: Sequence[Mapping[str, object]], + picks: Sequence[str], +) -> None: + recovered, unmatched = director_pick_diagnostics(entries, picks) + if recovered: + logger.warning( + "Lorebook: %d director pick(s) matched only after stripping catalog delimiters: %s", + len(recovered), + ", ".join(repr(pick) for pick in recovered), + ) + if unmatched: + logger.info( + "Lorebook: %d director pick(s) named no entry: %s", + len(unmatched), + ", ".join(repr(pick) for pick in unmatched), + ) + + @dataclass(slots=True) class LorebookSelectResult: """Typed result of the lorebook-select step, yielded as the ``done`` payload. @@ -40,6 +60,7 @@ async def lorebook_select_step( settings: Mapping[str, object], catalog: str, user_message: str, + entries: Sequence[Mapping[str, object]] | None = None, kv_tracker=None, reasoning_on: bool = False, reasoning_prefill: str = "", @@ -89,4 +110,7 @@ async def lorebook_select_step( if isinstance(picks, list): selected = [str(x) for x in picks] + if entries is not None: + _log_director_pick_diagnostics(entries, selected) + yield {"type": "done", "result": LorebookSelectResult(selected=selected, calls=calls)} diff --git a/backend/pipeline/passes/director/prompts.py b/backend/pipeline/passes/director/prompts.py new file mode 100644 index 00000000..15c2a827 --- /dev/null +++ b/backend/pipeline/passes/director/prompts.py @@ -0,0 +1,119 @@ +"""Instruction prompts owned by the Director passes.""" + +from __future__ import annotations + +from collections.abc import Mapping, Sequence +from typing import Any + +from ....prompting.tool_catalog import get_tool, require_tool +from .._prompting import REASONING_GUIDANCE, tool_call_instruction + +DIRECTOR_PREAMBLE = ( + "[OOC: Pause to direct the scene. Use tool calls to accomplish your task " + "accurately and creatively. Your output will directly influence the scenario. " + "Think outside the box, be intentional." +) + + +def _moods_options_block(active_moods: Sequence[str], mood_fragments: Sequence[Mapping[str, Any]]) -> str: + moods = ", ".join(active_moods) or "none" + fragments = "\n".join(f"* [{fragment['id']}] - use in case: {fragment['description']}" for fragment in mood_fragments) + return f"Previously active moods: {moods}\n\nAvailable writing moods:\n{fragments}" + + +def build_director_tool_prompt( + tool_name: str, + user_message: str, + active_moods: list[str], + mood_fragments: Sequence[Mapping[str, Any]], + reasoning_on: bool = False, + interactive_fragments: Sequence[Mapping[str, Any]] | None = None, + progressive_state: dict | None = None, + tool_schema: dict | None = None, + cast_instruction: str = "", +) -> str: + """Build the combined Director request for one tool.""" + tool = get_tool(tool_name) + if not tool: + return "" + schema = tool_schema if tool_schema is not None else tool["schema"] + preamble = DIRECTOR_PREAMBLE + (REASONING_GUIDANCE if reasoning_on else "") + parts = [preamble, tool_call_instruction(tool_name, schema)] + if tool_name == "direct_scene": + if cast_instruction: + parts.append(cast_instruction) + progressive_lines = [ + f"* [{fragment['id']}] ({fragment['description']}): {(progressive_state or {}).get(fragment['id'])}" + for fragment in (interactive_fragments or []) + if fragment.get("field_type") == "progressive" and (progressive_state or {}).get(fragment["id"]) + ] + if progressive_lines: + parts.append("Previous progressive fields - dynamically update these:\n" + "\n".join(progressive_lines)) + parts.append(_moods_options_block(active_moods, mood_fragments)) + parts.append(f'User\'s next message (for context, take this into account when directing):\n"""{user_message}"""') + return "\n\n".join(parts) + "]" + + +def _render_decided(value: Any) -> str: + return ", ".join(str(item) for item in value) if isinstance(value, list) else str(value) + + +def build_director_scene_step_prompt( + user_message: str, + active_moods: list[str], + mood_fragments: Sequence[Mapping[str, Any]], + *, + tool_schema: dict | None = None, + reasoning_on: bool = False, + target_fragment: Mapping[str, Any] | None = None, + decided_fields: Sequence[tuple[str, Any]] = (), + progressive_prior: Any = None, + cast_instruction: str = "", +) -> str: + """Build one ``direct_scene`` request targeting a single output.""" + schema = tool_schema if tool_schema is not None else require_tool("direct_scene")["schema"] + description = schema["function"]["description"] + parts = [DIRECTOR_PREAMBLE + (REASONING_GUIDANCE if reasoning_on else "")] + + if target_fragment is None: + parts.append(f"Call ONLY direct_scene - {description}\nFill ONLY: moods.") + scene = [f"- {label}: {_render_decided(value)}" for label, value in decided_fields if value] + if scene: + parts.append("Scene direction decided this turn (pick moods that fit it):\n" + "\n".join(scene)) + parts.append(_moods_options_block(active_moods, mood_fragments)) + else: + fragment_id = target_fragment["id"] + hint = { + "array": "list of strings", + "progressive": "single value, evolves across turns", + }.get(target_fragment["field_type"], "single value") + parts.append( + f"Call ONLY direct_scene - {description}\nFill ONLY the '{fragment_id}' parameter. " + "Leave moods and all other fields empty." + ) + parts.append(f"Field '{fragment_id}' ({hint}): {target_fragment['description']}") + if cast_instruction: + parts.append(cast_instruction) + prior = [f"- {label}: {_render_decided(value)}" for label, value in decided_fields if value] + if prior: + parts.append("Decided so far this turn (build on these, do not contradict):\n" + "\n".join(prior)) + if target_fragment["field_type"] == "progressive" and progressive_prior: + parts.append(f"Previous value (update it): {progressive_prior}") + + parts.append(f'User\'s next message (context):\n"""{user_message}"""') + return "\n\n".join(parts) + "]" + + +def build_lorebook_select_prompt(catalog: str, user_message: str, *, reasoning_on: bool = False) -> str: + """Build the standalone Agentic Lorebook selection request.""" + parts = [ + DIRECTOR_PREAMBLE + (REASONING_GUIDANCE if reasoning_on else ""), + ( + "Call ONLY select_lorebook. From the catalog below, choose ONLY the entries relevant to the " + "current scene and the user's next message (quoted after the catalog); leave the selection " + "empty if none apply." + ), + catalog, + f'User\'s next message:\n"""{user_message}"""', + ] + return "\n\n".join(parts) + "]" diff --git a/backend/pipeline/passes/editor/editor.py b/backend/pipeline/passes/editor/editor.py index ef6826f7..be17cc2f 100644 --- a/backend/pipeline/passes/editor/editor.py +++ b/backend/pipeline/passes/editor/editor.py @@ -39,17 +39,16 @@ ) from ....features.prose_rewriter import ProseRewriteConfig, rewrite_events from ....inference import ( - EDITOR_RENUMBER_NOTICE, - TOOLS, CachedBase, LLMClient, _KVCacheTracker, - build_editor_prompt, - build_feedback_tool, parse_tool_calls, reasoning_cfg, ) +from ....prompting.tool_catalog import require_tool +from ....prompting.tool_schemas import build_feedback_tool from .length_guard import LengthGuard, evaluate_length_guard +from .prompts import EDITOR_RENUMBER_NOTICE, build_editor_prompt logger = logging.getLogger(__name__) @@ -809,7 +808,7 @@ def _pick_tool_choice(length_guard_triggered: bool, report: AuditReport, audit_e if length_guard_triggered or _rewrite_only(report, targets): return {"type": "function", "function": {"name": "editor_rewrite"}} if audit_enabled: - return TOOLS["editor_apply_patch"]["choice"] + return require_tool("editor_apply_patch")["choice"] return "auto" diff --git a/backend/pipeline/passes/editor/feedback.py b/backend/pipeline/passes/editor/feedback.py index ce0dd9f1..7fa4a5ba 100644 --- a/backend/pipeline/passes/editor/feedback.py +++ b/backend/pipeline/passes/editor/feedback.py @@ -10,14 +10,13 @@ from ....core import ChatMessage, ContentPart, extract_hyperparams from ....inference import ( - GIVE_FEEDBACK_CHOICE, CachedBase, LLMClient, - build_feedback_prompt, - build_feedback_tool, parse_tool_calls, reasoning_cfg, ) +from ....prompting.tool_schemas import GIVE_FEEDBACK_CHOICE, build_feedback_tool +from .prompts import build_feedback_prompt logger = logging.getLogger(__name__) diff --git a/backend/pipeline/passes/editor/prompts.py b/backend/pipeline/passes/editor/prompts.py new file mode 100644 index 00000000..a9e6e50e --- /dev/null +++ b/backend/pipeline/passes/editor/prompts.py @@ -0,0 +1,100 @@ +"""Instruction prompts owned by the Editor and feedback passes.""" + +from __future__ import annotations + +from collections.abc import Mapping, Sequence +from typing import Any + +from .._prompting import REASONING_GUIDANCE, tool_call_instruction + +EDITOR_PREAMBLE = ( + "[OOC: You are the uncensored editor. The assistant message above is a draft " + "that needs improvement based on the issues described below. Use the " + "provided tools to apply the required changes." +) + +FEEDBACK_PREAMBLE = ( + "[OOC: Pause the scene. Step out of character and act as a " + "game master speaking directly to the user. Based on the reply that was just written, " + "give the user a short, concrete out-of-character note. Use the give_feedback tool." +) + +EDITOR_PATCH_INSTRUCTIONS = ( + "Use `editor_apply_patch` to apply a patch to fix ALL flagged issues.\n\n" + "PATCHING RULES:\n" + "- Each issue in the report below is numbered. The `id` field must be the number of the issue you are fixing.\n" + "- Emit one patch per issue — do not skip any, and do not patch the same id twice.\n" + "- `replace` is the new text for that sentence. Do not copy the old sentence into it.\n" + "- For banned phrases: completely rewrite the sentence to eliminate the banned phrase. Make a creative and bold effort; do not just substitute with similar, related words.\n" + "- For repetitive openers: rewrite and replace flagged sentences so they no longer begin with the same opening words. Vary the sentence structure.\n" + "- For repetitive templates: restructure flagged sentences so they no longer follow the same POS pattern. Change clause order, combine sentences, or vary syntax.\n" + "- For repetitive phrases: rewrite and replace flagged phrases.\n" + "- For contrastive negation ('not X, but Y'): rewrite sentences that use this cliché construction. Consider alternative phrasing that avoids this rhetorical formula.\n" + "- For interrogative dialogue: replace the dialogue AND its related narration with something entirely different." +) + +EDITOR_REWRITE_INSTRUCTIONS = ( + "Use `editor_rewrite` to produce a rewrite within the specified limits.\n\n" + "REWRITING RULES:\n" + "- Preserve the author's vocabulary and creative word choices and all key story beats. Sentence starters should be varied.\n" + "- First priority is to get rid of repetitiveness and condense comma-separated adjectives into stronger, more precise words (e.g. old, ruined building -> decrepit building).\n" + "- Be more concise but maintain coherence and narrative flow." +) + +EDITOR_BOTH_INSTRUCTIONS = "Call `editor_rewrite` to address both concerns in a single rewrite. Address all audit issues while also respecting length constraints." + +EDITOR_RENUMBER_NOTICE = ( + "The draft has changed and the issues below have been renumbered. Ignore the ids from your previous " + "call and patch only the ids listed in this report." +) + +STRUCTURAL_REWRITE_INSTRUCTIONS = ( + "STRUCTURAL REPETITION: This response follows the same paragraph layout as recent " + "previous messages. Call `editor_rewrite` with an entirely different structure — " + "change the order and balance of narration, dialogue, and internal thought so the " + "response is laid out distinctly from the previous ones." +) + + +def build_feedback_prompt( + feedback_fragments: Sequence[Mapping[str, Any]], + reasoning_on: bool = False, + tool_schema: dict | None = None, +) -> str: + """Build the post-Writer feedback request.""" + preamble = FEEDBACK_PREAMBLE + (REASONING_GUIDANCE if reasoning_on else "") + parts = [preamble] + if tool_schema is not None: + labels = {fragment["id"]: (fragment.get("injection_label") or "").strip() for fragment in feedback_fragments} + parts.append(tool_call_instruction("give_feedback", tool_schema, labels=labels)) + return "\n\n".join(parts) + "]" + + +def build_editor_prompt( + has_audit_issues: bool, + report_text: str, + length_guard_triggered: bool, + length_guard_instruction: str, + structural_rewrite: bool = False, + reasoning_on: bool = False, + patchable: bool = True, +) -> str: + """Assemble the Editor's request message.""" + preamble = EDITOR_PREAMBLE + (REASONING_GUIDANCE if reasoning_on else "") + parts = [preamble] + rewrite_triggered = length_guard_triggered or structural_rewrite or (has_audit_issues and not patchable) + + if rewrite_triggered: + parts.append(EDITOR_REWRITE_INSTRUCTIONS) + if has_audit_issues: + parts.append(report_text) + if structural_rewrite: + parts.append(STRUCTURAL_REWRITE_INSTRUCTIONS) + if length_guard_triggered: + parts.append(length_guard_instruction) + if has_audit_issues and length_guard_triggered: + parts.append(EDITOR_BOTH_INSTRUCTIONS) + elif has_audit_issues: + parts.append(EDITOR_PATCH_INSTRUCTIONS) + parts.append(report_text) + return "\n\n".join(parts) + "]" diff --git a/backend/pipeline/passes/world_change.py b/backend/pipeline/passes/world_change.py index 0c25229a..722900c7 100644 --- a/backend/pipeline/passes/world_change.py +++ b/backend/pipeline/passes/world_change.py @@ -15,14 +15,16 @@ validate_proposal, ) from ...inference import ( - PROPOSE_WORLD_CHANGES_CHOICE, CachedBase, LLMClient, - build_world_change_prompt, parse_tool_calls, reasoning_cfg, ) -from ...inference.tool_registry import PROPOSE_WORLD_CHANGES_TOOL +from ...prompting.tool_schemas import ( + PROPOSE_WORLD_CHANGES_CHOICE, + PROPOSE_WORLD_CHANGES_TOOL, +) +from .world_change_prompt import build_world_change_prompt logger = logging.getLogger(__name__) diff --git a/backend/pipeline/passes/world_change_prompt.py b/backend/pipeline/passes/world_change_prompt.py new file mode 100644 index 00000000..5c9ad1e0 --- /dev/null +++ b/backend/pipeline/passes/world_change_prompt.py @@ -0,0 +1,52 @@ +"""Instruction prompt owned by the Dynamic Worlds pass.""" + +from __future__ import annotations + +from ._prompting import REASONING_GUIDANCE, tool_call_instruction + +WORLD_CHANGE_PREAMBLE = ( + "[OOC: Pause the roleplay and step out of character. Review the exchange above and decide whether it " + "added anything to a World's long-term memory. Use the World catalog below. Leave operations empty " + "when nothing durable was established." +) + +WORLD_CHANGE_RULES = ( + "Record only durable facts established by the exchange for long-term memory. Most turns add nothing; " + "leave operations empty when nothing qualifies.\n" + "- Do not record plans, guesses, possibilities, or facts introduced only in the assistant's reply " + "until the user takes them up.\n" + "- Preserve uncertainty and attribution: record rumors, beliefs, and disputed claims as such.\n" + "- Write concise factual notes, not narrative prose.\n" + "- Create only new information. Revise or retract only when an existing entry is no longer accurate; " + "never duplicate, reword, or add detail to a correct entry." +) + +WORLD_CHANGE_CATALOG_HEADER = ( + "**Current World memory** -- each `##` heading is a World and its current entries. Headings use " + "`## [world_id: ]`; for a `create` with more than one World, copy that id into " + "`target_world`. Each entry's stable numeric id appears in brackets, `[id]`; use it as " + "`target_entry_id` for `revise` or `retract`. `Authored` is user-written memory; `Dynamic World " + "State` is accepted Agent-managed memory." +) + + +def build_world_change_prompt( + catalog: str, + *, + original_user_message: str = "", + reasoning_on: bool = False, + tool_schema: dict | None = None, +) -> str: + """Build the post-turn Dynamic Worlds proposal request.""" + preamble = WORLD_CHANGE_PREAMBLE + (REASONING_GUIDANCE if reasoning_on else "") + parts = [preamble, WORLD_CHANGE_RULES] + if original_user_message: + parts.append( + "The user turn above is Orb's own instruction to the writer, not something the user said. " + f'Judge this as the user\'s message instead:\n"""{original_user_message}"""' + ) + if catalog: + parts.append(f"{WORLD_CHANGE_CATALOG_HEADER}\n{catalog}") + if tool_schema is not None: + parts.append(tool_call_instruction("propose_world_changes", tool_schema)) + return "\n\n".join(parts) + "]" diff --git a/backend/pipeline/passes/writer.py b/backend/pipeline/passes/writer.py index cbab92eb..58d9877c 100644 --- a/backend/pipeline/passes/writer.py +++ b/backend/pipeline/passes/writer.py @@ -23,10 +23,9 @@ CachedBase, LLMClient, _KVCacheTracker, - member_macros, reasoning_cfg, - tail_carries_identity, ) +from ...prompting import member_macros, tail_carries_identity from .editor.length_guard import LengthGuard, writer_nudge if TYPE_CHECKING: diff --git a/backend/pipeline/state.py b/backend/pipeline/state.py index a059a3ec..3fb8fb25 100644 --- a/backend/pipeline/state.py +++ b/backend/pipeline/state.py @@ -7,13 +7,13 @@ from typing import Any from ..core import ChatMessage, ContentPart, Macros, joined_delta -from ..features.lorebook import ( +from ..features.prose_rewriter import ProseRewriteConfig +from ..inference import CachedBase, LLMClient +from ..prompting.lorebook import ( AGENTIC_LOREBOOK_SCAN_DEPTH, LOREBOOK_SCAN_DEPTH, compute_lorebook_block, ) -from ..features.prose_rewriter import ProseRewriteConfig -from ..inference import CachedBase, LLMClient from .passes.editor.length_guard import LengthGuard diff --git a/backend/pipeline/tools.py b/backend/pipeline/tools.py new file mode 100644 index 00000000..c4671705 --- /dev/null +++ b/backend/pipeline/tools.py @@ -0,0 +1,3 @@ +"""Pipeline-owned tool memberships.""" + +DIRECTOR_LOOP_TOOL_NAMES = frozenset({"direct_scene"}) diff --git a/backend/pipeline/workflow_bridge.py b/backend/pipeline/workflow_bridge.py index fc8adf80..cbd4d9bf 100644 --- a/backend/pipeline/workflow_bridge.py +++ b/backend/pipeline/workflow_bridge.py @@ -8,7 +8,8 @@ from typing import Any, cast from ..core import ChatMessage, workflow_character_state_lock, workflow_state_lock -from ..inference import TOOLS, LLMClient, _KVCacheTracker +from ..inference import LLMClient, _KVCacheTracker +from ..prompting.tool_catalog import has_tool from ..workflows import ( EV_ATTACH_ARTIFACT, EV_DRAFT_REPLACED, @@ -381,7 +382,7 @@ async def _iterate_pre_pipeline_hooks( val, ) continue - if name not in TOOLS: + if not has_tool(name): logger.warning( "workflow %r enabled unregistered tool %r; dropping", sub.workflow_id, diff --git a/backend/pipeline/world_proposal.py b/backend/pipeline/world_proposal.py index eb85b7ce..956e0e5c 100644 --- a/backend/pipeline/world_proposal.py +++ b/backend/pipeline/world_proposal.py @@ -13,8 +13,8 @@ CachedBase, agent_lane_from_settings, client_from_settings, - enabled_schemas, ) +from ..prompting.tool_catalog import enabled_schemas from ..workflows.toolkit import build_offturn_prefix from .context import conversation_macro_seed, persona_macros, resolve_card_and_persona from .passes.world_change import world_change_step diff --git a/backend/prompting/__init__.py b/backend/prompting/__init__.py new file mode 100644 index 00000000..63027184 --- /dev/null +++ b/backend/prompting/__init__.py @@ -0,0 +1,31 @@ +"""Deterministic, provider-independent model-facing construction.""" + +from .base import build_prefix, format_message_with_attachments, group_speaker_label +from .group_context import ( + context_size_components, + macro_identity, + member_macros, + prefix_is_speaker_scoped, + render_cast_section, + tail_carries_identity, +) +from .scene_direction import ( + build_style_injection, + compute_style_injection_block, + resolve_mood_fragment_randoms, +) + +__all__ = [ + "build_prefix", + "build_style_injection", + "compute_style_injection_block", + "context_size_components", + "format_message_with_attachments", + "group_speaker_label", + "macro_identity", + "member_macros", + "prefix_is_speaker_scoped", + "render_cast_section", + "resolve_mood_fragment_randoms", + "tail_carries_identity", +] diff --git a/backend/prompting/base.py b/backend/prompting/base.py new file mode 100644 index 00000000..3fd944e6 --- /dev/null +++ b/backend/prompting/base.py @@ -0,0 +1,144 @@ +"""Shared construction of model-facing message prefixes.""" + +from __future__ import annotations + +from collections.abc import Mapping, Sequence +from typing import Any + +from ..core import ChatMessage, ContentPart, Macros, TurnCast +from .group_context import render_cast_section + + +def format_message_with_attachments(message: Mapping[str, Any], macros: Macros | None) -> ChatMessage: + """Convert a message dict to chat format, embedding user attachments. + + Workflow attachment bytes never enter the prefix; annotations from root + rows are appended as text. + """ + role = message["role"] + raw = message.get("content", "") + text = macros.resolve_prompt(raw) if macros else raw + + user_atts: list[dict] = list(message.get("user_attachments") or []) + workflow_annotations: list[str] = [] + for attachment in message.get("workflow_attachments") or []: + if attachment.get("parent_attachment_id") is not None: + continue + annotation = attachment.get("annotation") + if isinstance(annotation, str) and annotation.strip(): + workflow_annotations.append(annotation) + + text_parts = [text] if text else [] + text_parts.extend(workflow_annotations) + combined_text = "\n\n".join(text_parts) + + if not user_atts: + return {"role": role, "content": combined_text} + + parts: list[ContentPart] = [] + if combined_text: + parts.append({"type": "text", "text": combined_text}) + for attachment in user_atts: + mime = attachment["mime_type"] + b64 = attachment["data_b64"] + parts.append({"type": "image_url", "image_url": {"url": f"data:{mime};base64,{b64}"}}) + return {"role": role, "content": parts} + + +def group_speaker_label(speaker_names: Mapping[str, str], speaker_member_id: object) -> str: + """Return the label used for one group assistant history row.""" + if not speaker_member_id: + return "Summary" + return speaker_names.get(str(speaker_member_id), "Unknown speaker") + + +def build_prefix( + system_prompt: str, + char_persona: str, + char_scenario: str, + mes_example: str = "", + post_history_instructions: str = "", + messages: Sequence[Mapping[str, Any]] | None = None, + macros: Macros | None = None, + user_description: str = "", + *, + constant_lorebook_block: str = "", + extra_system_blocks: list[str] | None = None, + cast: TurnCast | None = None, + speaker_names: Mapping[str, str] | None = None, +) -> list[ChatMessage]: + """Build the stable system prefix and rendered history messages.""" + resolve = macros.resolve_message if macros else (lambda text: text) + resolved = { + key: resolve(value) + for key, value in { + "persona": char_persona, + "scenario": char_scenario, + "mes_example": mes_example, + "post_history": post_history_instructions, + "user_desc": user_description, + }.items() + } + + parts = [system_prompt] + if cast and cast.grouped: + parts.append(render_cast_section(cast, macros)) + elif macros and macros.char: + parts.append(f"\n\n## Character: {macros.char}") + if resolved["persona"] and not (cast and cast.grouped): + parts.append(f"\n{resolved['persona']}") + if constant_lorebook_block: + parts.append(f"\n\n{constant_lorebook_block}") + if resolved["scenario"]: + parts.append(f"\n\n## Scenario\n{resolved['scenario']}") + if resolved["mes_example"] and not (cast and cast.grouped): + example = resolved["mes_example"] + if "" in example: + parts.append(f"\n\n{example.replace('', '## Example Dialogue')}") + else: + parts.append(f"\n\n## Example Dialogue\n{example}") + if resolved["post_history"]: + parts.append(f"\n\n## Additional Instructions\n{resolved['post_history']}") + if resolved["user_desc"].strip(): + user_label = macros.user if macros else "User" + parts.append(f"\n\n## User: {user_label}\n{resolved['user_desc']}") + + for block in extra_system_blocks or []: + parts.append(f"\n\n{block}") + + original_messages = messages or [] + processed_messages = [format_message_with_attachments(message, macros) for message in original_messages] + if cast and cast.grouped: + labelled: list[ChatMessage] = [] + names = dict(speaker_names or {}) + names.update({member.member_id: member.name for member in cast.members}) + for original, rendered in zip(original_messages, processed_messages, strict=True): + if rendered["role"] != "assistant": + labelled.append(rendered) + continue + label = group_speaker_label(names, original.get("speaker_member_id")) + content = rendered["content"] + if isinstance(content, str): + text = f"{label}: {content}" + if labelled and labelled[-1]["role"] == "assistant" and isinstance(labelled[-1]["content"], str): + labelled[-1] = { + "role": "assistant", + "content": str(labelled[-1]["content"]) + "\n\n" + text, + } + else: + labelled.append({"role": "assistant", "content": text}) + else: + content_parts = list(content) + if content_parts and content_parts[0]["type"] == "text": + first = content_parts[0] + content_parts = [ + {"type": "text", "text": f"{label}: {first['text']}"}, + *content_parts[1:], + ] + else: + content_parts.insert(0, {"type": "text", "text": f"{label}:"}) + labelled.append({"role": "assistant", "content": content_parts}) + processed_messages = labelled + + system_message: ChatMessage = {"role": "system", "content": "".join(parts)} + return [system_message] + processed_messages diff --git a/backend/inference/group_context.py b/backend/prompting/group_context.py similarity index 100% rename from backend/inference/group_context.py rename to backend/prompting/group_context.py diff --git a/backend/inference/lorebook.py b/backend/prompting/lorebook.py similarity index 90% rename from backend/inference/lorebook.py rename to backend/prompting/lorebook.py index 7875ff8a..2c6e949a 100644 --- a/backend/inference/lorebook.py +++ b/backend/prompting/lorebook.py @@ -2,15 +2,12 @@ from __future__ import annotations -import logging import re from collections.abc import Mapping, Sequence from typing import Any from ..core import Macros -logger = logging.getLogger(__name__) - LOREBOOK_SCAN_DEPTH = 6 # The agentic fallback scan only looks at the current turn (previous assistant # message + current user message), since the Director already saw the history. @@ -47,31 +44,6 @@ def select_effective_entries(entries: Sequence[Mapping[str, Any]]) -> list[Mappi return [e for e in live if not (is_dynamic(e) and e.get("overlay_action") == "suppress") and e.get("id") not in hidden] -def agentic_lorebook_active( - settings: Mapping[str, Any], - lorebook_entries: Sequence[Mapping[str, Any]], - *, - agent_on: bool, -) -> bool: - """Return True when the director should pick lorebook entries this turn. - - Requires the feature flag, the global agent on, and at least one non-constant - entry. It is independent of ``direct_scene``: the picks run in their own - forced ``select_lorebook`` call, so agentic lorebook works whether or not the - Director's scene-direction tool is enabled. Constant entries are always - injected and never managed by the director, so a pool of only constants does - not enable agentic mode. - - *agent_on* is passed in (rather than recomputed) so ``agent_enabled`` stays - the single source of truth — mirroring ``resolve_length_guard``. - """ - if not bool(settings.get("agentic_lorebook_enabled", 0)): - return False - if not agent_on: - return False - return any(not e.get("constant") for e in lorebook_entries) - - def build_lorebook_catalog(entries: Sequence[Mapping[str, Any]]) -> str: """Build the Director's lorebook catalog for the agentic activation path. @@ -217,8 +189,8 @@ def _resolve_director_picks( picks: Sequence[str], selectable: set[str], known: set[str], -) -> set[str]: - """Normalized picks that name a *selectable* entry, logging what needed undoing. +) -> tuple[set[str], list[str], list[str]]: + """Return matched, delimiter-recovered, and unknown Director picks. Two numbers fall out of this and both matter. A pick counted as *recovered* would have activated nothing before delimiter stripping, so its rate is the @@ -240,19 +212,22 @@ def _resolve_director_picks( recovered.append(raw) elif key not in known: unmatched.append(raw) - if recovered: - logger.warning( - "Lorebook: %d director pick(s) matched only after stripping catalog delimiters: %s", - len(recovered), - ", ".join(repr(p) for p in recovered), - ) - if unmatched: - logger.info( - "Lorebook: %d director pick(s) named no entry: %s", - len(unmatched), - ", ".join(repr(p) for p in unmatched), - ) - return matched + return matched, recovered, unmatched + + +def director_pick_diagnostics( + entries: Sequence[Mapping[str, Any]], + picks: Sequence[str], +) -> tuple[list[str], list[str]]: + """Return delimiter-recovered and unknown picks for upper-layer logging.""" + effective = select_effective_entries(entries) + candidates = [entry for entry in effective if not entry.get("constant")] + _, recovered, unmatched = _resolve_director_picks( + picks, + {_fold_name(entry) for entry in candidates}, + {_fold_name(entry) for entry in effective}, + ) + return recovered, unmatched def select_active_entries( @@ -276,7 +251,7 @@ def select_active_entries( """ effective = select_effective_entries(entries) candidates = [e for e in effective if not e.get("constant")] - director_named = _resolve_director_picks( + director_named, _, _ = _resolve_director_picks( director_selected, {_fold_name(e) for e in candidates}, {_fold_name(e) for e in effective}, diff --git a/backend/prompting/scene_direction.py b/backend/prompting/scene_direction.py new file mode 100644 index 00000000..977d35a4 --- /dev/null +++ b/backend/prompting/scene_direction.py @@ -0,0 +1,96 @@ +"""Deterministic Scene Direction projection and rendering.""" + +from __future__ import annotations + +from collections.abc import Collection, Mapping, MutableMapping, Sequence +from typing import Any + +from ..core import resolve_stored_random + + +def resolve_mood_fragment_randoms( + mood_fragments: Sequence[Mapping[str, Any]], + renderable_ids: Collection[str], + choices: MutableMapping[str, str], +) -> list[Mapping[str, Any]]: + """Resolve stored random macros in renderable mood prompt fields.""" + resolved: list[Mapping[str, Any]] = [] + for fragment in mood_fragments: + if fragment["id"] in renderable_ids: + prompt_text, negative_prompt = resolve_stored_random( + [fragment.get("prompt_text", ""), fragment.get("negative_prompt", "")], + choices, + f"mood:{fragment['id']}", + ) + fragment = {**fragment, "prompt_text": prompt_text, "negative_prompt": negative_prompt} + resolved.append(fragment) + return resolved + + +def compute_style_injection_block( + active_moods: list[str], + prior_moods: list[str], + mood_fragments: Sequence[Mapping[str, Any]], + interactive_fragments: Sequence[Mapping[str, Any]], + direct_scene_enabled: bool, + extra_fields: dict | None = None, + prior_progressive_state: dict | None = None, +) -> str: + """Compute the Scene Direction block from Director outputs.""" + if extra_fields is None: + extra_fields = {} + + if direct_scene_enabled: + injection_moods = active_moods + injection_extra = extra_fields + else: + injection_moods = [] + injection_extra = {} + + deactivated = ( + [fragment for fragment in mood_fragments if fragment["id"] in (set(prior_moods) - set(injection_moods))] + if direct_scene_enabled and injection_moods + else [] + ) + active = [fragment for fragment in mood_fragments if fragment["id"] in injection_moods] + + if not (active or deactivated or injection_extra): + return "" + return build_style_injection( + active, + deactivated, + interactive_fragments, + injection_extra, + prior_progressive_state, + ) + + +def build_style_injection( + active: Sequence[Mapping[str, Any]], + deactivated: Sequence[Mapping[str, Any]] | None = None, + interactive_fragments: Sequence[Mapping[str, Any]] | None = None, + extra_fields: dict | None = None, + prior_progressive_state: dict | None = None, +) -> str: + """Render the Scene Direction block for the Writer pass.""" + parts = ["**Scene Direction**"] + for fragment in active: + parts.append(fragment["prompt_text"]) + for fragment in deactivated or []: + if negative := fragment.get("negative_prompt", "").strip(): + parts.append(negative) + + for fragment in sorted(interactive_fragments or [], key=lambda item: item.get("sort_order", 0)): + value = (extra_fields or {}).get(fragment["id"]) + if not value: + continue + label = fragment["injection_label"] + if fragment["field_type"] == "array" and isinstance(value, list): + parts.append(label + ":\n" + "\n".join(f"- {item}" for item in value)) + elif fragment["field_type"] == "progressive": + old_value = (prior_progressive_state or {}).get(fragment["id"]) + transition = f"{old_value} -> {value}" if old_value and old_value != value else str(value) + parts.append(f"{label} ({fragment['description']}): {transition}") + else: + parts.append(f"{label}: {value}") + return "\n\n".join(parts) diff --git a/backend/prompting/tool_catalog.py b/backend/prompting/tool_catalog.py new file mode 100644 index 00000000..b8236049 --- /dev/null +++ b/backend/prompting/tool_catalog.py @@ -0,0 +1,173 @@ +"""Ordered lookup and registration for model-facing tool contracts.""" + +from __future__ import annotations + +from collections.abc import Iterator, Mapping, Set +from copy import deepcopy +from dataclasses import dataclass + +from .tool_schemas import ( + EDITOR_APPLY_PATCH_TOOL, + EDITOR_REWRITE_TOOL, + GIVE_FEEDBACK_CHOICE, + PROPOSE_WORLD_CHANGES_CHOICE, + PROPOSE_WORLD_CHANGES_TOOL, + RECORD_DIRECTION_NOTE_CHOICE, + SELECT_LOREBOOK_CHOICE, + SELECT_LOREBOOK_TOOL, + build_direct_scene_tool, + build_direction_note_tool, + build_feedback_tool, +) + +BUILTIN_TOOL_ORDER = ( + "direct_scene", + "editor_apply_patch", + "editor_rewrite", + "give_feedback", + "record_direction_note", + "select_lorebook", + "propose_world_changes", +) +BUILTIN_TOOL_NAMES = frozenset(BUILTIN_TOOL_ORDER) + +_tools: dict[str, dict] = { + "direct_scene": { + "choice": {"type": "function", "function": {"name": "direct_scene"}}, + "schema": build_direct_scene_tool([]), + }, + "editor_apply_patch": { + "choice": {"type": "function", "function": {"name": "editor_apply_patch"}}, + "schema": deepcopy(EDITOR_APPLY_PATCH_TOOL), + }, + "editor_rewrite": { + "choice": {"type": "function", "function": {"name": "editor_rewrite"}}, + "schema": deepcopy(EDITOR_REWRITE_TOOL), + }, + "give_feedback": { + "choice": deepcopy(GIVE_FEEDBACK_CHOICE), + "schema": build_feedback_tool([]), + }, + "record_direction_note": { + "choice": deepcopy(RECORD_DIRECTION_NOTE_CHOICE), + "schema": build_direction_note_tool([]), + }, + "select_lorebook": { + "choice": deepcopy(SELECT_LOREBOOK_CHOICE), + "schema": deepcopy(SELECT_LOREBOOK_TOOL), + }, + "propose_world_changes": { + "choice": deepcopy(PROPOSE_WORLD_CHANGES_CHOICE), + "schema": deepcopy(PROPOSE_WORLD_CHANGES_TOOL), + }, +} +assert tuple(_tools) == BUILTIN_TOOL_ORDER + +_standalone_tools: set[str] = set() + + +class _LiveSetView(Set[str]): + """Read-only set interface over mutable catalog-owned membership.""" + + def __contains__(self, value: object) -> bool: + return value in _standalone_tools + + def __iter__(self) -> Iterator[str]: + return iter(_standalone_tools) + + def __len__(self) -> int: + return len(_standalone_tools) + + +class _LiveToolsView(Mapping[str, dict]): + """Read-only live catalog view that does not expose mutable internals.""" + + def __getitem__(self, name: str) -> dict: + return deepcopy(_tools[name]) + + def __iter__(self) -> Iterator[str]: + return iter(_tools) + + def __len__(self) -> int: + return len(_tools) + + +TOOLS: Mapping[str, dict] = _LiveToolsView() +STANDALONE_TOOLS: Set[str] = _LiveSetView() + + +@dataclass(frozen=True, slots=True) +class CatalogSnapshot: + """Opaque state snapshot used by isolated registration fixtures.""" + + tools: tuple[tuple[str, dict], ...] + standalone_tools: frozenset[str] + + +def get_tool(name: str) -> dict | None: + """Return a registered tool specification, if present.""" + tool = _tools.get(name) + return deepcopy(tool) if tool is not None else None + + +def require_tool(name: str) -> dict: + """Return a registered tool specification or raise ``KeyError``.""" + return deepcopy(_tools[name]) + + +def has_tool(name: str) -> bool: + return name in _tools + + +def is_standalone_tool(name: str) -> bool: + return name in _standalone_tools + + +def register_tool( + name: str, schema: dict, choice: dict, *, standalone: bool = False +) -> None: + """Register or replace a tool while preserving an existing position.""" + _tools[name] = {"schema": deepcopy(schema), "choice": deepcopy(choice)} + if standalone: + _standalone_tools.add(name) + else: + _standalone_tools.discard(name) + + +def remove_tool(name: str) -> None: + """Remove a workflow tool from the catalog.""" + if name in BUILTIN_TOOL_NAMES: + raise ValueError(f"cannot remove built-in tool {name!r}") + _tools.pop(name, None) + _standalone_tools.discard(name) + + +def snapshot_catalog() -> CatalogSnapshot: + return CatalogSnapshot( + tuple((name, deepcopy(tool)) for name, tool in _tools.items()), + frozenset(_standalone_tools), + ) + + +def restore_catalog(snapshot: CatalogSnapshot) -> None: + """Restore a snapshot without exposing mutable catalog internals.""" + _tools.clear() + _tools.update((name, deepcopy(tool)) for name, tool in snapshot.tools) + _standalone_tools.clear() + _standalone_tools.update(snapshot.standalone_tools) + + +def enabled_schemas( + enabled_tools: Mapping[str, bool] | None, + overrides: Mapping[str, dict] | None = None, +) -> list[dict]: + """Return enabled, non-standalone schemas in catalog order.""" + overrides = overrides or {} + eligible = [name for name in _tools if name not in _standalone_tools] + if enabled_tools is not None: + eligible = [name for name in eligible if enabled_tools.get(name, False)] + return [ + deepcopy(schema) + for name in eligible + if (schema := overrides.get(name, _tools[name]["schema"])) is not None + ] diff --git a/backend/inference/tool_registry.py b/backend/prompting/tool_schemas.py similarity index 66% rename from backend/inference/tool_registry.py rename to backend/prompting/tool_schemas.py index 49f5fe13..5cdefad1 100644 --- a/backend/inference/tool_registry.py +++ b/backend/prompting/tool_schemas.py @@ -1,4 +1,4 @@ -"""Define and assemble built-in tool schemas.""" +"""Concrete built-in schemas, choices, and dynamic schema builders.""" from __future__ import annotations @@ -322,155 +322,3 @@ def build_direction_note_tool(direction_note_fragments: Sequence[Mapping[str, An }, }, } - - -# ── Tool registry & helpers - -TOOLS: dict[str, dict] = { - "direct_scene": { - "choice": {"type": "function", "function": {"name": "direct_scene"}}, - "schema": build_direct_scene_tool([]), - }, - "editor_apply_patch": { - "choice": {"type": "function", "function": {"name": "editor_apply_patch"}}, - "schema": EDITOR_APPLY_PATCH_TOOL, - }, - "editor_rewrite": { - "choice": {"type": "function", "function": {"name": "editor_rewrite"}}, - "schema": EDITOR_REWRITE_TOOL, - }, - # Internal, feedback-flag-gated (never user-toggleable, like editor_rewrite). - # The empty-properties placeholder schema is always overridden per-turn via - # schema_overrides with build_feedback_tool(feedback_fragments) when feedback - # is enabled; registering it here is what lets enabled_schemas() emit its - # bytes into the shared blob so the feedback step reuses the cached base. - "give_feedback": { - "choice": GIVE_FEEDBACK_CHOICE, - "schema": build_feedback_tool([]), - }, - # Internal, mode-gated (never user-toggleable). The empty-properties placeholder - # is overridden per-turn via schema_overrides with build_direction_note_tool(direction_note_ - # fragments); registering it here emits its bytes into the shared blob so the - # direction-note step reuses the cached base. - "record_direction_note": { - "choice": RECORD_DIRECTION_NOTE_CHOICE, - "schema": build_direction_note_tool([]), - }, - # Internal, flag-gated (never user-toggleable). Enabled for the turn when the - # Agentic Lorebook feature is active (see _build_writer_tools_blob); its fixed - # schema rides the shared blob so the select step reuses the cached base. The - # selectable catalog rides the select step's OOC trailing, not this schema. - "select_lorebook": { - "choice": SELECT_LOREBOOK_CHOICE, - "schema": SELECT_LOREBOOK_TOOL, - }, - # Internal, flag-gated (never user-toggleable). Enabled for the turn when the - # conversation's linked World has Dynamic Worlds on (see _build_writer_tools_blob); - # its fixed schema rides the shared blob so the post-turn proposal step reuses - # the cached base. The catalog of existing entries rides that step's OOC trailing. - "propose_world_changes": { - "choice": PROPOSE_WORLD_CHANGES_CHOICE, - "schema": PROPOSE_WORLD_CHANGES_TOOL, - }, -} - -# Built-in tool names declared as a literal and asserted equal to TOOLS keys at -# module load so the two cannot drift silently if a contributor edits one -# without the other. -BUILTIN_TOOL_NAMES: frozenset[str] = frozenset( - { - "direct_scene", - "editor_apply_patch", - "editor_rewrite", - "give_feedback", - "propose_world_changes", - "record_direction_note", - "select_lorebook", - } -) -assert BUILTIN_TOOL_NAMES == frozenset(TOOLS.keys()), "BUILTIN_TOOL_NAMES drift vs TOOLS literal keys" - -# Built-in tools partitioned into two sets so the director's interactive loop knows -# which tools it may offer. PRE = the director loop's own tools (it iterates these -# and calls them itself). POST = everything else: the post-writer editor tools AND -# the internal forced-step tools that ride the shared per-turn blob (Invariant 3) -# but must NOT be offered to or triggered by the director loop — give_feedback -# (post-writer feedback step), record_direction_note (its own step, pre- or -# post-writer), select_lorebook (the pre-writer agentic-lorebook select step), and -# propose_world_changes (the post-turn Dynamic Worlds proposal step). -# So "POST" here means "not a director-loop tool," not a literal pipeline phase. -PRE_WRITER_TOOLS = {"direct_scene"} -POST_WRITER_TOOLS = { - "editor_apply_patch", - "editor_rewrite", - "give_feedback", - "propose_world_changes", - "record_direction_note", - "select_lorebook", -} - -assert PRE_WRITER_TOOLS.isdisjoint(POST_WRITER_TOOLS), "phase sets overlap" -assert PRE_WRITER_TOOLS | POST_WRITER_TOOLS == BUILTIN_TOOL_NAMES, "phase sets must partition built-ins" - -# Tools registered with standalone=True are filtered out of the schemas array -# returned by enabled_schemas(). They remain reachable via direct tool_choice -# calls. -STANDALONE_TOOLS: set[str] = set() - - -def register_tool(name: str, schema: dict, choice: dict, *, standalone: bool = False) -> None: - """Register or replace a tool in the registry.""" - TOOLS[name] = {"schema": schema, "choice": choice} - if standalone: - STANDALONE_TOOLS.add(name) - else: - STANDALONE_TOOLS.discard(name) - - -def enabled_schemas( - enabled_tools: Mapping[str, bool] | None, - overrides: Mapping[str, dict] | None = None, -) -> list[dict]: - """Return schemas for enabled, non-standalone tools in registry order. - - ``enabled_tools=None`` returns every non-standalone schema. A dict - filters to entries with a truthy value. ``overrides`` replaces named - schemas with dynamic variants (e.g. the per-turn ``give_feedback`` - schema); an override value of ``None`` drops that tool from the result. - """ - overrides = overrides or {} - eligible = [n for n in TOOLS if n not in STANDALONE_TOOLS] - if enabled_tools is not None: - eligible = [n for n in eligible if enabled_tools.get(n, False)] - return [s for n in eligible if (s := overrides.get(n, TOOLS[n]["schema"])) is not None] - - -def strictify_schema(schema: dict) -> dict: - """Copy *schema* into OpenAI strict-mode shape, recursively. - - Strict structured output requires every object to list all properties in - ``required`` and set ``additionalProperties: false``. Originally-optional - properties are made nullable so "may omit" survives as "may be null" -- - the passes' unpackers already discard empty/null argument values. - """ - node = dict(schema) - props = node.get("properties") - if isinstance(props, dict): - required = set(node.get("required") or []) - out_props: dict = {} - for key, prop in props.items(): - sub = strictify_schema(prop) if isinstance(prop, dict) else prop - if key not in required and isinstance(sub, dict) and "type" in sub: - t = sub["type"] - if isinstance(t, list): - t = t if "null" in t else [*t, "null"] - elif t != "null": - t = [t, "null"] - sub = {**sub, "type": t} - out_props[key] = sub - node["properties"] = out_props - node["required"] = list(props.keys()) - node["additionalProperties"] = False - if isinstance(node.get("items"), dict): - node["items"] = strictify_schema(node["items"]) - return node diff --git a/backend/workflows/_forced_call.py b/backend/workflows/_forced_call.py index 54b91409..c1959d14 100644 --- a/backend/workflows/_forced_call.py +++ b/backend/workflows/_forced_call.py @@ -9,14 +9,12 @@ from ..core import ReasoningChannel, agent_lane_max_tokens, mark_call_start from ..inference import ( - STANDALONE_TOOLS, - TOOLS, - enabled_schemas, honors_forced_tool_choice, note_forced_tool_choice_ignored, parse_tool_calls, reasoning_cfg, ) +from ..prompting.tool_catalog import enabled_schemas, is_standalone_tool, require_tool logger = logging.getLogger(__name__) @@ -67,7 +65,8 @@ async def forced_tool_call( constant: a forced call fills a schema, so a roleplay preset would only add flourish to it -- the same split ``features.cards._drafting`` documents. """ - schema = TOOLS[tool_name]["schema"] + tool = require_tool(tool_name) + schema = tool["schema"] resolved_model = model_name or settings["model_name"] reasoning_params = reasoning_cfg(reasoning_on) base_url = getattr(client, "base_url", "") @@ -99,7 +98,7 @@ async def forced_tool_call( # and the loss where it doesn't is bounded to the blob -- a few hundred # tokens per image, not a prefix bust. Do not infer from a working forced # call that the sibling reuse is happening. - tools = [TOOLS[n]["schema"] for n in offer_tools] + tools = [require_tool(name)["schema"] for name in offer_tools] if schema not in tools: tools.append(schema) # ...unless the wire won't carry the forcing. Then a rival schema in the @@ -117,7 +116,7 @@ async def forced_tool_call( overrides_arg = _plain(schema_overrides) if schema_overrides else None tools = list(enabled_schemas(dict(enabled_tools), overrides_arg)) canonical = (overrides_arg or {}).get(tool_name, schema) - if canonical is not None and (tool_name in STANDALONE_TOOLS or canonical not in tools): + if canonical is not None and (is_standalone_tool(tool_name) or canonical not in tools): tools.append(canonical) messages = [_plain(m) for m in prefix] + [_plain(m) for m in tail_messages] @@ -143,7 +142,7 @@ async def _attempt(tool_array: list[dict]) -> AsyncIterator[dict]: messages=messages, model=resolved_model, tools=tool_array, - tool_choice=TOOLS[tool_name]["choice"], + tool_choice=tool["choice"], temperature=temperature, max_tokens=agent_lane_max_tokens(settings, floor=token_floor), tools_in_prompt=tools_in_prompt, diff --git a/backend/workflows/attachment_cache.py b/backend/workflows/attachment_cache.py index 0556036f..6cc4c4ab 100644 --- a/backend/workflows/attachment_cache.py +++ b/backend/workflows/attachment_cache.py @@ -830,7 +830,7 @@ async def delete_workflow_attachments( "UPDATE workflow_attachments SET parent_attachment_id = ? WHERE parent_attachment_id = ? AND id != ?", (new_root, root_id, new_root), ) - # Only a root row's annotation reaches the LLM prefix (prompt_builder), so + # Only a root row's annotation reaches the LLM prefix (prompting.base), so # the promoted root inherits the deleted root's annotation; otherwise # deleting the root variant would silently change the message's # model-visible text. diff --git a/backend/workflows/contracts.py b/backend/workflows/contracts.py index eaf3c037..cc3196ea 100644 --- a/backend/workflows/contracts.py +++ b/backend/workflows/contracts.py @@ -36,7 +36,7 @@ def _readonly(obj: Any) -> Any: @dataclass class ToolSpec: - """A tool a workflow contributes to the global tool registry. + """A tool a workflow contributes to the global tool catalog. ``name`` must equal ``schema["function"]["name"]``. ``choice`` is the pre-built ``tool_choice`` payload (almost always diff --git a/backend/workflows/registry.py b/backend/workflows/registry.py index de375b10..3d5d3e50 100644 --- a/backend/workflows/registry.py +++ b/backend/workflows/registry.py @@ -31,11 +31,10 @@ from ..database import ( set_workflow_state as _db_set_workflow_state, ) -from ..inference import ( +from ..prompting.tool_catalog import ( BUILTIN_TOOL_NAMES, - STANDALONE_TOOLS, - TOOLS, register_tool, + remove_tool, ) from .contracts import HookType, ToolSpec @@ -146,8 +145,7 @@ def register_workflow(w: Workflow) -> None: register_tool(spec.name, spec.schema, spec.choice, standalone=spec.standalone) for orphan in old_tool_names - new_tool_names: - TOOLS.pop(orphan, None) - STANDALONE_TOOLS.discard(orphan) + remove_tool(orphan) _WORKFLOWS_BY_ID[w.id] = w diff --git a/backend/workflows/toolkit.py b/backend/workflows/toolkit.py index 5e3fc3b0..7a0d59c9 100644 --- a/backend/workflows/toolkit.py +++ b/backend/workflows/toolkit.py @@ -40,19 +40,15 @@ resolve_char_context, ) from ..inference import ( - STANDALONE_TOOLS, - TOOLS, LLMClient, - build_prefix, - compute_constant_lorebook_block, - enabled_schemas, - format_message_with_attachments, local_ml, - macro_identity, parse_tool_calls, reasoning_cfg, separate_agent_lane_configured, ) +from ..prompting import build_prefix, format_message_with_attachments, macro_identity +from ..prompting.lorebook import compute_constant_lorebook_block +from ..prompting.tool_catalog import STANDALONE_TOOLS, TOOLS, enabled_schemas from ._forced_call import forced_tool_call from .attachment_cache import EVICTED_MARKER, insert_workflow_attachment from .registry import ( diff --git a/docs/architecture/dynamic-worlds.md b/docs/architecture/dynamic-worlds.md index da9f8d89..a46d602f 100644 --- a/docs/architecture/dynamic-worlds.md +++ b/docs/architecture/dynamic-worlds.md @@ -31,7 +31,7 @@ delete its overlay; an orphaned replacement becomes standalone lore. ### The effective view -Callers must use `inference/lorebook.select_effective_entries`, never the raw +Callers must use `prompting/lorebook.select_effective_entries`, never the raw pool. It: 1. removes disabled and archived rows; diff --git a/docs/architecture/group-chats.md b/docs/architecture/group-chats.md index ac9a2171..619114c2 100644 --- a/docs/architecture/group-chats.md +++ b/docs/architecture/group-chats.md @@ -52,7 +52,7 @@ the setting **Character context**. | `shared` | A dossier for every member, including card text and examples | That member's post-history instructions | | `swap` | Every member's public profile plus the active member's card text and examples | That member's post-history instructions | -`backend/inference/group_context.py` owns this projection. Prompt construction +`backend/prompting/group_context.py` owns this projection. Prompt construction and context-size reporting use it rather than deciding card visibility locally. The following rules apply in every mode: diff --git a/docs/architecture/kv-cache.md b/docs/architecture/kv-cache.md index f77b44af..a2d5e335 100644 --- a/docs/architecture/kv-cache.md +++ b/docs/architecture/kv-cache.md @@ -49,6 +49,11 @@ The system prompt and history are assembled once in `backend/inference/cached_call.py`. Passes call `base.complete(...)`; they do not assemble their own prefix. +Deterministic message and context construction lives in `backend/prompting/`. +Pass-specific instruction prose lives beside its pass under +`backend/pipeline/passes/`; provider and cache execution remain in +`backend/inference/`. + The system prompt includes stable card, persona, scenario, constant lore, and scene instructions. History is shared byte-for-byte, including attachment encoding. A macro that changes those bytes, such as an unseeded `{{roll}}`, @@ -79,6 +84,11 @@ not call a tool. The pass selects its behavior with `tool_choice`: - Writer uses `tool_choice="none"` so it writes prose. - Workflow tools add their schemas to the same per-turn list. +`backend/prompting/tool_catalog.py` owns the list. Built-ins are emitted in the +explicit `BUILTIN_TOOL_ORDER`; workflow tools append in registration order, and +re-registering one preserves its position. Schema property order and compact, +insertion-order-preserving JSON bytes are part of the cache contract. + Inference servers may render only the forced tool, or no tools for `none`. As a result, the three passes can share the conversation body without sharing the entire rendered prefix. Think in **cache lanes**: each distinct rendered shape diff --git a/docs/architecture/prompting.md b/docs/architecture/prompting.md new file mode 100644 index 00000000..524a15a7 --- /dev/null +++ b/docs/architecture/prompting.md @@ -0,0 +1,58 @@ +# Prompting boundary + +`backend/prompting/` owns deterministic, provider-independent model-facing +construction. A module belongs there only when it: + +1. deterministically transforms caller-supplied data; +2. constructs model-facing content or a canonical projection required by it; +3. is shared by multiple upper-layer consumers or the stable workflow toolkit; +4. performs no I/O, model call, persistence, or feature/pass enablement decision. + +Instruction prose and result contracts used by one pipeline pass stay with that +pass. Feature enablement stays in its feature slice. Provider adaptation, +retries, structured-output normalization, cache mechanics, and local runtimes +stay in `inference/`. + +## Allowed dependency graph + +```text +api -> pipeline, features, workflows, prompting, inference, analysis, database, core +pipeline -> features, workflows, prompting, inference, analysis, database, core +features -> prompting, inference, analysis, database, core +workflows -> prompting, inference, analysis, database, core +prompting -> core +inference -> core +analysis -> database, core +database -> core +core -> (nothing) +``` + +`features/` and `workflows/` are siblings and do not import one another. +Feature slices do not import peer slices. The exact matrix is enforced by +`scripts/check_backend_layers.py`; every Python-bearing top-level backend +package must be classified there. + +## Byte and order contracts + +Prompt text, whitespace, delimiters, message order, parameter order, macro +resolution timing, seeds, and stored random choices are public behavior for +this refactor and must remain byte-for-byte stable. + +Built-in tools use this order: + +```python +( + "direct_scene", + "editor_apply_patch", + "editor_rewrite", + "give_feedback", + "record_direction_note", + "select_lorebook", + "propose_world_changes", +) +``` + +Enabled built-ins preserve that order. Workflow tools append in registration +order, and re-registering a workflow tool preserves its position. Schema +property order and insertion-order-preserving JSON serialization are part of +the transport contract. diff --git a/docs/architecture/secondary-workflow.md b/docs/architecture/secondary-workflow.md index e1c5ac83..57cf87a7 100644 --- a/docs/architecture/secondary-workflow.md +++ b/docs/architecture/secondary-workflow.md @@ -30,6 +30,7 @@ chrome. The workflow owns its feature logic. | `backend/workflows/registry.py` | Workflow records, subscriptions, lookups, and state access | | `backend/workflows/contracts.py` | Hook types, context dataclasses, and `ToolSpec` | | `backend/workflows/toolkit.py` | Stable imports for workflow authors | +| `backend/prompting/tool_catalog.py` | Ordered tool lookup and workflow-tool registration | | `backend/workflows/attachment_cache.py` | Attachment storage, variants, budget, and eviction | | `backend/workflows/__init__.py` | Built-in registration and hook subscriptions | | `backend/pipeline/workflow_bridge.py` | Pipeline hook dispatch and attachment staging | @@ -71,6 +72,11 @@ Workflow( Tool names must be unique and must agree across `ToolSpec.name`, the schema, and `tool_choice`. +Workflow tools append after the fixed built-in tool order. Re-registering an +existing tool replaces its contract without changing its position; removing a +tool on workflow replacement removes it through the catalog API. The toolkit's +`TOOLS` and `STANDALONE_TOOLS` exports are live, read-only compatibility views. + Registration follows this shape: ```python diff --git a/docs/plans/inference-layer-audit.md b/docs/plans/inference-layer-audit.md deleted file mode 100644 index 219d7445..00000000 --- a/docs/plans/inference-layer-audit.md +++ /dev/null @@ -1,370 +0,0 @@ -# Inference boundary refactor implementation plan - -Status: ready for implementation. This document update changes no production -code. - -## 1. Outcome - -Create a lower shared `backend/prompting/` package for deterministic, -provider-independent model-facing construction, then move product policy out of -`backend/inference/`. - -The resulting ownership is: - -- `inference/`: model execution, provider adaptation, retries and errors, - structured-output normalization, cache-call mechanics, cache diagnostics, and - local model runtimes; -- `prompting/`: shared prompt-context projections, message construction, tool - schemas, and deterministic ordered tool composition; -- `pipeline/passes/`: instruction prose and result contracts owned by a - pipeline pass; -- `features/`: feature enablement and feature-specific adapters; -- `analysis/`: text interpretation and measurement. - -A module belongs in `prompting/` only when it: - -1. deterministically transforms caller-supplied data; -2. constructs model-facing content or a canonical projection required by that - content; -3. is shared by multiple upper-layer consumers or by the stable workflow - toolkit; and -4. performs no I/O, model call, persistence, or feature/pass enablement decision. - -### Enforced dependency graph - -```text -api -> pipeline, features, workflows, prompting, inference, analysis, database, core -pipeline -> features, workflows, prompting, inference, analysis, database, core -features -> prompting, inference, analysis, database, core -workflows -> prompting, inference, analysis, database, core -prompting -> core -inference -> core -analysis -> database, core -database -> core -core -> (nothing) -``` - -`features/` and `workflows/` are sibling composition inputs and do not import -each other. Feature slices remain isolated from peer feature slices. - -## 2. Planned source moves - -| Current surface | Implemented destination | -|---|---| -| autocomplete `local_ml.build_prompt` and the autocomplete-specific `complete` whitespace adapter | `features/autocomplete/` | -| `format_message_with_attachments`, `group_speaker_label`, and `build_prefix` | `prompting/base.py` | -| all of `inference/group_context.py` | `prompting/group_context.py` | -| `resolve_mood_fragment_randoms`, `compute_style_injection_block`, and `build_style_injection` | `prompting/scene_direction.py` | -| lorebook effective projection, catalog, selection, rendering, scan-depth contracts, and block builders | `prompting/lorebook.py` | -| `agentic_lorebook_active` | `features/lorebook/` | -| Director and Agentic Lorebook instruction builders | `pipeline/passes/director/prompts.py` | -| direction-note instruction and block rendering | `pipeline/passes/director/direction_note_prompts.py` | -| Editor and feedback instruction builders | `pipeline/passes/editor/prompts.py` | -| Dynamic Worlds instruction builder | `pipeline/passes/world_change_prompt.py` | -| shared tool-call instruction formatting and reasoning guidance | `pipeline/passes/_prompting.py` | -| concrete built-in schemas, choices, and dynamic schema builders | `prompting/tool_schemas.py` | -| ordered tool lookup, filtering, registration, and removal | `prompting/tool_catalog.py` | -| `strictify_schema` | `inference/schema.py` | -| Director-loop tool membership | `pipeline/tools.py` as `DIRECTOR_LOOP_TOOL_NAMES` | - -`features/lorebook/__init__.py` remains the feature facade. Feature callers use -that facade; lower consumers such as `workflows/toolkit.py` import -`prompting.lorebook` directly. - -The context-size route imports shared group and Scene Direction projections from -`prompting/`. It does not depend on private pass modules or expand the -`pipeline` facade for read-only formatting helpers. - -## 3. Preserved contracts - -Every relocation preserves: - -- HTTP request and response bodies; -- SSE event names, ordering, and payload shapes; -- database schema and stored JSON shapes; -- prompt text, whitespace, delimiters, message order, and parameter order; -- macro resolution timing, seeds, and stored random choices; -- tool names, descriptions, schemas, property order, choice objects, and final - array order; -- the names and signatures exported by `backend.workflows.toolkit`. - -Repository consumers migrate atomically with each move. Old -`backend.inference` re-exports are removed in the same phase so the final -architecture contains no `inference -> prompting` dependency. - -### Ordered tool contract - -The built-in order becomes an explicit tuple: - -```python -BUILTIN_TOOL_ORDER = ( - "direct_scene", - "editor_apply_patch", - "editor_rewrite", - "give_feedback", - "record_direction_note", - "select_lorebook", - "propose_world_changes", -) -``` - -`BUILTIN_TOOL_NAMES` is derived from that tuple. Enabled built-ins are emitted -in tuple order. Workflow tools append in workflow registration order. -Re-registering an existing workflow tool preserves its position. - -`prompting.tool_catalog` owns mutable catalog state. Production consumers use -catalog operations for lookup, registration, and removal; workflow replacement -does not mutate catalog dictionaries or standalone sets directly. -`workflows.toolkit` retains `TOOLS` and `STANDALONE_TOOLS` as read-only live -views for compatibility. Test fixtures restore catalog state through -catalog-owned operations instead of calling `clear()`, `update()`, `pop()`, -or `discard()` on those views. - -Tool-order verification uses the raw request body captured at the HTTP transport -boundary for both OpenAI-compatible and Anthropic requests. Cached-base -verification also compares insertion-order-preserving serialization: - -```python -json.dumps(tools, separators=(",", ":"), ensure_ascii=False) -``` - -## 4. Implementation sequence - -Each phase is one reviewable commit and must satisfy its acceptance checks -before the next phase starts. - -### Phase 0 — make the target architecture executable - -1. Create `docs/architecture/prompting.md` with the ownership test, allowed - dependency graph, tool-order contract, and prompt-byte contract. -2. Update `AGENTS.md` with the same layer order and link responsibilities to - the architecture note. -3. Replace numeric ranks in `scripts/check_backend_layers.py` with one explicit - allowed-edge matrix containing `prompting`. -4. Make the checker fail when a Python-bearing top-level `backend/` package is - absent from the matrix. -5. Preserve the feature peer-slice rule in the shared checker. -6. Replace the duplicate policy in `tests/unit/test_import_layering.py` with - calls and focused negative cases against the shared checker. -7. Keep `tests/unit/test_backend_layers.py` as the repository-wide execution - test for that checker. - -Acceptance: - -- the current backend passes the shared AST checker; -- fixtures prove rejection of `inference -> prompting`, - `prompting -> inference`, `features -> workflows`, - `workflows -> features`, peer feature-slice imports, and unclassified - Python packages; -- the script and both test modules enforce the same matrix. - -### Phase 1 — pin behavior before relocation - -Add or strengthen exact-output tests for: - -- solo prefixes and all group-context modes; -- user attachments and workflow attachment annotations; -- constant, depth, keyword, Agentic, replacement, and suppression lore paths; -- combined and per-fragment Director prompts; -- lorebook selection, feedback, direction-note, world-change, and Editor - prompts; -- Scene Direction rendering, progressive fields, disabled Director state, and - stored-random mutation; -- autocomplete prompts with blank summaries, named group messages, truncation, - whitespace-ending drafts, and non-whitespace-ending drafts; -- the complete built-in tools blob; -- dynamic schema overrides, workflow registration order, workflow replacement, - orphan-tool removal, standalone tools, and forced `offer_tools` calls. - -Capture the actual request body for representative OpenAI-compatible and -Anthropic tool calls. Assert byte equality, including object-key and array -order. Keep the runtime KV tracker assertions as a second witness rather than -the byte oracle. - -Acceptance: - -- every moved behavior listed in Section 2 has characterization coverage at its - current location; -- the tool tests fail on either schema-key reordering or tool-array reordering; -- existing expected prompt and tool fixtures remain unchanged. - -### Phase 2 — extract autocomplete as one feature - -1. Add `backend/features/autocomplete/__init__.py` and its implementation - module. -2. Move `local_ml.build_prompt` into the feature. -3. Move `local_ml.complete` into the feature, preserving its trailing - `rstrip`/conditional `lstrip` behavior. -4. Call `inference.local_ml.acomplete("autocomplete", ...)` as the lower - runtime primitive. -5. Update `api/routes/messages.py`, package imports, and autocomplete tests. -6. Remove the `local_ml.py` executable self-check block after autocomplete and - POV assertions are covered by unit tests. - -Acceptance: - -- the API returns the same completion for whitespace and non-whitespace tails; -- `inference/local_ml.py` contains only generic load, completion, scoring, and - classification runtime contracts. - -### Phase 3 — add shared prompt projections - -1. Add `backend/prompting/__init__.py`, `base.py`, - `group_context.py`, and `scene_direction.py`. -2. Move prefix/message construction to `prompting/base.py`. -3. Move the complete group-context projection to - `prompting/group_context.py`. -4. Move Scene Direction projection and stored-random resolution to - `prompting/scene_direction.py`. -5. Update pipeline context construction, Writer/Director composition, - summarization, context-size reporting, workflow toolkit imports, tests, and - package facades. -6. Remove `roster_names` from the public package facade; keep it private to - `prompting.group_context`. -7. Update `docs/architecture/group-chats.md`, `kv-cache.md`, and - `secondary-workflow.md`. -8. Delete `inference/group_context.py` after all imports migrate. - -Acceptance: - -- prefix, attachment, group, Scene Direction, and context-size fixtures are - unchanged; -- `prompting/` imports only `core/`; -- `backend.workflows.toolkit` retains its existing public names and - signatures. - -### Phase 4 — move lorebook projection as one unit - -1. Add `prompting/lorebook.py`. -2. Move the effective-layer projection, keyword and Director selection, - catalog construction, rendering, scan-depth constants, and every lorebook - block builder into it. -3. Move `agentic_lorebook_active` into `features/lorebook/`. -4. Update the lorebook feature facade, proposals, pipeline context and passes, - API effective views, workflow toolkit, tests, and package exports. -5. Update database docstrings and - `docs/architecture/dynamic-worlds.md` to the new canonical module. -6. Delete `inference/lorebook.py` after all imports migrate. - -Acceptance: - -- lore selection and rendering have one implementation in - `prompting.lorebook`; -- constant, depth, keyword, Agentic, replacement, suppression, and off-turn - outputs match Phase 1; -- the feature gate is reached through `features.lorebook`; -- lower consumers import `prompting.lorebook`, not the feature facade. - -### Phase 5 — co-locate pass instruction contracts - -1. Add `pipeline/passes/_prompting.py` for shared tool-call instruction - formatting and reasoning guidance. -2. Add `pipeline/passes/director/prompts.py` for combined/per-fragment - Director prompts and Agentic Lorebook selection prompts. -3. Add `pipeline/passes/director/direction_note_prompts.py` for direction-note - instructions and block rendering. -4. Add `pipeline/passes/editor/prompts.py` for Editor and feedback - instructions. -5. Add `pipeline/passes/world_change_prompt.py` for the Dynamic Worlds - instruction. -6. Update each pass to import its local contract and update focused tests. -7. Remove moved names from `inference/__init__.py`. -8. Delete `inference/prompt_builder.py` after all repository consumers migrate. - -Acceptance: - -- all Phase 1 prompt strings remain unchanged; -- each instruction builder is imported only by its owning pass and tests; -- `inference/` contains no Orb-domain instruction prose. - -### Phase 6 — separate tool contracts from transport - -1. Add `inference/schema.py`, move `strictify_schema` into it, and update both - provider adapters and tests. -2. Add `prompting/tool_schemas.py` and move all built-in schema/choice objects - and dynamic schema builders into it. -3. Add `prompting/tool_catalog.py` with: - - `BUILTIN_TOOL_ORDER` and derived `BUILTIN_TOOL_NAMES`; - - catalog lookup and membership operations; - - `enabled_schemas` with its existing signature; - - workflow tool registration and removal operations; - - read-only live compatibility views for `TOOLS` and - `STANDALONE_TOOLS`. -4. Build the initial catalog from `BUILTIN_TOOL_ORDER`; built-in population - uses no import-time registration side effects. -5. Update workflow replacement to remove orphaned tools through the catalog - operation. -6. Update workflow test fixtures to restore registration state through catalog - operations. -7. Add `pipeline/tools.py` with - `DIRECTOR_LOOP_TOOL_NAMES = frozenset({"direct_scene"})`, replace - `PRE_WRITER_TOOLS` consumers, and remove `POST_WRITER_TOOLS` and its - partition assertion. -8. Update pipeline configuration, pass prompt lookups, settings filtering, - forced-call helpers, document audit, workflow toolkit, tests, docstrings, and - comments to the new owners and terminology. -9. Delete `inference/tool_registry.py` after all imports migrate. - -Acceptance: - -- built-in tool bytes equal the Phase 1 baseline at the cached-base and raw - transport boundaries; -- dynamic override, registration, replacement, orphan-removal, standalone, and - forced-offer paths preserve their Phase 1 order and bytes; -- production code mutates tool-catalog state only through catalog operations; -- toolkit compatibility views reject direct mutation; -- provider strictification output is unchanged. - -### Phase 7 — finish facades and documentation - -1. Restrict `inference/__init__.py` to model/runtime contracts. -2. Give `prompting/__init__.py` a small facade for genuinely shared contracts; - specialized consumers import canonical submodules. -3. Remove all repository imports of: - - `inference.prompt_builder`; - - `inference.group_context`; - - `inference.lorebook`; - - `inference.tool_registry`; - - moved names from the flat `inference` facade. -4. Update architecture documents, test descriptions, code comments, and - docstrings that identify the previous owners. -5. Run the dependency checker and the full validation suite. - -Acceptance: - -- `inference/` has no dependency on `prompting/` or an upper layer; -- `prompting/` has no dependency outside `core/`; -- one AST checker is the source of truth for backend dependency edges; -- every moved source module has been deleted; -- no compatibility re-export recreates the old dependency boundary. - -## 5. Validation - -Run focused checks after each phase: - -| Phase | Required focused checks | -|---|---| -| 0 | backend layer checker, `test_backend_layers.py`, `test_import_layering.py` | -| 1 | all characterization tests added or strengthened in the phase | -| 2 | autocomplete and local-ML unit/integration tests | -| 3 | group context, prefix, attachment, Scene Direction, context-size, off-turn prefix, and KV entry-point tests | -| 4 | lore matching, Agentic Lorebook, Dynamic Worlds, constant/depth lore, off-turn prefix, and API effective-view tests | -| 5 | interactive fragments, Director, lore selection, feedback, direction notes, Editor, world proposal, and KV invariant tests | -| 6 | tool catalog, provider structured calls, workflow registry, forced calls, offer-tools blob, KV invariant, and real-stack tests | -| 7 | all affected tests above plus import and layer checks | - -For Phases 3–6, compare the Phase 1 prompt/tool fixtures before accepting the -commit. A relocation commit produces no fixture change. - -Final validation from the repository root: - -```sh -./scripts/format_backend.sh -./scripts/format_frontend.sh -./scripts/lint.sh -./scripts/tests.sh all -``` - -Review the final diff after formatting. The refactor is complete when all -acceptance checks pass and the public HTTP, SSE, persistence, frontend, and -workflow-toolkit contracts remain unchanged. diff --git a/scripts/check_backend_layers.py b/scripts/check_backend_layers.py index 6242fdbe..b561be50 100644 --- a/scripts/check_backend_layers.py +++ b/scripts/check_backend_layers.py @@ -1,18 +1,17 @@ #!/usr/bin/env python3 -"""Backend layering guardrail — the import direction AGENTS.md describes. +"""Backend layering guardrail — the import graph AGENTS.md describes. -The layer stack is a convention, and until now nothing enforced it: Ruff and -Pyright are both perfectly happy with ``inference/`` reaching up into -``features/``. This parses every backend module's imports, resolves the -relative ones, and fails on an edge that points the wrong way. +This parses every backend module's imports, resolves relative imports, and +fails on an edge that is absent from the explicit allowed-edge matrix. -Two rules: +Three rules: - 1. **Rank order.** Every top-level backend package has a rank; a module may - import its own rank or lower. ``database`` may import ``core``; ``api`` - may import anything; ``inference`` may import neither ``features`` nor - ``pipeline``. - 2. **Slices never import peers.** ``features/`` may not import + 1. **Explicit edges.** Each top-level Python package has a complete set of + backend packages it may import. Same-package imports are always allowed. + 2. **Every layer is classified.** A new Python-bearing top-level package or + module must be classified rather than silently becoming a composition + root. + 3. **Slices never import peers.** ``features/`` may not import ``features/``. A slice is self-contained by definition — a peer edge is how two features quietly become one. @@ -33,49 +32,49 @@ ROOT = Path(__file__).resolve().parent.parent BACKEND = ROOT / "backend" -# Lower number = lower layer. A module may import its own rank or lower. -# `analysis` and `inference` are peers by design: neither imports the other. -RANKS = { - "core": 0, - "database": 1, - "analysis": 2, - "inference": 2, - "workflows": 3, - "features": 4, - "pipeline": 5, - "api": 6, +# The source of truth for cross-package imports. Same-package imports are +# allowed implicitly. Features and workflows deliberately remain siblings. +ALLOWED_EDGES: dict[str, frozenset[str]] = { + "core": frozenset(), + "database": frozenset({"core"}), + "inference": frozenset({"core"}), + "prompting": frozenset({"core"}), + "analysis": frozenset({"database", "core"}), + "workflows": frozenset({"prompting", "inference", "analysis", "database", "core"}), + "features": frozenset({"prompting", "inference", "analysis", "database", "core"}), + "pipeline": frozenset({"features", "workflows", "prompting", "inference", "analysis", "database", "core"}), + "api": frozenset({"pipeline", "features", "workflows", "prompting", "inference", "analysis", "database", "core"}), } -#: backend/main.py and backend/__init__.py are the composition root; they sit -#: above everything and are ranked accordingly. -ROOT_RANK = max(RANKS.values()) +ROOT_ALLOWED = frozenset(ALLOWED_EDGES) +ROOT_LAYER = "root" -def _module_parts(path: Path) -> list[str]: +def _module_parts(path: Path, *, root: Path = ROOT) -> list[str]: """``backend/api/routes/local_ml.py`` -> ``['backend', 'api', 'routes', 'local_ml']``.""" - rel = path.relative_to(ROOT).with_suffix("") + rel = path.relative_to(root).with_suffix("") parts = list(rel.parts) if parts[-1] == "__init__": parts.pop() return parts -def _exists(parts: list[str]) -> bool: +def _exists(parts: list[str], *, root: Path = ROOT) -> bool: """Whether *parts* names a real module or package under the repo.""" - base = ROOT.joinpath(*parts) + base = root.joinpath(*parts) return base.with_suffix(".py").is_file() or (base / "__init__.py").is_file() -def _package_parts(path: Path) -> list[str]: +def _package_parts(path: Path, *, root: Path = ROOT) -> list[str]: """The package a file lives in — the base a relative import counts up from. The same for ``cards/parsing.py`` and ``cards/__init__.py``: an ``__init__`` IS its package, so deriving this from the module path would count one level too many and report every intra-slice import as a peer edge. """ - return list(path.relative_to(ROOT).parent.parts) + return list(path.relative_to(root).parent.parts) -def _targets(node: ast.AST, package: list[str]) -> list[list[str]]: +def _targets(node: ast.AST, package: list[str], *, root: Path = ROOT) -> list[list[str]]: """Every backend module *node* imports, as absolute part lists. A ``from .. import database`` resolves to the package ``backend``, and the @@ -102,7 +101,7 @@ def _targets(node: ast.AST, package: list[str]) -> list[list[str]]: out.append(base) for alias in node.names: # `from .. import database` — the name is the module candidate = [*base, alias.name] - if _exists(candidate) and candidate not in out: + if _exists(candidate, root=root) and candidate not in out: out.append(candidate) return out @@ -111,33 +110,68 @@ def _slice_of(parts: list[str]) -> tuple[str, str] | None: """``('features', 'cards')`` for a backend module, or ``None`` for anything else.""" if len(parts) < 2 or parts[0] != "backend": return None + if len(parts) == 2 and parts[1] == "main": + return ROOT_LAYER, "" return parts[1], (parts[2] if len(parts) > 2 else "") -def check() -> list[str]: +def _python_packages(backend: Path) -> set[str]: + return { + path.name + for path in backend.iterdir() + if path.is_dir() and any("__pycache__" not in module.parts for module in path.rglob("*.py")) + } + + +def _unclassified_top_level_modules(backend: Path) -> set[str]: + """Root modules other than the two explicit composition-root modules.""" + return { + path.name + for path in backend.glob("*.py") + if path.name not in {"__init__.py", "main.py"} + } + + +def check(*, root: Path = ROOT, backend: Path | None = None) -> list[str]: + backend = backend or root / "backend" problems: list[str] = [] - for path in sorted(BACKEND.rglob("*.py")): + for package in sorted(_python_packages(backend) - ALLOWED_EDGES.keys()): + problems.append( + f"backend/{package}/: unclassified Python package (add it to ALLOWED_EDGES)" + ) + for module in sorted(_unclassified_top_level_modules(backend)): + problems.append(f"backend/{module}: unclassified top-level Python module") + for path in sorted(backend.rglob("*.py")): if "__pycache__" in path.parts: continue - parts = _module_parts(path) - package = _package_parts(path) + parts = _module_parts(path, root=root) + package = _package_parts(path, root=root) own_layer, own_slice = _slice_of(parts) or ("", "") - own_rank = RANKS.get(own_layer, ROOT_RANK) + allowed = ( + ROOT_ALLOWED + if own_layer in ("", ROOT_LAYER) + else ALLOWED_EDGES.get(own_layer, frozenset()) + ) try: tree = ast.parse(path.read_text(encoding="utf-8")) except SyntaxError as exc: # a file that will not parse is its own failure - problems.append(f"{path.relative_to(ROOT)}: {exc}") + problems.append(f"{path.relative_to(root)}: {exc}") continue for node in ast.walk(tree): if not isinstance(node, ast.Import | ast.ImportFrom): continue - where = f"{path.relative_to(ROOT)}:{node.lineno}" + where = f"{path.relative_to(root)}:{node.lineno}" # One import statement resolves to both the package and the name # beside it (`from ..features import cards`), which is the same # edge said twice; report each layer and each peer slice once. - edges = {e for t in _targets(node, package) if (e := _slice_of(t)) and e[0] in RANKS} - for layer in sorted({layer for layer, _ in edges if RANKS[layer] > own_rank}): - problems.append(f"{where}: {own_layer or 'backend'} imports upward into {layer}/") + edges = { + edge + for target in _targets(node, package, root=root) + if (edge := _slice_of(target)) + and edge[0] in {*ALLOWED_EDGES, ROOT_LAYER} + } + for layer in sorted({layer for layer, _ in edges if layer != own_layer and layer not in allowed}): + problems.append(f"{where}: {own_layer or 'backend'} may not import {layer}") if own_layer == "features": peers = {s for layer, s in edges if layer == "features" and s and s != own_slice} for peer in sorted(peers): @@ -150,7 +184,7 @@ def main() -> int: if problems: print("Backend layer violations:\n - " + "\n - ".join(problems)) return 1 - print(f"Backend layers OK ({len(RANKS)} ranked packages).") + print(f"Backend layers OK ({len(ALLOWED_EDGES)} classified packages).") return 0 diff --git a/tests/integration/_llm_mock.py b/tests/integration/_llm_mock.py index c914692a..6d8dc22d 100644 --- a/tests/integration/_llm_mock.py +++ b/tests/integration/_llm_mock.py @@ -9,7 +9,7 @@ mid-pipeline while another concurrent action arrives. Pass dispatch is by ``tool_choice`` rather than a call counter, because -director may be skipped entirely (gated behind ``has_pre_writer_tools`` +director may be skipped entirely (gated behind ``has_director_loop_tools`` in the orchestrator) and editor iterates multiple times per turn -- a positional scheme would mis-bind queued responses. """ diff --git a/tests/integration/test_autocomplete.py b/tests/integration/test_autocomplete.py index 242cf710..1766dff5 100644 --- a/tests/integration/test_autocomplete.py +++ b/tests/integration/test_autocomplete.py @@ -25,7 +25,7 @@ async def fake_complete(prompt, *a, **k): assert prompt.endswith("I walk into the") # draft is the trailing line return " tavern and look around." - monkeypatch.setattr("backend.inference.local_ml.complete", fake_complete) + monkeypatch.setattr("backend.features.autocomplete.complete", fake_complete) await dbmod.create_conversation("conv-ac2", "Chat", "Nova", "") mid, _ = await dbmod.add_message("conv-ac2", "assistant", "You arrive at the gate.", 0, parent_id=None) await dbmod.set_active_leaf("conv-ac2", mid) @@ -41,7 +41,7 @@ async def test_autocomplete_blank_draft_skips_model(client, monkeypatch): async def boom(*a, **k): raise AssertionError("model must not be called for a blank draft") - monkeypatch.setattr("backend.inference.local_ml.complete", boom) + monkeypatch.setattr("backend.features.autocomplete.complete", boom) await dbmod.create_conversation("conv-ac3", "Chat", "Nova", "") resp = await client.post("/api/conversations/conv-ac3/autocomplete", json={"draft": " "}) @@ -59,7 +59,7 @@ async def fake_complete(prompt, *a, **k): captured["prompt"] = prompt return " toward the fire." - monkeypatch.setattr("backend.inference.local_ml.complete", fake_complete) + monkeypatch.setattr("backend.features.autocomplete.complete", fake_complete) aria = (await client.post("/api/characters", json={"name": "Aria"})).json()["id"] kael = (await client.post("/api/characters", json={"name": "Kael"})).json()["id"] conv = ( @@ -95,7 +95,7 @@ async def fake_complete(prompt, *a, **k): captured["prompt"] = prompt return " ..." - monkeypatch.setattr("backend.inference.local_ml.complete", fake_complete) + monkeypatch.setattr("backend.features.autocomplete.complete", fake_complete) await dbmod.create_conversation("conv-ac-solo-cast", "Chat", "Nova", "") resp = await client.post("/api/conversations/conv-ac-solo-cast/autocomplete", json={"draft": "I ask {{cast}} about"}) diff --git a/tests/integration/test_characters.py b/tests/integration/test_characters.py index 712f037c..7ee89e98 100644 --- a/tests/integration/test_characters.py +++ b/tests/integration/test_characters.py @@ -396,7 +396,7 @@ def _profile_call(**arguments) -> dict: ``_llm_mock._pass_from_tool_choice`` routes any forced tool name it does not recognise as a core pass tool to the ``workflow`` queue, and this schema is - deliberately not in ``inference.tool_registry.TOOLS`` — so this is the queue + deliberately not in ``prompting.tool_catalog.TOOLS`` — so this is the queue the public-profile drafter reads from. """ return {"tool_calls": [{"type": "function", "function": {"name": "draft_public_profile", "arguments": arguments}}]} diff --git a/tests/integration/test_endpoint_transport_passes.py b/tests/integration/test_endpoint_transport_passes.py index 853f5934..d2316450 100644 --- a/tests/integration/test_endpoint_transport_passes.py +++ b/tests/integration/test_endpoint_transport_passes.py @@ -7,14 +7,18 @@ from __future__ import annotations +import hashlib import json from unittest.mock import patch +import httpx import pytest +from backend.inference import anthropic from backend.inference import client as llm_mod from backend.inference import endpoint_profiles as ep from backend.inference.client import LLMClient, parse_tool_calls +from backend.prompting.tool_catalog import BUILTIN_TOOL_ORDER, enabled_schemas def _tool(name: str) -> dict: @@ -177,3 +181,56 @@ async def test_director_writer_editor_calls_cross_protocol_boundary(provider, en assert "tools" not in bodies[1] and "tool_choice" not in bodies[1] else: assert [body["tool_choice"] for body in bodies] == [DIRECTOR, "none", EDITOR] + + +@pytest.mark.parametrize("provider", ["openai", "anthropic"]) +async def test_builtin_tool_order_reaches_raw_http_transport_byte_exact(provider): + """Pin object-key and tool-array order at the actual HTTP request boundary.""" + ep._RESOLVED_ROUTES.clear() + model = "claude-haiku-4-5" if provider == "anthropic" else "openai-model" + endpoint = "https://api.anthropic.com/v1/messages" if provider == "anthropic" else "https://openai.test/v1/chat/completions" + messages = [ + {"role": "system", "content": "system"}, + {"role": "user", "content": "turn"}, + ] + tools = enabled_schemas({name: True for name in BUILTIN_TOOL_ORDER}) + choice = {"type": "function", "function": {"name": "direct_scene"}} + captured: list[str] = [] + + def handler(request: httpx.Request) -> httpx.Response: + captured.append(request.content.decode()) + lines = _anthropic_tool("direct_scene") if provider == "anthropic" else _openai_tool("direct_scene") + return httpx.Response( + 200, + content="\n".join(lines), + headers={"content-type": "text/event-stream"}, + ) + + transport_client = httpx.AsyncClient(transport=httpx.MockTransport(handler)) + client = LLMClient(endpoint, "secret") + with patch.object(llm_mod.httpx, "AsyncClient", lambda *args, **kwargs: transport_client): + async for _ in client.complete( + messages, + model, + tools=tools, + tool_choice=choice, + max_tokens=100, + ): + pass + + openai_body = { + "model": model, + "messages": messages, + "stream": True, + "max_tokens": 100, + "tools": tools, + "tool_choice": choice, + "stream_options": {"include_usage": True}, + } + expected = anthropic.build_request_body(openai_body, endpoint, model) if provider == "anthropic" else openai_body + assert captured == [json.dumps(expected, separators=(",", ":"), ensure_ascii=False)] + expected_bytes = { + "openai": (5064, "1893a6046f145ca17758c4e7f7f86813a47792247d52655e10b8edd351bac5b8"), + "anthropic": (5393, "a73b312db3d6a3bbfb7e7c325c3e15e476ff28ae1102162ea7fa5d65625abc28"), + } + assert (len(captured[0]), hashlib.sha256(captured[0].encode()).hexdigest()) == expected_bytes[provider] diff --git a/tests/integration/test_group_chats.py b/tests/integration/test_group_chats.py index c06465e3..f59d6f1a 100644 --- a/tests/integration/test_group_chats.py +++ b/tests/integration/test_group_chats.py @@ -848,7 +848,7 @@ async def test_compression_never_re_asserts_a_members_sheet_into_the_summary(cli # ── The post-exchange sheet-update pass ───────────────────────────────────────── # One call per member the exchange touched, staged pending, never applied. Routed # through the mock's `workflow` queue for the reason `_profile_call` states: the -# schema is deliberately absent from `inference.tool_registry.TOOLS`. +# schema is deliberately absent from `prompting.tool_catalog.TOOLS`. def _sheet_call(**arguments) -> dict: @@ -1429,7 +1429,7 @@ def _profile_call(**arguments) -> dict: ``_pass_from_tool_choice`` routes any forced tool name it does not recognise as a core pass tool to the ``workflow`` queue, and this schema is - deliberately absent from ``inference.tool_registry.TOOLS``. + deliberately absent from ``prompting.tool_catalog.TOOLS``. """ return {"tool_calls": [{"type": "function", "function": {"name": "draft_public_profile", "arguments": arguments}}]} diff --git a/tests/integration/test_settings.py b/tests/integration/test_settings.py index b5c8af66..d5a1456f 100644 --- a/tests/integration/test_settings.py +++ b/tests/integration/test_settings.py @@ -115,7 +115,7 @@ async def test_update_enabled_tools_json_field(client, db): async def test_enabled_tools_sanitized_to_registered_tools(client, db): # Non-tool keys (the former length_guard* feature flags, or anything else not - # in the tool registry) must never be persisted back into enabled_tools. + # in the tool catalog) must never be persisted back into enabled_tools. resp = await client.put( "/api/settings", json={"enabled_tools": {"direct_scene": True, "length_guard": True, "not_a_tool": True}}, diff --git a/tests/integration/workflows/_fixtures.py b/tests/integration/workflows/_fixtures.py index 8f764222..ff2f5527 100644 --- a/tests/integration/workflows/_fixtures.py +++ b/tests/integration/workflows/_fixtures.py @@ -1,7 +1,7 @@ """Test helpers for workflow hook coverage and workflow_attachments rows. -``register_for_test`` snapshots ``_registry._WORKFLOWS_BY_ID``, ``TOOLS``, -and ``STANDALONE_TOOLS`` with ``deepcopy`` on enter and restores them on +``register_for_test`` snapshots ``_registry._WORKFLOWS_BY_ID`` and the tool +catalog on enter and restores them through catalog-owned operations on exit, so a failed assertion inside the ``with`` block cannot leak registry mutations into adjacent tests. The same ``Workflow`` instance is held by both the test and the registry (see clear at end of @@ -24,7 +24,10 @@ set_workflow_state, ) from backend.database.queries.workflow_attachments import get_workflow_attachment_by_id -from backend.inference import STANDALONE_TOOLS, TOOLS +from backend.prompting.tool_catalog import ( + restore_catalog, + snapshot_catalog, +) from backend.workflows import ( HookType, ToolSpec, @@ -80,15 +83,11 @@ def _restore_registry(): activates it. """ by_id_snapshot = {k: deepcopy(v) for k, v in _registry._WORKFLOWS_BY_ID.items()} - tools_snapshot = {n: dict(spec) for n, spec in TOOLS.items()} - standalone_snapshot = set(STANDALONE_TOOLS) + catalog_snapshot = snapshot_catalog() yield _registry._WORKFLOWS_BY_ID.clear() _registry._WORKFLOWS_BY_ID.update(by_id_snapshot) - TOOLS.clear() - TOOLS.update(tools_snapshot) - STANDALONE_TOOLS.clear() - STANDALONE_TOOLS.update(standalone_snapshot) + restore_catalog(catalog_snapshot) def make_workflow( @@ -147,13 +146,12 @@ def register_for_test(workflow: Workflow, *, finalize: bool = True) -> Iterator[ that exercise the mandate's raise path pass ``finalize=False`` to skip the validation. - On exit: restores the registry, ``TOOLS``, and ``STANDALONE_TOOLS`` to + On exit: restores the workflow registry and tool catalog to a deep-copied snapshot captured before enter so subscription mutations inside the block cannot leak across teardown. """ by_id_snapshot = {k: deepcopy(v) for k, v in _registry._WORKFLOWS_BY_ID.items()} - tools_snapshot = {n: dict(spec) for n, spec in TOOLS.items()} - standalone_snapshot = set(STANDALONE_TOOLS) + catalog_snapshot = snapshot_catalog() register_workflow(workflow) for hook_type, fn, priority in getattr(workflow, "_pending_hooks", []): @@ -165,10 +163,7 @@ def register_for_test(workflow: Workflow, *, finalize: bool = True) -> Iterator[ finally: _registry._WORKFLOWS_BY_ID.clear() _registry._WORKFLOWS_BY_ID.update(by_id_snapshot) - TOOLS.clear() - TOOLS.update(tools_snapshot) - STANDALONE_TOOLS.clear() - STANDALONE_TOOLS.update(standalone_snapshot) + restore_catalog(catalog_snapshot) # register_workflow stores the same Workflow instance the test holds, # so workflow.subscriptions is identity-shared with the registry's # record. Restoring the dict to the deepcopied snapshot above does diff --git a/tests/unit/test_abort_pipeline.py b/tests/unit/test_abort_pipeline.py index e39c8ff1..0f944457 100644 --- a/tests/unit/test_abort_pipeline.py +++ b/tests/unit/test_abort_pipeline.py @@ -97,8 +97,8 @@ async def mock_editor(*args, **kwargs): editor_calls[0] += 1 yield {"type": "done", "draft": "edited"} - # editor_apply_patch is a POST_WRITER_TOOL, so has_pre_writer_tools=False - # (director pass skipped). phrase_bank not None makes do_edit=True. + # editor_apply_patch is not a Director-loop tool, so the Director is skipped. + # phrase_bank being non-None makes do_edit=True. settings = { "model_name": "test", "enable_agent": 1, @@ -181,8 +181,8 @@ async def mock_editor(*args, **kwargs): raise RuntimeError("editor endpoint exploded") yield # pragma: no cover — makes this an async generator - # editor_apply_patch is a POST_WRITER_TOOL → director skipped; phrase_bank - # not None makes do_edit=True so the editor runs over the writer draft. + # editor_apply_patch is not a Director-loop tool, so the Director is skipped; + # phrase_bank being non-None makes do_edit=True over the Writer draft. settings = { "model_name": "test", "enable_agent": 1, diff --git a/tests/unit/test_agentic_lorebook.py b/tests/unit/test_agentic_lorebook.py index 4c246238..9fe22bb5 100644 --- a/tests/unit/test_agentic_lorebook.py +++ b/tests/unit/test_agentic_lorebook.py @@ -23,15 +23,16 @@ select_active_entries, select_keyword_entries, ) -from backend.inference import ( - TOOLS, - CachedBase, - build_direct_scene_tool, - build_lorebook_select_prompt, -) +from backend.inference import CachedBase from backend.pipeline import LorebookTurn from backend.pipeline.passes.director import lorebook_select_step +from backend.pipeline.passes.director.lorebook_select import ( + _log_director_pick_diagnostics, +) +from backend.pipeline.passes.director.prompts import build_lorebook_select_prompt from backend.pipeline.passes.writer import build_writer_content +from backend.prompting.tool_catalog import TOOLS +from backend.prompting.tool_schemas import build_direct_scene_tool def _entry( @@ -361,21 +362,21 @@ def test_a_name_that_contains_brackets_is_matched_as_stored(self): def test_a_recovered_pick_is_logged_as_a_warning(self, caplog): # That warning count is the per-model rate of this failure. - with caplog.at_level(logging.WARNING, logger="backend.inference.lorebook"): - assert self._names("[The Ashen Seal]") == ["The Ashen Seal"] + with caplog.at_level(logging.WARNING, logger="backend.pipeline.passes.director.lorebook_select"): + _log_director_pick_diagnostics(self._entries, ["[The Ashen Seal]"]) assert "matched only after stripping catalog delimiters" in caplog.text def test_a_clean_pick_logs_nothing(self, caplog): - with caplog.at_level(logging.INFO, logger="backend.inference.lorebook"): - assert self._names("The Ashen Seal") == ["The Ashen Seal"] + with caplog.at_level(logging.INFO, logger="backend.pipeline.passes.director.lorebook_select"): + _log_director_pick_diagnostics(self._entries, ["The Ashen Seal"]) assert caplog.text == "" def test_a_pick_naming_a_constant_entry_stays_silent(self, caplog): # Constant entries ride the cached prefix; excluding them here is by # design, so it must not read as a failed pick. entries = [_entry("Const", constant=True)] - with caplog.at_level(logging.INFO, logger="backend.inference.lorebook"): - assert select_active_entries(entries, [], scan_depth=2, director_selected=["[Const]"]) == [] + with caplog.at_level(logging.INFO, logger="backend.pipeline.passes.director.lorebook_select"): + _log_director_pick_diagnostics(entries, ["[Const]"]) assert caplog.text == "" def test_the_block_renders_from_a_bracketed_pick(self): diff --git a/tests/unit/test_autocomplete.py b/tests/unit/test_autocomplete.py index c8b372cd..ce6cd88c 100644 --- a/tests/unit/test_autocomplete.py +++ b/tests/unit/test_autocomplete.py @@ -8,7 +8,7 @@ import asyncio -from backend.inference import local_ml as lc +from backend.features import autocomplete as lc def test_build_prompt_ends_at_draft_and_excludes_injection(): @@ -50,7 +50,7 @@ async def fake_acomplete(feature, prompt, *args, **kwargs): seen["prompt"] = prompt return " hands" # model re-emits a leading word separator - monkeypatch.setattr(lc, "acomplete", fake_acomplete) + monkeypatch.setattr("backend.inference.local_ml.acomplete", fake_acomplete) # Trailing space: prompt trimmed before generation, leading space dropped # (the user already typed the separator). diff --git a/tests/unit/test_card_v3.py b/tests/unit/test_card_v3.py index 142dcd94..b6cd1e44 100644 --- a/tests/unit/test_card_v3.py +++ b/tests/unit/test_card_v3.py @@ -17,7 +17,7 @@ from backend.api.deps import _normalise_lorebook_entry, lorebook_to_book from backend.features.cards.parsing import card_to_dict, parse, to_png -from backend.inference.lorebook import select_keyword_entries +from backend.prompting.lorebook import select_keyword_entries def _b64(payload: dict) -> str: diff --git a/tests/unit/test_director_per_fragment.py b/tests/unit/test_director_per_fragment.py index 85ffaa20..52f206f2 100644 --- a/tests/unit/test_director_per_fragment.py +++ b/tests/unit/test_director_per_fragment.py @@ -9,12 +9,10 @@ import json -from backend.inference import ( - CachedBase, - build_direct_scene_tool, - build_director_scene_step_prompt, -) +from backend.inference import CachedBase from backend.pipeline.passes.director.director import director_pass +from backend.pipeline.passes.director.prompts import build_director_scene_step_prompt +from backend.prompting.tool_schemas import build_direct_scene_tool _MOODS = [{"id": "tense", "description": "suspenseful"}] _FRAGMENTS = [ diff --git a/tests/unit/test_document_audit.py b/tests/unit/test_document_audit.py index 7b5d2ff3..d831bf3e 100644 --- a/tests/unit/test_document_audit.py +++ b/tests/unit/test_document_audit.py @@ -29,7 +29,7 @@ DOC_CHAT_INSTRUCTION, build_generation_messages, ) -from backend.inference import TOOLS +from backend.prompting.tool_catalog import TOOLS _BANNED = "shivers down her spine" _BANK = [[_BANNED]] # one literal phrase group, detector-facing shape diff --git a/tests/unit/test_dynamic_worlds.py b/tests/unit/test_dynamic_worlds.py index f224f5d0..9284d61e 100644 --- a/tests/unit/test_dynamic_worlds.py +++ b/tests/unit/test_dynamic_worlds.py @@ -22,15 +22,15 @@ split_by_world, validate_proposal, ) -from backend.inference.lorebook import ( +from backend.pipeline.state import TurnState, WorldProposalTurn +from backend.pipeline.world_proposal import world_proposal_stage +from backend.prompting.lorebook import ( compute_constant_lorebook_block, compute_depth_lorebook_block, compute_lorebook_injection_block, render_lorebook_block, select_effective_entries, ) -from backend.pipeline.state import TurnState, WorldProposalTurn -from backend.pipeline.world_proposal import world_proposal_stage def _authored(entry_id: int, name: str, content: str = "body", **kw) -> dict: diff --git a/tests/unit/test_editor_abort.py b/tests/unit/test_editor_abort.py index 605f60f4..f086e589 100644 --- a/tests/unit/test_editor_abort.py +++ b/tests/unit/test_editor_abort.py @@ -19,8 +19,9 @@ FlaggedSentence, ) from backend.analysis.detectors.template_repetition import TemplateResult -from backend.inference import CachedBase, LLMClient, enabled_schemas +from backend.inference import CachedBase, LLMClient from backend.pipeline.passes.editor.editor import editor_pass +from backend.prompting.tool_catalog import enabled_schemas def _make_client() -> LLMClient: diff --git a/tests/unit/test_editor_draft_update.py b/tests/unit/test_editor_draft_update.py index 002a1367..54f5cc47 100644 --- a/tests/unit/test_editor_draft_update.py +++ b/tests/unit/test_editor_draft_update.py @@ -26,12 +26,12 @@ from backend.analysis.detectors.template_repetition import TemplateResult from backend.analysis.patching import apply_id_patches from backend.inference import ( - EDITOR_RENUMBER_NOTICE, CachedBase, LLMClient, - enabled_schemas, ) from backend.pipeline.passes.editor.editor import editor_pass +from backend.pipeline.passes.editor.prompts import EDITOR_RENUMBER_NOTICE +from backend.prompting.tool_catalog import enabled_schemas SETTINGS = { "model_name": "test-model", diff --git a/tests/unit/test_group_context_modes.py b/tests/unit/test_group_context_modes.py index 86f68f35..36faab8a 100644 --- a/tests/unit/test_group_context_modes.py +++ b/tests/unit/test_group_context_modes.py @@ -11,13 +11,13 @@ import pytest from backend.core import CastMember, Macros, TurnCast -from backend.inference.group_context import ( +from backend.pipeline.passes.writer import SHEET_FRAMING, build_writer_content +from backend.prompting import build_prefix +from backend.prompting.group_context import ( context_size_components, render_active_card, render_cast_section, ) -from backend.inference.prompt_builder import build_prefix -from backend.pipeline.passes.writer import SHEET_FRAMING, build_writer_content MODES = ("private", "shared", "swap") diff --git a/tests/unit/test_group_prompt.py b/tests/unit/test_group_prompt.py index c12c9341..8a9bec75 100644 --- a/tests/unit/test_group_prompt.py +++ b/tests/unit/test_group_prompt.py @@ -2,7 +2,6 @@ from backend.core import CastMember, Macros, TurnCast from backend.database.queries.group_members import allocate_speaker_key -from backend.inference.prompt_builder import build_prefix from backend.pipeline.cast import parse_speaking_plan, plan_cue, round_robin_member from backend.pipeline.passes.director import ( build_direct_scene_override, @@ -10,6 +9,7 @@ ) from backend.pipeline.passes.writer import build_writer_content, strip_speaker_label from backend.pipeline.state import _DIRECTOR_SEED_FIELDS, TurnState +from backend.prompting import build_prefix def _member(mid: str, name: str, public: str, private: str) -> CastMember: diff --git a/tests/unit/test_import_layering.py b/tests/unit/test_import_layering.py index d72fcb1e..7845cd93 100644 --- a/tests/unit/test_import_layering.py +++ b/tests/unit/test_import_layering.py @@ -1,131 +1,78 @@ -"""Static guard for the backend's one-way layered architecture. - -The dependency direction is strictly downward (see ``agents-md-analyze-the-file`` -and each layer's ``__init__`` docstring): - - api -> {pipeline, features} -> workflows -> {inference, analysis} -> core - \\-> database -> core - -A layer may import only from the layers below it (same-layer imports are fine), -and a ``features`` slice may never import a *peer* slice. This test parses every -``backend`` module with the AST and fails on any forbidden edge. - -It walks *all* AST nodes, so it also catches lazy ``import`` statements buried -inside functions -- the form the historical ``database -> features`` back-edge -took before it was relocated to the ``api`` composition root. -""" +"""Focused fixtures for the shared backend dependency checker.""" from __future__ import annotations -import ast +import importlib.util +import sys from pathlib import Path +import pytest + REPO_ROOT = Path(__file__).resolve().parents[2] -BACKEND = REPO_ROOT / "backend" - -# What each layer MAY import (internal layers only). Same-layer imports are -# always allowed and are not listed here. -ALLOWED: dict[str, set[str]] = { - "core": set(), - "database": {"core"}, - "inference": {"core"}, - "analysis": {"core", "database"}, - "workflows": {"core", "database", "inference", "analysis"}, - "features": {"core", "database", "inference", "analysis"}, - "pipeline": {"core", "database", "inference", "analysis", "workflows", "features"}, - "api": {"core", "database", "inference", "analysis", "workflows", "features", "pipeline"}, - # ``main.py`` / ``__init__.py`` sitting directly in ``backend/`` -- the - # composition root; may wire anything below it. - "root": {"core", "database", "inference", "analysis", "workflows", "features", "pipeline", "api"}, -} -LAYERS = set(ALLOWED) - {"root"} - -FEATURE_SLICES = {p.name for p in (BACKEND / "features").iterdir() if p.is_dir() and p.name != "__pycache__"} - - -def _iter_modules(): - """Yield (path, dotted_parts, is_init) for every backend .py module. - - Skips ``__pycache__`` and one-shot migration scripts (which use dynamic - intra-package imports and are not living application surface).""" - for path in BACKEND.rglob("*.py"): - rel = path.relative_to(REPO_ROOT) - if "__pycache__" in rel.parts or "migrations" in rel.parts: - continue - parts = rel.with_suffix("").parts # ("backend", "database", "bootstrap") - is_init = path.name == "__init__.py" - if is_init: - parts = parts[:-1] # the module IS the package - yield path, parts, is_init - - -def _layer_of(parts: tuple[str, ...]) -> str | None: - if len(parts) < 2 or parts[0] != "backend": - return None - if parts[1] in ("main", "__init__"): - return "root" - return parts[1] - - -def _resolve(parts: tuple[str, ...], is_init: bool, level: int, module: str) -> list[str]: - """Resolve an import to absolute dotted parts, handling relative imports.""" - if level == 0: - return module.split(".") if module else [] - pkg = list(parts) if is_init else list(parts[:-1]) - base = pkg[: len(pkg) - (level - 1)] - return base + (module.split(".") if module else []) - - -def _imports(path: Path, parts: tuple[str, ...], is_init: bool): - """Yield (target_parts, lineno, source_text) for every backend-targeting import.""" - tree = ast.parse(path.read_text(encoding="utf-8")) - for node in ast.walk(tree): - if isinstance(node, ast.ImportFrom): - target = _resolve(parts, is_init, node.level, node.module or "") - if target and target[0] == "backend": - names = ", ".join(a.name for a in node.names) - yield target, node.lineno, f"from {'.' * node.level}{node.module or ''} import {names}" - elif isinstance(node, ast.Import): - for alias in node.names: - target = alias.name.split(".") - if target and target[0] == "backend": - yield target, node.lineno, f"import {alias.name}" - - -def test_no_upward_layer_imports(): - violations = [] - for path, parts, is_init in _iter_modules(): - src = _layer_of(parts) - if src is None: - continue - for target, lineno, text in _imports(path, parts, is_init): - dst = _layer_of(tuple(target)) - if dst is None or dst == src: - continue - if dst not in ALLOWED.get(src, set()): - rel = path.relative_to(REPO_ROOT) - violations.append(f" {src} -> {dst} {rel}:{lineno} ({text})") - assert not violations, "Forbidden cross-layer imports (a layer reached up to one it may not import):\n" + "\n".join( - sorted(violations) - ) -def test_no_peer_slice_imports(): - violations = [] - for path, parts, is_init in _iter_modules(): - if _layer_of(parts) != "features" or len(parts) < 3: - continue - own_slice = parts[2] - for target, lineno, text in _imports(path, parts, is_init): - if ( - len(target) >= 3 - and target[0] == "backend" - and target[1] == "features" - and target[2] in FEATURE_SLICES - and target[2] != own_slice - ): - rel = path.relative_to(REPO_ROOT) - violations.append(f" {own_slice} -> {target[2]} {rel}:{lineno} ({text})") - assert not violations, "A features slice imported a peer slice (slices must stay isolated):\n" + "\n".join( - sorted(violations) +def _checker(): + spec = importlib.util.spec_from_file_location( + "check_backend_layers_fixtures", REPO_ROOT / "scripts" / "check_backend_layers.py" ) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = module + spec.loader.exec_module(module) + return module + + +def _fixture(tmp_path: Path, source: str, statement: str) -> tuple[Path, Path]: + checker = _checker() + root = tmp_path / "repo" + backend = root / "backend" + for package in checker.ALLOWED_EDGES: + directory = backend / package + directory.mkdir(parents=True, exist_ok=True) + (directory / "__init__.py").write_text("", encoding="utf-8") + (backend / "main.py").write_text("app = object()\n", encoding="utf-8") + source_path = backend / f"{source}.py" + source_path.parent.mkdir(parents=True, exist_ok=True) + source_path.write_text(statement, encoding="utf-8") + return root, backend + + +@pytest.mark.parametrize( + ("source", "statement", "message"), + [ + ("inference/bad", "from backend.prompting import base\n", "inference may not import prompting"), + ("inference/bad", "from backend.main import app\n", "inference may not import root"), + ("prompting/bad", "from backend.inference import client\n", "prompting may not import inference"), + ("features/alpha/bad", "from backend.workflows import toolkit\n", "features may not import workflows"), + ("workflows/bad", "from backend.features import cards\n", "workflows may not import features"), + ], +) +def test_forbidden_edges_use_the_shared_matrix(tmp_path: Path, source: str, statement: str, message: str): + root, backend = _fixture(tmp_path, source, statement) + problems = _checker().check(root=root, backend=backend) + assert any(message in problem for problem in problems), problems + + +def test_feature_slices_cannot_import_peers(tmp_path: Path): + root, backend = _fixture(tmp_path, "features/alpha/bad", "from backend.features.beta import value\n") + beta = backend / "features" / "beta" + beta.mkdir() + (beta / "__init__.py").write_text("value = 1\n", encoding="utf-8") + problems = _checker().check(root=root, backend=backend) + assert any("feature slice 'alpha' imports peer slice 'beta'" in problem for problem in problems), problems + + +def test_python_packages_must_be_classified(tmp_path: Path): + root, backend = _fixture(tmp_path, "inference/good", "from backend.core import value\n") + unknown = backend / "mystery" + unknown.mkdir() + (unknown / "module.py").write_text("", encoding="utf-8") + problems = _checker().check(root=root, backend=backend) + assert any("backend/mystery/: unclassified Python package" in problem for problem in problems), problems + + +def test_top_level_python_modules_must_be_classified(tmp_path: Path): + root, backend = _fixture(tmp_path, "inference/good", "from backend.core import value\n") + (backend / "mystery.py").write_text("", encoding="utf-8") + problems = _checker().check(root=root, backend=backend) + assert any("backend/mystery.py: unclassified top-level Python module" in problem for problem in problems), problems diff --git a/tests/unit/test_interactive_fragments.py b/tests/unit/test_interactive_fragments.py index 88f4731e..3414c2ee 100644 --- a/tests/unit/test_interactive_fragments.py +++ b/tests/unit/test_interactive_fragments.py @@ -3,20 +3,25 @@ from __future__ import annotations from backend.database import SEED_INTERACTIVE_FRAGMENTS -from backend.inference import ( - build_direct_scene_tool, +from backend.pipeline.passes.director import apply_tool_calls +from backend.pipeline.passes.director.direction_note_prompts import ( build_direction_note_prompt, - build_direction_note_tool, +) +from backend.pipeline.passes.director.prompts import ( build_director_scene_step_prompt, build_director_tool_prompt, +) +from backend.pipeline.passes.editor import extract_feedback_values +from backend.pipeline.passes.editor.prompts import ( build_editor_prompt, build_feedback_prompt, +) +from backend.prompting import build_style_injection, compute_style_injection_block +from backend.prompting.tool_schemas import ( + build_direct_scene_tool, + build_direction_note_tool, build_feedback_tool, - build_style_injection, - compute_style_injection_block, ) -from backend.pipeline.passes.director import apply_tool_calls -from backend.pipeline.passes.editor import extract_feedback_values # ── build_direct_scene_tool ────────────────────────────────────────────────── diff --git a/tests/unit/test_kv_cache_invariants.py b/tests/unit/test_kv_cache_invariants.py index d621f722..e2f65504 100644 --- a/tests/unit/test_kv_cache_invariants.py +++ b/tests/unit/test_kv_cache_invariants.py @@ -44,14 +44,7 @@ import pytest -from backend.inference import ( - AbortToken, - CachedBase, - build_direct_scene_tool, - build_direction_note_tool, - build_feedback_tool, - enabled_schemas, -) +from backend.inference import AbortToken, CachedBase from backend.inference.kv_tracker import ( _common_prefix_len, _KVCacheTracker, @@ -60,6 +53,12 @@ ) from backend.pipeline.orchestrator import _run_pipeline from backend.pipeline.passes.editor.editor import editor_pass +from backend.prompting.tool_catalog import enabled_schemas +from backend.prompting.tool_schemas import ( + build_direct_scene_tool, + build_direction_note_tool, + build_feedback_tool, +) def _wire_tools(tools: Any) -> str: diff --git a/tests/unit/test_lorebook_matching.py b/tests/unit/test_lorebook_matching.py index 238e0a9d..64cd5626 100644 --- a/tests/unit/test_lorebook_matching.py +++ b/tests/unit/test_lorebook_matching.py @@ -7,7 +7,7 @@ from __future__ import annotations -from backend.inference.lorebook import select_active_entries, select_keyword_entries +from backend.prompting.lorebook import select_active_entries, select_keyword_entries def _entry(**kw): diff --git a/tests/unit/test_offer_tools_blob.py b/tests/unit/test_offer_tools_blob.py index 2437c79a..06c18bda 100644 --- a/tests/unit/test_offer_tools_blob.py +++ b/tests/unit/test_offer_tools_blob.py @@ -7,7 +7,7 @@ difference between the two requests must be `tool_choice`. Nothing else pins this. `enabled_schemas()` ordering is covered by -test_tool_registry.py, but the `offer_tools` path bypasses `enabled_schemas` +test_tool_catalog.py, but the `offer_tools` path bypasses `enabled_schemas` entirely: it builds the array from the caller's tuple, so a reordered OFFER_TOOLS or an append-on-miss regression would silently split the two calls onto different prefixes with no test failing. @@ -24,7 +24,7 @@ import pytest -from backend.inference.tool_registry import TOOLS +from backend.prompting.tool_catalog import TOOLS from backend.workflows._forced_call import forced_tool_call from backend.workflows.image_gen.prompts import OFFER_TOOLS diff --git a/tests/unit/test_prompt_builder_extras.py b/tests/unit/test_prompt_builder_extras.py index 2d663108..d905b17f 100644 --- a/tests/unit/test_prompt_builder_extras.py +++ b/tests/unit/test_prompt_builder_extras.py @@ -9,7 +9,7 @@ import pytest -from backend.inference import build_prefix, format_message_with_attachments +from backend.prompting import build_prefix, format_message_with_attachments _BASE_KWARGS = dict( system_prompt="You are an assistant.", diff --git a/tests/unit/test_public_profile_draft.py b/tests/unit/test_public_profile_draft.py index 311d6321..ee8dfab0 100644 --- a/tests/unit/test_public_profile_draft.py +++ b/tests/unit/test_public_profile_draft.py @@ -10,7 +10,7 @@ * a blank field silently publishes nothing about that member; * a brace survives into a string that is macro-resolved at *turn* time - (``inference/group_context._render_public_cast``), so an approved profile + (``prompting/group_context._render_public_cast``), so an approved profile would mutate months later; * an overlong field is billed to every member of the cast on every call. """ diff --git a/tests/unit/test_reasoning_breaks.py b/tests/unit/test_reasoning_breaks.py index 5313c89c..00f0f6f1 100644 --- a/tests/unit/test_reasoning_breaks.py +++ b/tests/unit/test_reasoning_breaks.py @@ -19,9 +19,10 @@ mark_call_start, reasoning_delta_event, ) -from backend.inference import CachedBase, LLMClient, enabled_schemas +from backend.inference import CachedBase, LLMClient from backend.pipeline.passes.editor.editor import editor_pass from backend.pipeline.state import TurnState +from backend.prompting.tool_catalog import enabled_schemas def _start(delta: str) -> dict: diff --git a/tests/unit/test_structured_tool_calls.py b/tests/unit/test_structured_tool_calls.py index f088a878..e452a841 100644 --- a/tests/unit/test_structured_tool_calls.py +++ b/tests/unit/test_structured_tool_calls.py @@ -9,8 +9,9 @@ import backend.inference.client as llm_mod import backend.inference.endpoint_profiles as ep_mod -from backend.inference.client import LLMClient, parse_tool_calls, strictify_schema +from backend.inference.client import LLMClient, parse_tool_calls from backend.inference.endpoint_profiles import supports_structured_tool_calls +from backend.inference.schema import strictify_schema @pytest.fixture(autouse=True) diff --git a/tests/unit/test_super_regen_audit_context.py b/tests/unit/test_super_regen_audit_context.py index 03f96d70..9d698baf 100644 --- a/tests/unit/test_super_regen_audit_context.py +++ b/tests/unit/test_super_regen_audit_context.py @@ -19,8 +19,9 @@ from backend.analysis.detectors.slop_detector import DetectionResult from backend.analysis.detectors.structural_repetition import StructuralResult from backend.analysis.detectors.template_repetition import TemplateResult -from backend.inference import CachedBase, LLMClient, enabled_schemas +from backend.inference import CachedBase, LLMClient from backend.pipeline.passes.editor.editor import editor_pass +from backend.prompting.tool_catalog import enabled_schemas def _editor_base(prefix: list[dict]) -> CachedBase: diff --git a/tests/unit/test_tool_catalog.py b/tests/unit/test_tool_catalog.py new file mode 100644 index 00000000..0000af67 --- /dev/null +++ b/tests/unit/test_tool_catalog.py @@ -0,0 +1,156 @@ +"""Ordered tool catalog contracts.""" + +from __future__ import annotations + +import hashlib +import json +from collections.abc import AsyncIterator +from typing import Any + +import pytest + +from backend.database.seeds import DEFAULT_ENABLED_TOOLS +from backend.inference import CachedBase +from backend.pipeline.tools import DIRECTOR_LOOP_TOOL_NAMES +from backend.prompting.tool_catalog import ( + BUILTIN_TOOL_NAMES, + BUILTIN_TOOL_ORDER, + STANDALONE_TOOLS, + TOOLS, + enabled_schemas, + register_tool, + require_tool, + restore_catalog, + snapshot_catalog, +) + +_TEST_TOOL_NAME = "ut_tool_catalog_test" +_TEST_SCHEMA = { + "type": "function", + "function": { + "name": _TEST_TOOL_NAME, + "description": "test", + "parameters": {"type": "object", "properties": {}}, + }, +} +_TEST_CHOICE = {"type": "function", "function": {"name": _TEST_TOOL_NAME}} +_BUILTIN_BLOB_LENGTH = 4808 +_BUILTIN_BLOB_SHA256 = "671820cf9eff1beefc6707f3f50c2bff646aa167212198d2fd1ab05d5efb7faf" + + +def _tool_blob(tools: list[dict]) -> str: + return json.dumps(tools, separators=(",", ":"), ensure_ascii=False) + + +@pytest.fixture +def _restore_registry(): + snapshot = snapshot_catalog() + yield + restore_catalog(snapshot) + + +def test_builtin_order_is_explicit_and_complete(): + assert BUILTIN_TOOL_ORDER == ( + "direct_scene", + "editor_apply_patch", + "editor_rewrite", + "give_feedback", + "record_direction_note", + "select_lorebook", + "propose_world_changes", + ) + assert BUILTIN_TOOL_NAMES == frozenset(BUILTIN_TOOL_ORDER) + assert tuple(TOOLS)[: len(BUILTIN_TOOL_ORDER)] == BUILTIN_TOOL_ORDER + + +def test_director_loop_membership_is_pipeline_owned(): + assert DIRECTOR_LOOP_TOOL_NAMES == frozenset({"direct_scene"}) + assert DIRECTOR_LOOP_TOOL_NAMES <= BUILTIN_TOOL_NAMES + + +def test_default_enabled_tools_subset_of_catalog(): + assert set(DEFAULT_ENABLED_TOOLS) <= set(TOOLS) + + +def test_enabled_schemas_preserves_builtin_order(): + names = [schema["function"]["name"] for schema in enabled_schemas(None)] + assert names == list(BUILTIN_TOOL_ORDER) + + +def test_complete_builtin_blob_is_byte_stable(): + blob = _tool_blob(enabled_schemas(None)) + assert len(blob) == _BUILTIN_BLOB_LENGTH + assert hashlib.sha256(blob.encode()).hexdigest() == _BUILTIN_BLOB_SHA256 + + +async def test_complete_builtin_blob_survives_cached_base_boundary(): + captured: list[str] = [] + + class _CapturingClient: + async def complete(self, **kwargs: Any) -> AsyncIterator[dict]: + captured.append(_tool_blob(kwargs["tools"])) + yield {"type": "done", "message": {"role": "assistant", "content": ""}} + + base = CachedBase( + prefix=({"role": "system", "content": "system"},), + tools=tuple(enabled_schemas(None)), + model="model", + ) + async for _ in base.complete(_CapturingClient(), label="writer", trailing=[]): + pass + + assert len(captured[0]) == _BUILTIN_BLOB_LENGTH + assert hashlib.sha256(captured[0].encode()).hexdigest() == _BUILTIN_BLOB_SHA256 + + +def test_enabled_schemas_filters_without_caller_order(): + gated = { + "editor_rewrite": True, + "editor_apply_patch": True, + "direct_scene": False, + } + names = [schema["function"]["name"] for schema in enabled_schemas(gated)] + assert names == ["editor_apply_patch", "editor_rewrite"] + assert enabled_schemas({}) == [] + + +def test_compatibility_views_are_read_only(): + with pytest.raises(TypeError): + TOOLS["bad"] = {} # type: ignore[index] + with pytest.raises(AttributeError): + STANDALONE_TOOLS.add("bad") # type: ignore[attr-defined] + + exposed = TOOLS["direct_scene"] + exposed["schema"]["function"]["name"] = "bad" + assert require_tool("direct_scene")["schema"]["function"]["name"] == "direct_scene" + + schemas = enabled_schemas({"direct_scene": True}) + schemas[0]["function"]["name"] = "also_bad" + assert require_tool("direct_scene")["schema"]["function"]["name"] == "direct_scene" + + +def test_standalone_registration_is_filtered(_restore_registry): + register_tool(_TEST_TOOL_NAME, _TEST_SCHEMA, _TEST_CHOICE, standalone=True) + assert _TEST_TOOL_NAME in TOOLS + assert _TEST_TOOL_NAME in STANDALONE_TOOLS + assert _TEST_TOOL_NAME not in [schema["function"]["name"] for schema in enabled_schemas(None)] + + +def test_non_standalone_registration_appends(_restore_registry): + before = [schema["function"]["name"] for schema in enabled_schemas(None)] + register_tool(_TEST_TOOL_NAME, _TEST_SCHEMA, _TEST_CHOICE) + assert [schema["function"]["name"] for schema in enabled_schemas(None)] == [ + *before, + _TEST_TOOL_NAME, + ] + + +def test_reregistration_preserves_position_and_toggles_standalone(_restore_registry): + register_tool(_TEST_TOOL_NAME, _TEST_SCHEMA, _TEST_CHOICE, standalone=True) + position = tuple(TOOLS).index(_TEST_TOOL_NAME) + register_tool(_TEST_TOOL_NAME, _TEST_SCHEMA, _TEST_CHOICE, standalone=False) + assert tuple(TOOLS).index(_TEST_TOOL_NAME) == position + assert _TEST_TOOL_NAME not in STANDALONE_TOOLS + register_tool(_TEST_TOOL_NAME, _TEST_SCHEMA, _TEST_CHOICE, standalone=True) + assert tuple(TOOLS).index(_TEST_TOOL_NAME) == position + assert _TEST_TOOL_NAME in STANDALONE_TOOLS diff --git a/tests/unit/test_tool_registry.py b/tests/unit/test_tool_registry.py deleted file mode 100644 index ebf51057..00000000 --- a/tests/unit/test_tool_registry.py +++ /dev/null @@ -1,137 +0,0 @@ -"""Unit tests for the tool registry surface: built-in name set, register_tool, -standalone filter, and enabled_schemas ordering.""" - -from __future__ import annotations - -import pytest - -from backend.database.seeds import DEFAULT_ENABLED_TOOLS -from backend.inference import ( - BUILTIN_TOOL_NAMES, - POST_WRITER_TOOLS, - PRE_WRITER_TOOLS, - STANDALONE_TOOLS, - TOOLS, - enabled_schemas, - register_tool, -) - -_TEST_TOOL_NAME = "ut_tool_registry_test" -_TEST_SCHEMA = { - "type": "function", - "function": { - "name": _TEST_TOOL_NAME, - "description": "test", - "parameters": {"type": "object", "properties": {}}, - }, -} -_TEST_CHOICE = {"type": "function", "function": {"name": _TEST_TOOL_NAME}} - - -@pytest.fixture -def _restore_registry(): - """Restore TOOLS and STANDALONE_TOOLS after a test mutates them.""" - tools_snapshot = dict(TOOLS) - standalone_snapshot = set(STANDALONE_TOOLS) - yield - TOOLS.clear() - TOOLS.update(tools_snapshot) - STANDALONE_TOOLS.clear() - STANDALONE_TOOLS.update(standalone_snapshot) - - -class TestBuiltinToolNames: - def test_matches_tools_keys_at_module_load(self): - assert BUILTIN_TOOL_NAMES == frozenset(TOOLS) - STANDALONE_TOOLS - - -class TestStandaloneToolsBaseline: - def test_empty_at_module_load(self): - assert BUILTIN_TOOL_NAMES.isdisjoint(STANDALONE_TOOLS) - - -class TestPipelinePhaseSets: - """PRE_WRITER_TOOLS and POST_WRITER_TOOLS partition the built-in tools by - pipeline phase — no overlap, full coverage. give_feedback is a post-writer - feedback-step tool, not a director tool.""" - - def test_phase_sets_are_disjoint(self): - assert PRE_WRITER_TOOLS.isdisjoint(POST_WRITER_TOOLS) - - def test_phase_sets_partition_builtins(self): - assert PRE_WRITER_TOOLS | POST_WRITER_TOOLS == BUILTIN_TOOL_NAMES - - -class TestEnabledToolsHoldsOnlyTools: - """enabled_tools is a tool-registry switch, not a feature-flag bag. The - seeded default must only name registered tools — feature flags (length_guard, - length_guard_enforce, ...) live in their own settings columns.""" - - def test_default_enabled_tools_subset_of_registry(self): - assert set(DEFAULT_ENABLED_TOOLS) <= set(TOOLS) - - -class TestEnabledSchemasBaseline: - def test_none_returns_tools_insertion_order(self): - schemas = enabled_schemas(None) - names = [s["function"]["name"] for s in schemas] - assert names == [ - "direct_scene", - "editor_apply_patch", - "editor_rewrite", - "give_feedback", - "record_direction_note", - "select_lorebook", - "propose_world_changes", - ] - - def test_dict_filter_returns_insertion_order_subset(self): - gated = { - "editor_rewrite": True, - "editor_apply_patch": True, - "direct_scene": False, - } - names = [s["function"]["name"] for s in enabled_schemas(gated)] - assert names == ["editor_apply_patch", "editor_rewrite"] - - def test_empty_dict_returns_nothing(self): - assert enabled_schemas({}) == [] - - -class TestRegisterTool: - def test_standalone_true_filters_out_of_schemas(self, _restore_registry): - register_tool(_TEST_TOOL_NAME, _TEST_SCHEMA, _TEST_CHOICE, standalone=True) - assert _TEST_TOOL_NAME in TOOLS - assert _TEST_TOOL_NAME in STANDALONE_TOOLS - names = [s["function"]["name"] for s in enabled_schemas(None)] - assert _TEST_TOOL_NAME not in names - - def test_standalone_false_appears_in_schemas(self, _restore_registry): - register_tool(_TEST_TOOL_NAME, _TEST_SCHEMA, _TEST_CHOICE, standalone=False) - assert _TEST_TOOL_NAME in TOOLS - assert _TEST_TOOL_NAME not in STANDALONE_TOOLS - names = [s["function"]["name"] for s in enabled_schemas(None)] - assert _TEST_TOOL_NAME in names - - def test_standalone_bit_symmetric_on_reregistration(self, _restore_registry): - register_tool(_TEST_TOOL_NAME, _TEST_SCHEMA, _TEST_CHOICE, standalone=True) - assert _TEST_TOOL_NAME in STANDALONE_TOOLS - - register_tool(_TEST_TOOL_NAME, _TEST_SCHEMA, _TEST_CHOICE, standalone=False) - assert _TEST_TOOL_NAME not in STANDALONE_TOOLS - - register_tool(_TEST_TOOL_NAME, _TEST_SCHEMA, _TEST_CHOICE, standalone=True) - assert _TEST_TOOL_NAME in STANDALONE_TOOLS - - def test_registered_tool_lands_at_end_under_insertion_order(self, _restore_registry): - before = [s["function"]["name"] for s in enabled_schemas(None)] - register_tool( - "z_late_tool", - {"type": "function", "function": {"name": "z_late_tool"}}, - {"type": "function", "function": {"name": "z_late_tool"}}, - standalone=False, - ) - names = [s["function"]["name"] for s in enabled_schemas(None)] - # A late registration appends; it must never reorder the built-in prefix, - # which the cross-pass KV cache depends on staying byte-identical. - assert names == [*before, "z_late_tool"] diff --git a/tests/unit/workflows/test_forced_call.py b/tests/unit/workflows/test_forced_call.py index dd154201..197ead7f 100644 --- a/tests/unit/workflows/test_forced_call.py +++ b/tests/unit/workflows/test_forced_call.py @@ -7,7 +7,7 @@ from collections.abc import AsyncIterator from typing import Any -from backend.inference import STANDALONE_TOOLS, TOOLS +from backend.prompting.tool_catalog import TOOLS, register_tool, require_tool from backend.workflows._forced_call import forced_tool_call _TOOL_NAME = "editor_rewrite" @@ -240,7 +240,8 @@ async def test_enabled_tools_dict_matches_enabled_schemas(self): assert names == ["editor_apply_patch", "editor_rewrite"] async def test_standalone_forced_tool_appended_to_array(self): - STANDALONE_TOOLS.add(_TOOL_NAME) + tool = require_tool(_TOOL_NAME) + register_tool(_TOOL_NAME, tool["schema"], tool["choice"], standalone=True) try: client = _FakeClient([_done_event_with_tool_call(_TOOL_NAME, {})]) await _collect( @@ -257,7 +258,7 @@ async def test_standalone_forced_tool_appended_to_array(self): assert _TOOL_NAME in names assert "editor_apply_patch" in names finally: - STANDALONE_TOOLS.discard(_TOOL_NAME) + register_tool(_TOOL_NAME, tool["schema"], tool["choice"], standalone=False) async def test_force_tool_missing_from_enabled_dict_appended(self): client = _FakeClient([_done_event_with_tool_call(_TOOL_NAME, {})]) diff --git a/tests/unit/workflows/test_registry.py b/tests/unit/workflows/test_registry.py index 42c99649..0f1cb5e5 100644 --- a/tests/unit/workflows/test_registry.py +++ b/tests/unit/workflows/test_registry.py @@ -8,7 +8,12 @@ import pytest -from backend.inference import STANDALONE_TOOLS, TOOLS +from backend.prompting.tool_catalog import ( + STANDALONE_TOOLS, + TOOLS, + restore_catalog, + snapshot_catalog, +) from backend.workflows import ( HookType, Subscription, @@ -60,8 +65,7 @@ def _tool_spec(name: str, *, standalone: bool = True) -> ToolSpec: @pytest.fixture(autouse=True) def _restore_globals(): by_id_snapshot = {k: deepcopy(v) for k, v in registry_module._WORKFLOWS_BY_ID.items()} - tools_snapshot = dict(TOOLS) - standalone_snapshot = set(STANDALONE_TOOLS) + catalog_snapshot = snapshot_catalog() # Tests below assert exact registry contents, so start from an empty # workflow registry rather than the first-party workflows registered at # import time. Built-in tools in TOOLS are left intact. @@ -69,10 +73,7 @@ def _restore_globals(): yield registry_module._WORKFLOWS_BY_ID.clear() registry_module._WORKFLOWS_BY_ID.update(by_id_snapshot) - TOOLS.clear() - TOOLS.update(tools_snapshot) - STANDALONE_TOOLS.clear() - STANDALONE_TOOLS.update(standalone_snapshot) + restore_catalog(catalog_snapshot) class TestFreshRegistration: @@ -149,7 +150,7 @@ def test_raises_and_leaves_builtin_unchanged(self): clash = _tool_spec("editor_rewrite") with pytest.raises(ToolNameCollision): register_workflow(Workflow(id="ws_clash", display_name="X", tools=[clash])) - assert TOOLS["editor_rewrite"] is before + assert TOOLS["editor_rewrite"] == before assert get_workflow("ws_clash") is None def test_built_in_check_fires_before_cross_workflow_check(self): From 5de5bf1728f387b1e5615ba08132031e092d0ba4 Mon Sep 17 00:00:00 2001 From: Chi Date: Sun, 6 Sep 2026 20:05:17 +0700 Subject: [PATCH 5/6] enforce secondary workflow contracts --- AGENTS.md | 7 + backend/inference/local_ml.py | 4 +- .../workflows/format_consistency/__init__.py | 2 +- backend/workflows/format_consistency/hooks.py | 3 +- backend/workflows/image_gen/__init__.py | 2 +- .../workflows/image_gen/engine/contracts.py | 2 +- backend/workflows/image_gen/hooks.py | 2 +- backend/workflows/image_gen/pov.py | 6 +- backend/workflows/image_gen/prompts.py | 2 +- backend/workflows/toolkit.py | 54 +++++--- backend/workflows/tts/__init__.py | 2 +- docs/architecture/prompting.md | 9 +- docs/architecture/secondary-workflow.md | 23 +++- scripts/check_backend_layers.py | 130 ++++++++++++++++-- tests/unit/test_import_layering.py | 66 +++++++++ tests/unit/workflows/test_toolkit_surface.py | 37 +++++ 16 files changed, 299 insertions(+), 52 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 30bddc8b..26a565bd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -44,6 +44,13 @@ Before changing prompt assembly, pass ordering, tool schemas, or streaming behav - Use registered actions and `data-*` attributes for UI events. Do not add globals or inline event handlers. - Keep frontend layer checks passing. +Backend workflow plug-ins under `backend/workflows//` follow the same rule: +import only their own package and `backend.workflows.toolkit`. Root modules +directly under `backend/workflows/` are host adapters and may bridge to lower +application layers. Import named toolkit exports explicitly; wildcard, +module-object, and non-`__all__` toolkit imports are not part of the plug-in API. +The backend layer checker enforces this distinction. + ## Validation Use the repository scripts from the project root: diff --git a/backend/inference/local_ml.py b/backend/inference/local_ml.py index 609469c4..766621b3 100644 --- a/backend/inference/local_ml.py +++ b/backend/inference/local_ml.py @@ -30,8 +30,8 @@ ) #: Re-exported from :mod:`local_models` for callers that address this module by -#: name — ``workflows/toolkit.py`` publishes it as the workflow author's API, -#: and the Local ML routes and tests import from here. NOTE FOR TESTS: these +#: name. The workflow toolkit wraps the small capabilities plug-ins need; Local +#: ML routes and tests import this implementation module directly. NOTE FOR TESTS: these #: are second bindings. Production code calls ``local_models``' own copies, so #: a monkeypatch belongs on the module that OWNS the name (``assets.download``, #: ``dependencies.deps_ok``), not on the re-export. diff --git a/backend/workflows/format_consistency/__init__.py b/backend/workflows/format_consistency/__init__.py index fde1e20b..7566cef6 100644 --- a/backend/workflows/format_consistency/__init__.py +++ b/backend/workflows/format_consistency/__init__.py @@ -2,7 +2,7 @@ from __future__ import annotations -from ..registry import Workflow +from ..toolkit import Workflow format_consistency_workflow = Workflow( id="format_consistency", diff --git a/backend/workflows/format_consistency/hooks.py b/backend/workflows/format_consistency/hooks.py index bfd109d7..6501989f 100644 --- a/backend/workflows/format_consistency/hooks.py +++ b/backend/workflows/format_consistency/hooks.py @@ -4,8 +4,7 @@ import logging -from ..contracts import EV_DRAFT_REPLACED -from ..toolkit import normalize_to_baseline +from ..toolkit import EV_DRAFT_REPLACED, normalize_to_baseline logger = logging.getLogger(__name__) diff --git a/backend/workflows/image_gen/__init__.py b/backend/workflows/image_gen/__init__.py index 92712fdf..b3e50e8f 100644 --- a/backend/workflows/image_gen/__init__.py +++ b/backend/workflows/image_gen/__init__.py @@ -2,7 +2,7 @@ from __future__ import annotations -from ..registry import Workflow +from ..toolkit import Workflow from .config import CONFIG_DEFAULTS, SOURCES, normalize_config from .pov import POV_MODES from .prompts import ANALYZE_TOOL, COMPOSE_TOOL diff --git a/backend/workflows/image_gen/engine/contracts.py b/backend/workflows/image_gen/engine/contracts.py index 1f8d52b6..f3de8bdf 100644 --- a/backend/workflows/image_gen/engine/contracts.py +++ b/backend/workflows/image_gen/engine/contracts.py @@ -6,7 +6,7 @@ from dataclasses import dataclass, field from typing import Any, TypedDict -from ...errors import WorkflowUserFacingError +from ...toolkit import WorkflowUserFacingError ProgressCallback = Callable[[str, Mapping[str, Any]], Awaitable[None] | None] diff --git a/backend/workflows/image_gen/hooks.py b/backend/workflows/image_gen/hooks.py index 6f1ad400..d0d39da0 100644 --- a/backend/workflows/image_gen/hooks.py +++ b/backend/workflows/image_gen/hooks.py @@ -8,8 +8,8 @@ from collections.abc import Mapping, Sequence from typing import Any -from ..contracts import WorkflowEventStream from ..toolkit import ( + WorkflowEventStream, build_offturn_prefix, get_message_by_id, get_workflow_character_state, diff --git a/backend/workflows/image_gen/pov.py b/backend/workflows/image_gen/pov.py index b88e5ca7..946f0cce 100644 --- a/backend/workflows/image_gen/pov.py +++ b/backend/workflows/image_gen/pov.py @@ -6,7 +6,7 @@ from collections.abc import Mapping, Sequence from typing import Any -from ..toolkit import get_settings, local_ml +from ..toolkit import classify_pov, get_settings, local_feature_available logger = logging.getLogger(__name__) @@ -35,7 +35,7 @@ def normalize_mode(value: Any) -> str: async def classifier_ready() -> bool: """Extras installed, model on disk, and the feature toggle left on.""" - ok, _reason = local_ml.available(FEATURE) + ok, _reason = local_feature_available(FEATURE) if not ok: return False settings = await get_settings() @@ -67,7 +67,7 @@ async def _classify(history: Sequence[Mapping[str, Any]]) -> str | None: """ for text in _assistant_texts(history): try: - label = await local_ml.aclassify_pov(text) + label = await classify_pov(text) except Exception: logger.exception("[image_gen] POV classification failed; falling back") return None diff --git a/backend/workflows/image_gen/prompts.py b/backend/workflows/image_gen/prompts.py index 79fcbe69..42a89fac 100644 --- a/backend/workflows/image_gen/prompts.py +++ b/backend/workflows/image_gen/prompts.py @@ -4,7 +4,7 @@ from collections.abc import Sequence -from ..contracts import ToolSpec +from ..toolkit import ToolSpec from .pov import FIRST, THIRD from .scrub import SubjectAppearance, bounded, normalize_prompt_format diff --git a/backend/workflows/toolkit.py b/backend/workflows/toolkit.py index 7a0d59c9..697b1acf 100644 --- a/backend/workflows/toolkit.py +++ b/backend/workflows/toolkit.py @@ -39,19 +39,21 @@ resolve_cast, resolve_char_context, ) +from ..inference import local_ml as _local_ml from ..inference import ( - LLMClient, - local_ml, - parse_tool_calls, - reasoning_cfg, - separate_agent_lane_configured, + separate_agent_lane_configured as _separate_agent_lane_configured, +) +from ..prompting import build_prefix as _build_prefix +from ..prompting import macro_identity as _macro_identity +from ..prompting.lorebook import ( + compute_constant_lorebook_block as _compute_constant_lorebook_block, ) -from ..prompting import build_prefix, format_message_with_attachments, macro_identity -from ..prompting.lorebook import compute_constant_lorebook_block -from ..prompting.tool_catalog import STANDALONE_TOOLS, TOOLS, enabled_schemas from ._forced_call import forced_tool_call from .attachment_cache import EVICTED_MARKER, insert_workflow_attachment +from .contracts import EV_DRAFT_REPLACED, ToolSpec, WorkflowEventStream +from .errors import WorkflowUserFacingError from .registry import ( + Workflow, get_workflow_character_state, get_workflow_config, get_workflow_message_state, @@ -66,16 +68,16 @@ __all__ = [ "CastMember", "EVICTED_MARKER", + "EV_DRAFT_REPLACED", "FormatDriftReport", - "LLMClient", "Macros", - "STANDALONE_TOOLS", - "TOOLS", + "ToolSpec", "TurnCast", - "build_prefix", - "enabled_schemas", + "Workflow", + "WorkflowEventStream", + "WorkflowUserFacingError", + "classify_pov", "forced_tool_call", - "format_message_with_attachments", "build_targets", "format_numbered_report", "format_report", @@ -99,11 +101,9 @@ "get_workflow_message_state", "get_workflow_state", "insert_workflow_attachment", - "local_ml", + "local_feature_available", "normalize_to_baseline", "overlay_enable_tools", - "parse_tool_calls", - "reasoning_cfg", "run_audit", "build_offturn_prefix", "set_workflow_character_state", @@ -116,6 +116,16 @@ ] +def local_feature_available(feature: str) -> tuple[bool, str]: + """Return whether a host-provided local classifier is ready.""" + return _local_ml.available(feature) + + +async def classify_pov(text: str) -> str: + """Classify narrative point of view through the host inference service.""" + return await _local_ml.aclassify_pov(text) + + async def get_scene_cast(conversation_id: str) -> TurnCast: """Return the conversation's resolved cast.""" conv = await get_conversation(conversation_id) @@ -144,7 +154,7 @@ async def build_offturn_prefix( # runs on in every mode, Classic card swap included. turn_cast = await resolve_cast(conv) system_prompt, char_persona, mes_example = await resolve_char_context(conv, settings, card=card) - dual_agent = lane == "agent" and separate_agent_lane_configured(settings) + dual_agent = lane == "agent" and _separate_agent_lane_configured(settings) if dual_agent: system_prompt, _, _ = await resolve_char_context( conv, @@ -156,13 +166,13 @@ async def build_offturn_prefix( conv.get("persona_lock_id") or (card.get("persona_lock_id") if card else None) or settings.get("active_persona_id") ) persona = await get_user_persona(persona_id) if persona_id else None - macro_char, cast_names = macro_identity(conv, turn_cast) + macro_char, cast_names = _macro_identity(conv, turn_cast) macros = Macros.from_settings( settings, macro_char, persona, seed=conv.get("macro_seed") or conv.get("id", ""), cast=cast_names ) speaker_names = await get_speaker_names(conversation_id) if turn_cast.grouped else {} user_description = persona.get("description", "") if persona else settings.get("user_description", "") - return build_prefix( + return _build_prefix( system_prompt, char_persona, conv.get("character_scenario", ""), @@ -171,7 +181,9 @@ async def build_offturn_prefix( history, macros, user_description, - constant_lorebook_block=compute_constant_lorebook_block(await get_active_lorebook_entries(), macros), + constant_lorebook_block=_compute_constant_lorebook_block( + await get_active_lorebook_entries(), macros + ), cast=turn_cast, speaker_names=speaker_names, ) diff --git a/backend/workflows/tts/__init__.py b/backend/workflows/tts/__init__.py index 819994b1..04d7d54a 100644 --- a/backend/workflows/tts/__init__.py +++ b/backend/workflows/tts/__init__.py @@ -2,7 +2,7 @@ from __future__ import annotations -from ..registry import Workflow +from ..toolkit import Workflow from .config import CONFIG_DEFAULTS, CONFIG_SCHEMA, normalize_config tts_workflow = Workflow( diff --git a/docs/architecture/prompting.md b/docs/architecture/prompting.md index 524a15a7..7311ce95 100644 --- a/docs/architecture/prompting.md +++ b/docs/architecture/prompting.md @@ -28,9 +28,12 @@ core -> (nothing) ``` `features/` and `workflows/` are siblings and do not import one another. -Feature slices do not import peer slices. The exact matrix is enforced by -`scripts/check_backend_layers.py`; every Python-bearing top-level backend -package must be classified there. +Feature slices do not import peer slices. The `workflows` row applies to the +host framework modules directly under `backend/workflows/`; plug-in slices +under `backend/workflows//` may import only their own package and the public +workflow toolkit, never these lower layers, host internals, or peer plug-ins. The exact rules are +enforced by `scripts/check_backend_layers.py`; every Python-bearing top-level +backend package must be classified there. ## Byte and order contracts diff --git a/docs/architecture/secondary-workflow.md b/docs/architecture/secondary-workflow.md index 57cf87a7..968f2719 100644 --- a/docs/architecture/secondary-workflow.md +++ b/docs/architecture/secondary-workflow.md @@ -38,6 +38,15 @@ chrome. The workflow owns its feature logic. Each workflow has a directory such as `backend/workflows/tts/`. +Code under `backend/workflows//` is a plug-in slice. It may import its own +package and `backend.workflows.toolkit`, but not other framework modules, +application layers, or peer workflows. Root modules directly under +`backend/workflows/` are host adapters and own the integration with prompting, +inference, persistence, and the pipeline. The toolkit must be consumed through +explicit names in its literal `__all__`; wildcard imports, importing the module +object, and private names are rejected. The backend layer checker enforces this +boundary. + ### Frontend | Path | Purpose | @@ -74,8 +83,8 @@ and `tool_choice`. Workflow tools append after the fixed built-in tool order. Re-registering an existing tool replaces its contract without changing its position; removing a -tool on workflow replacement removes it through the catalog API. The toolkit's -`TOOLS` and `STANDALONE_TOOLS` exports are live, read-only compatibility views. +tool on workflow replacement removes it through the framework-owned catalog +API. The catalog itself is not part of the plug-in API. Registration follows this shape: @@ -154,10 +163,12 @@ read-modify-write operations. | `workflow_config` | Workflow | `workflow_config_lock()` | | Attachments | Root attachment group | Framework's root lock | -The required import surface is `backend.workflows.toolkit`. It provides the LLM -client and prompt helpers, read-only database queries, state getters/setters, -`forced_tool_call`, attachment insertion, and the workflow locks. Mutating core -database helpers are intentionally not exposed to workflows. +The primary runtime import surface is `backend.workflows.toolkit`. Hook contexts +carry the LLM clients; the toolkit provides semantic host operations, read-only +database queries, state getters/setters, `forced_tool_call`, attachment +insertion, and workflow locks. Raw prompting, inference, and tool-catalog +objects are intentionally not exposed to plug-ins. Mutating core database +helpers are also excluded. ## A workflow inside a turn diff --git a/scripts/check_backend_layers.py b/scripts/check_backend_layers.py index b561be50..b60f406c 100644 --- a/scripts/check_backend_layers.py +++ b/scripts/check_backend_layers.py @@ -4,7 +4,7 @@ This parses every backend module's imports, resolves relative imports, and fails on an edge that is absent from the explicit allowed-edge matrix. -Three rules: +Four rules: 1. **Explicit edges.** Each top-level Python package has a complete set of backend packages it may import. Same-package imports are always allowed. @@ -14,6 +14,9 @@ 3. **Slices never import peers.** ``features/`` may not import ``features/``. A slice is self-contained by definition — a peer edge is how two features quietly become one. + 4. **Workflow plug-ins use their API.** ``workflows/`` may import only its + own package and the public workflow framework modules, never application + layers or peer workflow plug-ins. DO NOT SPELL THIS AS A GREP. ``inference/local_models/llama_server/binary.py`` contains the literal ``https://api.github.com/repos/...``, so a grep for @@ -47,6 +50,7 @@ } ROOT_ALLOWED = frozenset(ALLOWED_EDGES) ROOT_LAYER = "root" +WORKFLOW_PLUGIN_API_MODULES = frozenset({"toolkit"}) def _module_parts(path: Path, *, root: Path = ROOT) -> list[str]: @@ -74,6 +78,14 @@ def _package_parts(path: Path, *, root: Path = ROOT) -> list[str]: return list(path.relative_to(root).parent.parts) +def _import_from_base(node: ast.ImportFrom, package: list[str]) -> list[str]: + """Resolve an ``ImportFrom`` node's module without its imported names.""" + if node.level == 0: + return node.module.split(".") if node.module else [] + base = package[: len(package) - (node.level - 1)] + return [*base, *node.module.split(".")] if node.module else base + + def _targets(node: ast.AST, package: list[str], *, root: Path = ROOT) -> list[list[str]]: """Every backend module *node* imports, as absolute part lists. @@ -89,13 +101,8 @@ def _targets(node: ast.AST, package: list[str], *, root: Path = ROOT) -> list[li return out if not isinstance(node, ast.ImportFrom): return out - if node.level == 0: - base = (node.module or "").split(".") - else: - # level 1 is the containing package, level 2 its parent, and so on. - base = package[: len(package) - (node.level - 1)] - if node.module: - base = [*base, *node.module.split(".")] + # level 1 is the containing package, level 2 its parent, and so on. + base = _import_from_base(node, package) if not base: return out out.append(base) @@ -132,9 +139,94 @@ def _unclassified_top_level_modules(backend: Path) -> set[str]: } +def _workflow_plugin_slice(path: Path, backend: Path) -> str: + """Return the workflow plug-in directory containing *path*, if any.""" + parts = path.relative_to(backend).parts + return parts[1] if len(parts) >= 3 and parts[0] == "workflows" else "" + + +def _forbidden_workflow_plugin_targets( + targets: list[list[str]], + plugin: str, +) -> set[str]: + """Backend imports outside a workflow plug-in's supported host surface.""" + forbidden: set[str] = set() + for target in targets: + if not target or target[0] != "backend" or len(target) == 1: + continue + if target[:2] != ["backend", "workflows"]: + forbidden.add(".".join(target)) + continue + own_package = len(target) >= 3 and target[2] == plugin + public_api = len(target) == 3 and target[2] in WORKFLOW_PLUGIN_API_MODULES + if not own_package and not public_api: + forbidden.add(".".join(target)) + return forbidden + + +def _literal_all(path: Path) -> frozenset[str] | None: + """Return a module's literal ``__all__``, without importing the module.""" + if not path.is_file(): + return None + try: + tree = ast.parse(path.read_text(encoding="utf-8")) + except SyntaxError: + return None + for node in tree.body: + if not isinstance(node, ast.Assign) or not any( + isinstance(target, ast.Name) and target.id == "__all__" + for target in node.targets + ): + continue + try: + value = ast.literal_eval(node.value) + except (ValueError, TypeError): + return None + if isinstance(value, (list, tuple)) and all( + isinstance(name, str) for name in value + ): + return frozenset(value) + return None + return None + + +def _nonpublic_toolkit_imports( + node: ast.AST, + package: list[str], + public_names: frozenset[str], +) -> set[str]: + if not isinstance(node, ast.ImportFrom): + return set() + if _import_from_base(node, package) != ["backend", "workflows", "toolkit"]: + return set() + return { + alias.name + for alias in node.names + if alias.name == "*" or alias.name not in public_names + } + + +def _imports_toolkit_module(node: ast.AST, package: list[str]) -> bool: + """Whether a plug-in imports the toolkit module instead of public names.""" + toolkit = ["backend", "workflows", "toolkit"] + if isinstance(node, ast.Import): + return any(alias.name.split(".")[:3] == toolkit for alias in node.names) + if not isinstance(node, ast.ImportFrom): + return False + return _import_from_base(node, package) == toolkit[:2] and any( + alias.name == "toolkit" for alias in node.names + ) + + def check(*, root: Path = ROOT, backend: Path | None = None) -> list[str]: backend = backend or root / "backend" problems: list[str] = [] + toolkit_path = backend / "workflows" / "toolkit.py" + toolkit_exports = _literal_all(toolkit_path) + if toolkit_path.is_file() and toolkit_exports is None: + problems.append( + "backend/workflows/toolkit.py: workflow plug-in API must declare a literal __all__" + ) for package in sorted(_python_packages(backend) - ALLOWED_EDGES.keys()): problems.append( f"backend/{package}/: unclassified Python package (add it to ALLOWED_EDGES)" @@ -147,6 +239,7 @@ def check(*, root: Path = ROOT, backend: Path | None = None) -> list[str]: parts = _module_parts(path, root=root) package = _package_parts(path, root=root) own_layer, own_slice = _slice_of(parts) or ("", "") + workflow_plugin = _workflow_plugin_slice(path, backend) allowed = ( ROOT_ALLOWED if own_layer in ("", ROOT_LAYER) @@ -161,12 +254,13 @@ def check(*, root: Path = ROOT, backend: Path | None = None) -> list[str]: if not isinstance(node, ast.Import | ast.ImportFrom): continue where = f"{path.relative_to(root)}:{node.lineno}" + targets = _targets(node, package, root=root) # One import statement resolves to both the package and the name # beside it (`from ..features import cards`), which is the same # edge said twice; report each layer and each peer slice once. edges = { edge - for target in _targets(node, package, root=root) + for target in targets if (edge := _slice_of(target)) and edge[0] in {*ALLOWED_EDGES, ROOT_LAYER} } @@ -176,6 +270,24 @@ def check(*, root: Path = ROOT, backend: Path | None = None) -> list[str]: peers = {s for layer, s in edges if layer == "features" and s and s != own_slice} for peer in sorted(peers): problems.append(f"{where}: feature slice {own_slice!r} imports peer slice {peer!r}") + if workflow_plugin: + for target in sorted( + _forbidden_workflow_plugin_targets(targets, workflow_plugin) + ): + problems.append( + f"{where}: workflow slice {workflow_plugin!r} may import only its own package or workflow APIs, not {target}" + ) + if toolkit_exports is not None: + for name in sorted( + _nonpublic_toolkit_imports(node, package, toolkit_exports) + ): + problems.append( + f"{where}: workflow slice {workflow_plugin!r} imports non-public toolkit name {name!r}" + ) + if _imports_toolkit_module(node, package): + problems.append( + f"{where}: workflow slice {workflow_plugin!r} must import public toolkit names, not the toolkit module" + ) return problems diff --git a/tests/unit/test_import_layering.py b/tests/unit/test_import_layering.py index 7845cd93..d07eafb1 100644 --- a/tests/unit/test_import_layering.py +++ b/tests/unit/test_import_layering.py @@ -31,6 +31,9 @@ def _fixture(tmp_path: Path, source: str, statement: str) -> tuple[Path, Path]: directory.mkdir(parents=True, exist_ok=True) (directory / "__init__.py").write_text("", encoding="utf-8") (backend / "main.py").write_text("app = object()\n", encoding="utf-8") + (backend / "workflows" / "toolkit.py").write_text( + "__all__ = ['forced_tool_call']\n", encoding="utf-8" + ) source_path = backend / f"{source}.py" source_path.parent.mkdir(parents=True, exist_ok=True) source_path.write_text(statement, encoding="utf-8") @@ -62,6 +65,69 @@ def test_feature_slices_cannot_import_peers(tmp_path: Path): assert any("feature slice 'alpha' imports peer slice 'beta'" in problem for problem in problems), problems +@pytest.mark.parametrize( + ("statement", "target"), + [ + ("from backend.prompting import build_prefix\n", "backend.prompting"), + ("from backend.inference import LLMClient\n", "backend.inference"), + ("from backend.database import get_settings\n", "backend.database"), + ("from backend.workflows.peer import workflow\n", "backend.workflows.peer"), + ("from backend.workflows.registry import Workflow\n", "backend.workflows.registry"), + ("from backend.workflows.contracts import ToolSpec\n", "backend.workflows.contracts"), + ( + "from backend.workflows.attachment_cache import insert_workflow_attachment\n", + "backend.workflows.attachment_cache", + ), + ], +) +def test_workflow_slices_import_only_their_api(tmp_path: Path, statement: str, target: str): + root, backend = _fixture(tmp_path, "workflows/plugin/bad", statement) + problems = _checker().check(root=root, backend=backend) + assert any( + f"workflow slice 'plugin' may import only its own package or workflow APIs, not {target}" in problem + for problem in problems + ), problems + + +def test_workflow_slices_may_import_own_package_and_public_apis(tmp_path: Path): + root, backend = _fixture( + tmp_path, + "workflows/plugin/good", + """from backend.workflows.toolkit import forced_tool_call +from backend.workflows.plugin.local import helper +""", + ) + assert _checker().check(root=root, backend=backend) == [] + + +def test_workflow_framework_modules_remain_host_adapters(tmp_path: Path): + root, backend = _fixture( + tmp_path, + "workflows/toolkit", + "__all__ = []\nfrom backend.prompting import build_prefix\n", + ) + assert _checker().check(root=root, backend=backend) == [] + + +@pytest.mark.parametrize( + "statement", + [ + "from backend.workflows.toolkit import _local_ml\n", + "from backend.workflows.toolkit import *\n", + "import backend.workflows.toolkit as toolkit\n", + "from backend.workflows import toolkit\n", + "import backend.workflows as workflows\n", + "from backend import workflows\n", + ], +) +def test_workflow_slices_use_only_named_public_toolkit_exports( + tmp_path: Path, statement: str +): + root, backend = _fixture(tmp_path, "workflows/plugin/bad", statement) + problems = _checker().check(root=root, backend=backend) + assert any("workflow slice 'plugin'" in problem for problem in problems), problems + + def test_python_packages_must_be_classified(tmp_path: Path): root, backend = _fixture(tmp_path, "inference/good", "from backend.core import value\n") unknown = backend / "mystery" diff --git a/tests/unit/workflows/test_toolkit_surface.py b/tests/unit/workflows/test_toolkit_surface.py index ad1e349e..0117e1f4 100644 --- a/tests/unit/workflows/test_toolkit_surface.py +++ b/tests/unit/workflows/test_toolkit_surface.py @@ -18,6 +18,26 @@ "workflow_config_lock", ) +_LOWER_LAYER_INTERNALS = ( + "LLMClient", + "STANDALONE_TOOLS", + "TOOLS", + "build_prefix", + "enabled_schemas", + "format_message_with_attachments", + "local_ml", + "parse_tool_calls", + "reasoning_cfg", +) + +_PLUGIN_CONTRACTS = ( + "EV_DRAFT_REPLACED", + "ToolSpec", + "Workflow", + "WorkflowEventStream", + "WorkflowUserFacingError", +) + def test_locks_exported_from_toolkit(): for name in _LOCK_NAMES: @@ -28,3 +48,20 @@ def test_locks_exported_from_toolkit(): def test_toolkit_locks_are_canonical(): for name in _LOCK_NAMES: assert getattr(toolkit, name) is getattr(locks, name), f"{name} is not the backend.core.locks object" + + +def test_toolkit_does_not_expose_lower_layer_implementation_objects(): + for name in _LOWER_LAYER_INTERNALS: + assert not hasattr(toolkit, name), f"{name} leaks through the workflow API" + assert name not in toolkit.__all__ + + +def test_toolkit_exposes_local_ml_as_narrow_capabilities(): + assert "local_feature_available" in toolkit.__all__ + assert "classify_pov" in toolkit.__all__ + + +def test_toolkit_is_the_single_plugin_contract_surface(): + for name in _PLUGIN_CONTRACTS: + assert hasattr(toolkit, name) + assert name in toolkit.__all__ From 45760a0f61c9c40e2ef5c4ba1bf147b5b9fd2fd2 Mon Sep 17 00:00:00 2001 From: Chi Date: Sun, 6 Sep 2026 20:07:18 +0700 Subject: [PATCH 6/6] format + lint --- backend/prompting/tool_catalog.py | 10 ++-------- backend/workflows/toolkit.py | 4 +--- tests/unit/test_import_layering.py | 8 ++------ 3 files changed, 5 insertions(+), 17 deletions(-) diff --git a/backend/prompting/tool_catalog.py b/backend/prompting/tool_catalog.py index b8236049..f563ceac 100644 --- a/backend/prompting/tool_catalog.py +++ b/backend/prompting/tool_catalog.py @@ -123,9 +123,7 @@ def is_standalone_tool(name: str) -> bool: return name in _standalone_tools -def register_tool( - name: str, schema: dict, choice: dict, *, standalone: bool = False -) -> None: +def register_tool(name: str, schema: dict, choice: dict, *, standalone: bool = False) -> None: """Register or replace a tool while preserving an existing position.""" _tools[name] = {"schema": deepcopy(schema), "choice": deepcopy(choice)} if standalone: @@ -166,8 +164,4 @@ def enabled_schemas( eligible = [name for name in _tools if name not in _standalone_tools] if enabled_tools is not None: eligible = [name for name in eligible if enabled_tools.get(name, False)] - return [ - deepcopy(schema) - for name in eligible - if (schema := overrides.get(name, _tools[name]["schema"])) is not None - ] + return [deepcopy(schema) for name in eligible if (schema := overrides.get(name, _tools[name]["schema"])) is not None] diff --git a/backend/workflows/toolkit.py b/backend/workflows/toolkit.py index 697b1acf..8538de0c 100644 --- a/backend/workflows/toolkit.py +++ b/backend/workflows/toolkit.py @@ -181,9 +181,7 @@ async def build_offturn_prefix( history, macros, user_description, - constant_lorebook_block=_compute_constant_lorebook_block( - await get_active_lorebook_entries(), macros - ), + constant_lorebook_block=_compute_constant_lorebook_block(await get_active_lorebook_entries(), macros), cast=turn_cast, speaker_names=speaker_names, ) diff --git a/tests/unit/test_import_layering.py b/tests/unit/test_import_layering.py index d07eafb1..da3c12fd 100644 --- a/tests/unit/test_import_layering.py +++ b/tests/unit/test_import_layering.py @@ -31,9 +31,7 @@ def _fixture(tmp_path: Path, source: str, statement: str) -> tuple[Path, Path]: directory.mkdir(parents=True, exist_ok=True) (directory / "__init__.py").write_text("", encoding="utf-8") (backend / "main.py").write_text("app = object()\n", encoding="utf-8") - (backend / "workflows" / "toolkit.py").write_text( - "__all__ = ['forced_tool_call']\n", encoding="utf-8" - ) + (backend / "workflows" / "toolkit.py").write_text("__all__ = ['forced_tool_call']\n", encoding="utf-8") source_path = backend / f"{source}.py" source_path.parent.mkdir(parents=True, exist_ok=True) source_path.write_text(statement, encoding="utf-8") @@ -120,9 +118,7 @@ def test_workflow_framework_modules_remain_host_adapters(tmp_path: Path): "from backend import workflows\n", ], ) -def test_workflow_slices_use_only_named_public_toolkit_exports( - tmp_path: Path, statement: str -): +def test_workflow_slices_use_only_named_public_toolkit_exports(tmp_path: Path, statement: str): root, backend = _fixture(tmp_path, "workflows/plugin/bad", statement) problems = _checker().check(root=root, backend=backend) assert any("workflow slice 'plugin'" in problem for problem in problems), problems