Skip to content

feat(prompts): allow embedders to override constitutional prompt text via OnceLock hooks#2356

Open
h3c-hexin wants to merge 1 commit into
Hmbown:mainfrom
h3c-hexin:pr2/prompt-override-hooks
Open

feat(prompts): allow embedders to override constitutional prompt text via OnceLock hooks#2356
h3c-hexin wants to merge 1 commit into
Hmbown:mainfrom
h3c-hexin:pr2/prompt-override-hooks

Conversation

@h3c-hexin
Copy link
Copy Markdown
Contributor

@h3c-hexin h3c-hexin commented May 29, 2026

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, or AUTHORITY_RECAP is 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 OnceLock and 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 -- --check
  • cargo build -p codewhale-tui (compiles)
  • cargo clippy --workspace --all-targets --all-features
  • cargo test --workspace --all-features

Checklist

  • Updated docs or comments as needed
  • Added or updated tests where relevant (mechanism is opt-in; default path unchanged)
  • Verified TUI behavior manually if UI changes (n/a)

Greptile Summary

Adds three process-global, set-once (OnceLock) override hooks for BASE_PROMPT, LOCALE_PREAMBLE_ZH_HANS, and AUTHORITY_RECAP, each backed by a small effective_*() accessor that falls back to the bundled constant when no override is installed.

  • compose_prompt_with_approval_and_model and system_prompt_for_mode_with_context_skills_session_and_approval are updated to call the accessors instead of the constants directly; default behavior is unchanged.
  • The locale "bookend" pattern is only half overridable: the zh-Hans opening preamble can be replaced, but the matching zh-Hans closer (LOCALE_CLOSER_ZH_HANS) still always uses the bundled constant; the ja and pt-BR preambles have no override path at all.
  • set_* functions silently discard the OnceLock::set result, 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

Filename Overview
crates/tui/src/prompts.rs Adds three OnceLock-backed prompt override functions with fallback-to-constant accessors; used correctly in compose_prompt_with_approval_and_model and system_prompt_for_mode_with_context_skills_session_and_approval. zh-Hans preamble override is asymmetric (opener overridable, closer is not), and ja/pt-BR preamble overrides are absent. Silent no-op return on repeated set_* calls is undiscoverable to callers.

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]
Loading

Fix All in Codex Fix All in Claude Code Fix All in Cursor

Reviews (1): Last reviewed commit: "feat(prompts): allow embedders to overri..." | Re-trigger Greptile

Greptile also left 3 inline comments on this PR.

… 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>
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread crates/tui/src/prompts.rs
Comment on lines +243 to +255
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);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
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)
}

Comment thread crates/tui/src/prompts.rs
Comment on lines +237 to +239
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();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Comment thread crates/tui/src/prompts.rs
Comment on lines +248 to +250
pub fn set_locale_preamble_zh_hans_override(s: String) {
let _ = LOCALE_PREAMBLE_ZH_HANS_OVERRIDE.set(s);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.

Fix in Codex Fix in Claude Code Fix in Cursor

Comment thread crates/tui/src/prompts.rs
Comment on lines +243 to +254
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);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.

Fix in Codex Fix in Claude Code Fix in Cursor

Comment thread crates/tui/src/prompts.rs
Comment on lines +248 to +250
pub fn set_locale_preamble_zh_hans_override(s: String) {
let _ = LOCALE_PREAMBLE_ZH_HANS_OVERRIDE.set(s);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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!

Fix in Codex Fix in Claude Code Fix in Cursor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant