Skip to content

feat(system-prompt): add prompt profiles with CRUD, section options, and model overrides#466

Open
penso wants to merge 3 commits intomainfrom
settings-system-prompt
Open

feat(system-prompt): add prompt profiles with CRUD, section options, and model overrides#466
penso wants to merge 3 commits intomainfrom
settings-system-prompt

Conversation

@penso
Copy link
Copy Markdown
Collaborator

@penso penso commented Mar 23, 2026

Summary

  • Add configurable system prompt profiles with full CRUD lifecycle (create, delete, set-default) via 6 new system_prompt.config.* RPC methods
  • Per-profile section options (runtime, user details, memory bootstrap, datetime tail), enabled sections toggle grid, template variables with insertion, and live preview with token estimation
  • Model overrides UI with glob-based provider/model routing to different prompt profiles, backed by flattened [[prompt_profiles.overrides]] TOML schema
  • New typed config (PromptProfilesConfig, PromptSectionId, PromptSectionOptions, etc.) in moltis-config, profile-aware prompt building in moltis-agents, profile resolution and RPC handlers in moltis-chat

Validation

Completed

  • cargo +nightly-2025-11-30 fmt --all -- --check passes
  • cargo +nightly-2025-11-30 clippy --workspace --all-targets -- -D warnings passes
  • cargo check --workspace passes
  • cargo test -p moltis-chat -p moltis-config -p moltis-agents -p moltis-gateway passes (128+ tests, 0 failures)
  • biome check passes on all JS files
  • taplo fmt passes
  • Tailwind CSS rebuilt (crates/web/ui)

Remaining

  • ./scripts/local-validate.sh <PR_NUMBER>
  • E2E tests: cd crates/web/ui && npx playwright test e2e/specs/settings-nav.spec.js

Manual QA

  1. Navigate to Settings > System Prompt
  2. Verify default profile loads with template editor and live preview
  3. Create a new profile ("test-profile"), verify it appears in the dropdown
  4. Toggle section options (Show/Hide Section Options) and verify checkboxes
  5. Toggle enabled sections grid, verify required sections are locked
  6. Insert a template variable from the collapsible variables list
  7. Set the new profile as default, then restore original default
  8. Open model overrides panel, add/remove override rows
  9. Delete the test profile
  10. Verify no JS console errors throughout

penso added 2 commits March 23, 2026 15:31
…and model overrides

- Add configurable system prompt profiles with full CRUD lifecycle via
  6 new system_prompt.config.* RPC methods
- Per-profile section options, enabled sections toggle grid, template
  variables with insertion, and live preview with token estimation
- Model overrides UI with glob-based provider/model routing to
  different prompt profiles
- New typed config (PromptProfilesConfig, PromptSectionId, etc.) in
  moltis-config, profile-aware prompt building in moltis-agents,
  profile resolution and RPC handlers in moltis-chat

Entire-Checkpoint: 63e4befced95
@codspeed-hq
Copy link
Copy Markdown
Contributor

codspeed-hq bot commented Mar 23, 2026

Merging this PR will degrade performance by 11.95%

❌ 3 regressed benchmarks
✅ 36 untouched benchmarks
⏩ 5 skipped benchmarks1

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
config_serde_roundtrip 1.5 ms 1.7 ms -11.95%
config_validate_toml 2.1 ms 2.4 ms -11.15%
full_config_boot_path 3.6 ms 4 ms -10.27%

Comparing settings-system-prompt (6030745) with main (0d6a5d7)

Open in CodSpeed

Footnotes

  1. 5 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 23, 2026

Greptile Summary

This PR adds a complete system-prompt profile management system: typed config structs (PromptProfilesConfig, PromptSectionOptions, etc.) in moltis-config, profile-aware prompt assembly in moltis-agents, glob-based model/provider override resolution in moltis-chat, six new RPC methods in moltis-gateway, and a full settings UI with CRUD, section toggles, live preview, and model overrides.

Key issues found:

  • CHANGELOG.md manual entries — The PR directly edits CHANGELOG.md, which the project's CLAUDE.md explicitly prohibits. Entries must be auto-generated via git-cliff from conventional commit messages; the CI guard script (scripts/check-changelog-guard.sh) will flag this.
  • system_prompt.config.create silently succeeds for duplicate names — When a profile with the same name already exists the update_config closure returns early (no-op), but update_config still returns Ok. The handler then returns a success response and the UI displays Profile "X" created. — giving the user false confirmation that a new profile was created.
  • system_prompt.config.update acts as an upsert — If the provided profile name does not exist, the handler creates a brand-new PromptProfileConfig instead of returning an error. This bypasses the explicit create flow and can silently produce phantom profiles on name typos.
  • enabled_sections: null coercion prevents clearing all sections — In onSave, an empty enabledSections array is sent as null. The server's parse_enabled_sections treats null as "key absent — no change", so a user who removes all toggleable sections will see their change silently dropped.

Confidence Score: 3/5

  • Not safe to merge as-is — one policy violation and two RPC handlers with logic issues that produce misleading feedback.
  • The core Rust config types, schema validation, glob-based override resolution, and prompt-building logic are well-structured and correctly implemented. However, the create handler's silent duplicate no-op, the update handler's implicit upsert, the enabled_sections null coercion in the JS, and the prohibited CHANGELOG edits each need to be addressed before merging. None of these represent data-loss or security risks, but two of them produce misleading success responses visible to end users.
  • crates/gateway/src/methods/services.rs (create/update RPC handlers), crates/web/src/assets/js/page-settings.js (enabled_sections null coercion), CHANGELOG.md (manual entries must be removed).

Important Files Changed

Filename Overview
crates/gateway/src/methods/services.rs Adds 6 new RPC handlers for system prompt profile CRUD. Two logic issues: create silently no-ops on duplicate names while returning success, and update acts as an upsert (creates new profiles when the name is not found).
crates/web/src/assets/js/page-settings.js New SystemPromptSection Preact component with profile CRUD, section toggles, template editor, live preview, and model overrides UI. enabled_sections is coerced to null when empty, preventing an explicit section clear from being persisted.
CHANGELOG.md Manually adds changelog entries, which violates the project policy of auto-generating CHANGELOG.md from conventional commits via git-cliff.
crates/config/src/schema.rs Adds well-structured PromptProfilesConfig, PromptProfileConfig, PromptProfileOverride, PromptSectionId, and PromptSectionOptions types with sane defaults and backward-compatible legacy [match] deserialization.
crates/config/src/validate.rs Schema map correctly updated with all new prompt_profiles fields including nested section_options and overrides entries; schema drift guard test should pass.
crates/chat/src/lib.rs Adds profile resolution functions (resolve_prompt_profile, resolve_prompt_profile_by_name_or_default), a hand-rolled glob matcher (prompt_glob_matches) with tests, and apply_section_options. The glob implementation and fallback logic look correct.
crates/agents/src/prompt.rs Profile-aware prompt building correctly falls back to defaults when enabled_sections is empty, and always includes required sections (Guidelines, AvailableTools, ToolCallGuidance). Logic is sound.
crates/config/src/template.rs Default config template updated with full [[prompt_profiles.profiles]] documentation including section_options and override examples; kept in sync with the new schema.
crates/web/ui/e2e/specs/settings-nav.spec.js New E2E spec for settings navigation; uses proper getByRole and getByText selectors, no waitForTimeout calls, uses shared helpers. Note the PR description marks the system-prompt E2E test file as still remaining (not run).

Sequence Diagram

sequenceDiagram
    participant UI as Web UI (SystemPromptSection)
    participant RPC as RPC Gateway
    participant Config as moltis-config
    participant Agent as moltis-agents (prompt.rs)

    UI->>RPC: system_prompt.config.create {name, description}
    RPC->>Config: update_config(|cfg| push new profile)
    Config-->>RPC: Ok
    RPC-->>UI: success + full config payload

    UI->>RPC: system_prompt.config.update {profile, prompt_template, section_options, enabled_sections}
    RPC->>Config: update_config(|cfg| find & update profile)
    Config-->>RPC: Ok
    RPC-->>UI: success + full config payload

    UI->>RPC: system_prompt.config.set_default {name}
    RPC->>Config: discover_and_load() — validate profile exists
    RPC->>Config: update_config(|cfg| set default)
    Config-->>RPC: Ok
    RPC-->>UI: success + full config payload

    UI->>RPC: system_prompt.config.delete {name}
    RPC->>Config: update_config(|cfg| retain if not name)
    Config-->>RPC: Ok
    RPC->>Config: discover_and_load() — verify deletion
    RPC-->>UI: success or error

    UI->>RPC: system_prompt.config.overrides.save {overrides[]}
    RPC->>Config: update_config(|cfg| replace overrides)
    Config-->>RPC: Ok
    RPC-->>UI: success + full config payload

    UI->>RPC: chat.raw_prompt {preview_mode, prompt_profile, ...}
    RPC->>Agent: resolve_prompt_profile(provider, model)
    Agent-->>RPC: PromptProfileResolution
    RPC->>Agent: build_system_prompt_with_profile(...)
    Agent-->>RPC: rendered prompt string
    RPC-->>UI: {prompt, char_count, estimated_tokens}
Loading

Comments Outside Diff (1)

  1. CHANGELOG.md, line 303-358 (link)

    P2 Manual CHANGELOG entries violate project policy

    The CHANGELOG.md is being manually edited in this PR. Per the project's CLAUDE.md guide:

    Do not add manual CHANGELOG.md entries in normal PRs.
    CHANGELOG.md entries are generated from commit history via git-cliff (cliff.toml).
    PR CI enforces this via scripts/check-changelog-guard.sh.

    Manual entries here will conflict with the auto-generated output from git-cliff and are explicitly prohibited by the engineering guide. These lines should be removed; the conventional commit message (feat(system-prompt): ...) will produce the correct entry automatically.

    Context Used: CLAUDE.md (source)

Reviews (1): Last reviewed commit: "fix(ci): revert changelog changes (gener..." | Re-trigger Greptile

Comment on lines +4313 to +4333
if let Err(error) = moltis_config::update_config(|cfg| {
if cfg.prompt_profiles.profiles.iter().any(|p| p.name == name) {
return;
}
cfg.prompt_profiles
.profiles
.push(moltis_config::PromptProfileConfig {
name: name.clone(),
description,
..moltis_config::PromptProfileConfig::default()
});
}) {
return Err(ErrorShape::new(
error_codes::UNAVAILABLE,
format!("failed to persist system prompt config: {error}"),
));
}

MethodRegistry::build_system_prompt_config_response()
})
}),
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.

P2 Silent no-op returns success for duplicate profile names

When a profile with the same name already exists, the update_config closure returns early without creating anything. However, update_config itself still returns Ok, so this handler falls through to build_system_prompt_config_response() and returns a success response. The JS caller then shows Profile "${name}" created. even though nothing was actually created.

This misleads the user into thinking a new profile was created when in fact the existing one was untouched. Since system_prompt.config.create is explicitly a create operation (not an upsert), a duplicate name should be rejected with a clear error:

