feat(prompts): allow embedders to override constitutional prompt text via OnceLock hooks#2356
feat(prompts): allow embedders to override constitutional prompt text via OnceLock hooks#2356h3c-hexin wants to merge 1 commit into
Conversation
… via OnceLock hooks An application embedding this engine may want to ship its own constitutional preamble, locale preamble, or authority-recap block without forking the prompt constants. Today the only way to change `BASE_PROMPT`, `LOCALE_PREAMBLE_ZH_HANS`, or `AUTHORITY_RECAP` is to edit the bundled markdown/strings in this crate. Add three process-global, set-once override hooks: - `set_base_prompt_override(String)` - `set_locale_preamble_zh_hans_override(String)` - `set_authority_recap_override(String)` Each is backed by a `OnceLock` and read through a small accessor (`base_prompt()` / `locale_preamble_zh_hans()` / `authority_recap()`) that falls back to the existing default constant when no override is set. Prompt assembly now calls these accessors instead of the constants directly. Default builds are unaffected (no override set → upstream constant is used). Overrides are expected to be installed once at startup, before the engine builds any prompt; `set_*` returns `Err` with the supplied value if called twice. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces startup override mechanisms for compile-time prompt constants (BASE_PROMPT, LOCALE_PREAMBLE_ZH_HANS, and AUTHORITY_RECAP) using std::sync::OnceLock, allowing embedders to customize prompts without modifying the files in-tree. Feedback suggests returning a Result from the setter functions instead of silently ignoring failures when setting the OnceLock values. Additionally, it is recommended to use isolated integration tests rather than standard unit tests to prevent global state pollution and test flakiness.
| pub fn set_base_prompt_override(s: String) { | ||
| let _ = BASE_PROMPT_OVERRIDE.set(s); | ||
| } | ||
|
|
||
| /// Replace the Simplified-Chinese locale preamble (`## 语言要求`). | ||
| pub fn set_locale_preamble_zh_hans_override(s: String) { | ||
| let _ = LOCALE_PREAMBLE_ZH_HANS_OVERRIDE.set(s); | ||
| } | ||
|
|
||
| /// Replace the trailing `## Authority Recap` block. | ||
| pub fn set_authority_recap_override(s: String) { | ||
| let _ = AUTHORITY_RECAP_OVERRIDE.set(s); | ||
| } |
There was a problem hiding this comment.
Instead of silently ignoring the result of OnceLock::set, it is highly recommended to return Result<(), String>. This allows the embedder/caller to handle or log failures if they attempt to set the override multiple times or after initialization.
| pub fn set_base_prompt_override(s: String) { | |
| let _ = BASE_PROMPT_OVERRIDE.set(s); | |
| } | |
| /// Replace the Simplified-Chinese locale preamble (`## 语言要求`). | |
| pub fn set_locale_preamble_zh_hans_override(s: String) { | |
| let _ = LOCALE_PREAMBLE_ZH_HANS_OVERRIDE.set(s); | |
| } | |
| /// Replace the trailing `## Authority Recap` block. | |
| pub fn set_authority_recap_override(s: String) { | |
| let _ = AUTHORITY_RECAP_OVERRIDE.set(s); | |
| } | |
| pub fn set_base_prompt_override(s: String) -> Result<(), String> { | |
| BASE_PROMPT_OVERRIDE.set(s) | |
| } | |
| /// Replace the Simplified-Chinese locale preamble (## 语言要求). | |
| pub fn set_locale_preamble_zh_hans_override(s: String) -> Result<(), String> { | |
| LOCALE_PREAMBLE_ZH_HANS_OVERRIDE.set(s) | |
| } | |
| /// Replace the trailing ## Authority Recap block. | |
| pub fn set_authority_recap_override(s: String) -> Result<(), String> { | |
| AUTHORITY_RECAP_OVERRIDE.set(s) | |
| } |
| static BASE_PROMPT_OVERRIDE: std::sync::OnceLock<String> = std::sync::OnceLock::new(); | ||
| static LOCALE_PREAMBLE_ZH_HANS_OVERRIDE: std::sync::OnceLock<String> = std::sync::OnceLock::new(); | ||
| static AUTHORITY_RECAP_OVERRIDE: std::sync::OnceLock<String> = std::sync::OnceLock::new(); |
There was a problem hiding this comment.
Since OnceLock is process-global, calling these setters in standard unit tests will pollute the global state and cause other tests in the same test runner process to fail or behave flakily. To safely test these overrides, we should add integration tests in a separate test target (e.g., under tests/prompt_overrides.rs), which runs in its own isolated process.
| pub fn set_locale_preamble_zh_hans_override(s: String) { | ||
| let _ = LOCALE_PREAMBLE_ZH_HANS_OVERRIDE.set(s); | ||
| } |
There was a problem hiding this comment.
Asymmetric bookend override — closer not overridable
locale_reinforcement_preamble for zh-Hans now reads from LOCALE_PREAMBLE_ZH_HANS_OVERRIDE, but locale_reinforcement_closer (line ~368) still unconditionally returns LOCALE_CLOSER_ZH_HANS. An embedder who replaces the opening preamble (e.g., to reword the language directive) will get a closer that refers to the original codewhale framing — the two bookends will be semantically inconsistent. There's no set_locale_closer_zh_hans_override counterpart.
| pub fn set_base_prompt_override(s: String) { | ||
| let _ = BASE_PROMPT_OVERRIDE.set(s); | ||
| } | ||
|
|
||
| /// Replace the Simplified-Chinese locale preamble (`## 语言要求`). | ||
| pub fn set_locale_preamble_zh_hans_override(s: String) { | ||
| let _ = LOCALE_PREAMBLE_ZH_HANS_OVERRIDE.set(s); | ||
| } | ||
|
|
||
| /// Replace the trailing `## Authority Recap` block. | ||
| pub fn set_authority_recap_override(s: String) { | ||
| let _ = AUTHORITY_RECAP_OVERRIDE.set(s); |
There was a problem hiding this comment.
No signal when override is silently ignored
All three set_* functions discard the Result from OnceLock::set with let _ = .... If embedder startup code calls set_base_prompt_override more than once (or a library dependency also calls it), the second call becomes an undetected no-op — the caller cannot distinguish "successfully registered" from "already locked, your value was dropped." Returning bool (or Result<(), String>) matching OnceLock::set's own contract would let callers log a warning or panic-on-conflict at startup.
| pub fn set_locale_preamble_zh_hans_override(s: String) { | ||
| let _ = LOCALE_PREAMBLE_ZH_HANS_OVERRIDE.set(s); | ||
| } |
There was a problem hiding this comment.
Override coverage is uneven across locales
Only the zh-Hans locale preamble is overridable. LOCALE_PREAMBLE_JA and LOCALE_PREAMBLE_PT_BR (used in locale_reinforcement_preamble at lines 344–345) have no corresponding set_* or effective_* path. An embedder shipping a product that supports Japanese or Brazilian-Portuguese users faces the same motivating problem described in the PR description — needing to tweak locale preambles without forking the crate — but only gets partial coverage.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Summary
An application embedding this engine may want to ship its own constitutional preamble, locale preamble, or authority-recap block without forking the prompt constants. Today the only way to change
BASE_PROMPT,LOCALE_PREAMBLE_ZH_HANS, orAUTHORITY_RECAPis to edit the bundled markdown/strings in this crate.This adds three process-global, set-once override hooks:
set_base_prompt_override(String)set_locale_preamble_zh_hans_override(String)set_authority_recap_override(String)Each is backed by a
OnceLockand read through a small accessor (effective_base_prompt()/effective_locale_preamble_zh_hans()/effective_authority_recap()) that falls back to the existing default constant when no override is set. Prompt assembly now calls these accessors instead of the constants directly.Default builds are unaffected (no override set → bundled constant is used). Overrides are expected to be installed once at startup, before the engine builds any prompt; later
set_*calls are no-ops (OnceLock semantics).Testing
cargo fmt --all -- --checkcargo build -p codewhale-tui(compiles)cargo clippy --workspace --all-targets --all-featurescargo test --workspace --all-featuresChecklist
Greptile Summary
Adds three process-global, set-once (
OnceLock) override hooks forBASE_PROMPT,LOCALE_PREAMBLE_ZH_HANS, andAUTHORITY_RECAP, each backed by a smalleffective_*()accessor that falls back to the bundled constant when no override is installed.compose_prompt_with_approval_and_modelandsystem_prompt_for_mode_with_context_skills_session_and_approvalare updated to call the accessors instead of the constants directly; default behavior is unchanged.LOCALE_CLOSER_ZH_HANS) still always uses the bundled constant; thejaandpt-BRpreambles have no override path at all.set_*functions silently discard theOnceLock::setresult, giving callers no way to know whether their override was accepted or dropped.Confidence Score: 4/5
Safe to merge — the default path is byte-identical to before, and the new OnceLock accessors are straightforward Rust idioms.
The mechanism is correct: OnceLock on a static gives a 'static reference, the fallback-to-constant logic is simple, and both updated call sites use the new accessors consistently. The open items are that the zh-Hans closer has no override counterpart, ja/pt-BR preambles are not covered, and callers have no feedback on whether their set_* call actually took effect. None of these affect existing behavior.
crates/tui/src/prompts.rs — the three set_* / effective_* additions and the updated call sites in compose_prompt_with_approval_and_model and system_prompt_for_mode_with_context_skills_session_and_approval.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Embedder startup] -->|set_base_prompt_override| B[BASE_PROMPT_OVERRIDE OnceLock] A -->|set_locale_preamble_zh_hans_override| C[LOCALE_PREAMBLE_ZH_HANS_OVERRIDE OnceLock] A -->|set_authority_recap_override| D[AUTHORITY_RECAP_OVERRIDE OnceLock] B --> E[effective_base_prompt] C --> F[effective_locale_preamble_zh_hans] D --> G[effective_authority_recap] E -->|fallback| BP[BASE_PROMPT const] F -->|fallback| LP[LOCALE_PREAMBLE_ZH_HANS const] G -->|fallback| AR[AUTHORITY_RECAP const] E --> H[compose_prompt_with_approval_and_model] F --> I[locale_reinforcement_preamble zh-Hans branch] G --> J[system_prompt_for_mode step 7a] I2[locale_reinforcement_closer zh-Hans branch] -->|always uses| LC[LOCALE_CLOSER_ZH_HANS const]Reviews (1): Last reviewed commit: "feat(prompts): allow embedders to overri..." | Re-trigger Greptile