if let Err(error) = moltis_config::update_config(|cfg| {
    if cfg.prompt_profiles.profiles.iter().any(|p| p.name == name) {
        return;
    }
    cfg.prompt_profiles
        .profiles
        .push(moltis_config::PromptProfileConfig {
            name: name.clone(),
            description,
            ..moltis_config::PromptProfileConfig::default()
        });
}) {

Consider checking for the duplicate before calling update_config and returning an INVALID_REQUEST error:

{
    let config = moltis_config::discover_and_load();
    if config.prompt_profiles.profiles.iter().any(|p| p.name == name) {
        return Err(ErrorShape::new(
            error_codes::INVALID_REQUEST,
            format!("profile '{name}' already exists"),
        ));
    }
}

Comment on lines +4241 to +4278
if let Some(profile) = cfg
.prompt_profiles
.profiles
.iter_mut()
.find(|profile| profile.name == profile_name)
{
if let Some(template) = prompt_template.clone() {
profile.prompt_template = template;
}
if let Some(template) = prompt_tail_template.clone() {
profile.prompt_tail_template = template;
}
if let Some(opts) = section_options {
moltis_chat::apply_section_options(&mut profile.section_options, opts);
}
if let Some(ref sections) = enabled_sections {
profile.enabled_sections.clone_from(sections);
}
return;
}

let mut profile = moltis_config::PromptProfileConfig {
name: profile_name.clone(),
..moltis_config::PromptProfileConfig::default()
};
if let Some(template) = prompt_template.clone() {
profile.prompt_template = template;
}
if let Some(template) = prompt_tail_template.clone() {
profile.prompt_tail_template = template;
}
if let Some(opts) = section_options {
moltis_chat::apply_section_options(&mut profile.section_options, opts);
}
if let Some(ref sections) = enabled_sections {
profile.enabled_sections.clone_from(sections);
}
cfg.prompt_profiles.profiles.push(profile);
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.

P2 update silently creates a new profile when the name doesn't exist

When the profile named profile_name is not found in the profiles list, the closure falls through to create a brand-new PromptProfileConfig and pushes it. This means system_prompt.config.update acts as an upsert.

This is unexpected: a caller with a typo in the profile name will silently create a phantom profile instead of getting an error. The explicit system_prompt.config.create RPC exists for creation; update should reject unknown profile names:

if let Some(profile) = cfg
    .prompt_profiles
    .profiles
    .iter_mut()
    .find(|profile| profile.name == profile_name)
{
    // ... apply changes ...
    return;
}

// ⚠️ This block silently creates a new profile if the name is unknown.
let mut profile = moltis_config::PromptProfileConfig {
    name: profile_name.clone(),
    ..moltis_config::PromptProfileConfig::default()
};

Consider returning an error here instead of pushing a new profile.

prompt_template: normalizedTemplateValue(promptTemplate),
prompt_tail_template: normalizedTemplateValue(promptTailTemplate),
section_options: buildSectionOptionsPayload(),
enabled_sections: enabledSections.length ? enabledSections : null,
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.

P2 Empty enabled_sections array is coerced to null, preventing a full section clear

When all toggleable sections are unchecked (or when a profile loads with an empty enabled_sections), enabledSections.length is 0, so the client sends enabled_sections: null. On the server, null is treated as "key absent — no change" by parse_enabled_sections, so the existing section list is never updated to the intended empty value.

In practice the prompt builder falls back to defaults when enabled_sections is empty, so the real-world impact is limited. However, the null-coercion creates a silent discrepancy between the user's intent and what gets persisted. Sending an empty array explicitly would be more accurate:

Suggested change
enabled_sections: enabledSections.length ? enabledSections : null,
enabled_sections: enabledSections,

The server's parse_enabled_sections already handles an actual empty array correctly (it returns Some(vec![]), which does update the stored list).

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 23, 2026

Codecov Report

❌ Patch coverage is 75.14124% with 396 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crates/gateway/src/methods/services.rs 4.24% 271 Missing ⚠️
crates/chat/src/lib.rs 80.70% 71 Missing ⚠️
crates/gateway/src/methods/mod.rs 0.00% 34 Missing ⚠️
crates/agents/src/prompt.rs 97.42% 20 Missing ⚠️

📢 Thoughts on this report? Let us know!

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages bot commented Mar 23, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
withered-breeze-e956 6030745 Commit Preview URL

Branch Preview URL
Mar 23 2026, 04:07 PM

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