Skip to content

Support model prompts in both models.json and settings.json - #485

Merged
m-aebrer merged 3 commits into
masterfrom
feature/issue-484-model-prompts-config-sources
Aug 20, 2026
Merged

Support model prompts in both models.json and settings.json#485
m-aebrer merged 3 commits into
masterfrom
feature/issue-484-model-prompts-config-sources

Conversation

@m-aebrer

Copy link
Copy Markdown
Collaborator

Closes #484

Allow model-specific system-prompt replacement or append behavior to be configured with custom models and built-in model overrides in models.json, while retaining settings.json support and rejecting source conflicts loudly.

Implementation plan posted as a comment below.

@m-aebrer

Copy link
Copy Markdown
Collaborator Author

Implementation Plan

Problem analysis

The v2.60.0 implementation resolves model-specific prompt behavior only from canonical provider/model entries in settings.json. models.json already owns custom model definitions and per-model overrides for built-ins, but its two model schemas discard prompt fields and the registry returns no prompt metadata for AgentSession to compose. This forces a custom model's transport/capability definition and behavioral prompt into separate files.

Extend the coding-agent model metadata loaded from models.json, then keep final source resolution and prompt composition centralized in AgentSession. The existing settings.json behavior and precedence remain intact. A prompt declaration from models.json and any prompt declaration for the same canonical model in settings.json are an error regardless of whether the modes match; dreb must not invent precedence between files.

Deliverables

  1. Add optional systemPrompt and appendSystemPrompt fields to both custom models[] entries and built-in modelOverrides entries in models.json.
  2. Preserve those fields as coding-agent-local model prompt metadata when custom models are materialized and built-in overrides are applied, without coupling prompt composition to the generic provider streaming layer.
  3. Validate prompt fields as non-empty strings and reject replacement-plus-append declarations within a single models.json model entry.
  4. Centralize active-model prompt resolution in AgentSession: combine model metadata with SettingsManager only after validating that exactly one source owns prompt behavior for that canonical provider/model.
  5. Preserve current semantics and ordering: explicit session replacement sources remain stronger than model replacement; append behavior retains the selected base prompt and follows existing append sources.
  6. Apply the unified resolver during startup, direct switching, both cycling paths, session restore, and reload. Validate candidates before mutating model/session/default state or tearing down a live runtime.
  7. Refresh models.json during the normal reload path so edits and removals affect the active model's prompt without a process restart, while surfacing invalid configuration loudly.
  8. Document both configuration surfaces, examples for custom and built-in models, precedence, exact canonical identity, reload behavior, and the cross-source conflict rule across every public documentation layer.

Acceptance criteria

  • Existing canonical provider/model entries under settings.json modelSettings continue to support replacement and append behavior unchanged.
  • A custom model can define its transport, capabilities, and exactly one prompt mode in one models.json object, with no matching settings entry.
  • A built-in model can receive exactly one prompt mode through its provider's modelOverrides entry.
  • Replacement removes the built-in base prompt while retaining normal runtime context; append preserves the selected base and follows existing append sources.
  • Explicit CLI/file/SDK session replacement still wins over model replacement, while model append still composes with that explicit base.
  • Model IDs containing / are matched by exact provider plus opaque model ID, without ambiguous string splitting or cross-provider leakage.
  • Empty/malformed prompt values and replacement-plus-append declarations fail loudly.
  • Any prompt declaration in both files for the same canonical model fails loudly rather than selecting precedence, including same-mode and mixed-mode combinations.
  • Invalid target configuration is rejected atomically during direct selection, cycling, restore, and reload; stale instructions do not survive a successful model change or configuration refresh.
  • Removing or changing a models.json prompt and reloading updates the active prompt.

Files to modify

  • packages/coding-agent/src/core/model-registry.ts — extend both TypeBox schemas and coding-agent model metadata; validate prompt declarations; preserve prompt fields through custom-model parsing, built-in override application, registry refresh, and removal.
  • packages/coding-agent/src/core/agent-session.ts — resolve model metadata and SettingsManager prompt sources centrally; reject cross-source conflicts; reuse the resolver for composition and atomic preflight across startup, switching/cycling, restore, and reload; refresh/rebind active registry metadata on reload.
  • packages/coding-agent/test/model-registry.test.ts — cover custom and built-in prompt metadata, replacement/append validation, exact identities, refresh changes, and removal/restoration behavior.
  • packages/coding-agent/test/agent-session-model-switch-thinking.test.ts — extend integration coverage for both models.json paths, source conflicts, composition semantics, switching/cycling, restore, and reload atomicity.
  • README.md — state that per-model prompts can live in either configuration file and link to the detailed conflict rules.
  • packages/coding-agent/README.md — update the System Prompt guidance with both sources and precedence/conflict behavior.
  • packages/coding-agent/docs/models.md — add both fields to custom-model and modelOverrides references, with complete examples and validation/reload notes.
  • packages/coding-agent/docs/settings.md — retain the canonical modelSettings reference while documenting the equivalent models.json surface and no-precedence conflict rule.

No new files are expected.

Testing approach

  • Extend model-registry.test.ts to verify:
    • custom models[] replacement and append metadata are retained on the resolved model;
    • built-in modelOverrides replacement and append metadata affect only the exact target;
    • invalid types, blank values, and both modes in one entry produce loud model/file-specific errors;
    • refresh() picks up changed prompt metadata and removes stale metadata when configuration is removed.
  • Extend agent-session-model-switch-thinking.test.ts to verify:
    • startup replacement and append semantics from custom-model metadata and built-in overrides;
    • explicit replacement plus model append composition and recognizable built-in-base absence/presence;
    • same-source and cross-source conflicts, including same-mode and mixed-mode declarations;
    • direct selection, scoped cycling, available-model cycling, and restore apply only the destination model's prompt and reject conflicts before state/default/history mutation;
    • /reload picks up models-file changes/removals and rejects malformed or cross-source-conflicting edits before runtime teardown or prompt/model mutation;
    • slash-containing model IDs and provider isolation remain exact.
  • Retain the existing settings-manager.test.ts canonical lookup, global/project merge, malformed-value, and settings-only behavior as regression coverage.
  • Run focused Vitest files and Biome checks for touched source/test files, then npm test and npm run check.
  • Run the required npm run build, followed by npm run verify-workspace-links.
  • After building, exercise the compiled dreb -p binary with a custom model prompt configured solely in models.json, and verify a deliberate dual-source conflict fails loudly.

Risks and open questions

  • Metadata preservation: model objects are copied in registry overrides and context-tier handling. Prompt metadata must survive those copies without leaking into provider request payloads.
  • Reload atomicity: registry/settings refresh occurs alongside extension runtime rebuilds. Prompt-source validation must happen before teardown or live model/prompt mutation so a bad edit leaves the running session usable.
  • Custom-over-built-in replacement: a custom model with the same provider/id replaces an overridden built-in model. Its own prompt metadata must be the sole models.json declaration used after the existing upsert semantics.
  • Unknown built-in override IDs: retain current behavior for unknown IDs; they do not create models or active prompt targets.

No unresolved product decision remains: declarations in both files are always conflicts, and no implicit precedence is introduced.


Plan created by mach6

@github-actions

github-actions Bot commented Aug 20, 2026

Copy link
Copy Markdown

Vitest coverage

Metric Covered Total Coverage
Statements 40246 56267 71.52%
Branches 21705 35288 61.5%
Functions 8540 11801 72.36%
Lines 29106 40475 71.91%

View full coverage run

@m-aebrer

Copy link
Copy Markdown
Collaborator Author

Progress Update

Implemented model-specific system prompts from both configuration surfaces:

  • Added systemPrompt and appendSystemPrompt to custom models[] and built-in modelOverrides schemas.
  • Kept prompt metadata in a coding-agent registry sidecar so it is not serialized with RPC model state or passed through the generic provider layer.
  • Centralized prompt-source resolution in AgentSession, with loud rejection whenever one canonical model is configured in both models.json and settings.json.
  • Preserved replacement, append, explicit-session precedence, and exact slash-containing model-ID semantics.
  • Made startup, direct switching, both cycling paths, restore, and /reload validate before model/session/default mutation or runtime teardown.
  • Made /reload refresh changed and removed models.json prompt metadata.
  • Added registry and session regression coverage for custom models, built-in overrides, validation, source conflicts, atomic transitions, composition, restore, and reload.
  • Updated the root README, coding-agent README, custom-model docs, and settings docs.

Verification passed: focused tests, full npm test, npm run check, npm run build, workspace-link verification, compiled dreb -p QA, and the commit hook's complete non-live suite (5,824 passed).

Commit: 8536426


Progress tracked by mach6

@m-aebrer
m-aebrer marked this pull request as ready for review August 20, 2026 21:15
@m-aebrer

Copy link
Copy Markdown
Collaborator Author

Unverified Review Candidates — Pending Assessment

Review round: 1
Reviewed commit: 8536426

These are unverified candidates. Severity reflects reviewer confidence; do not treat any item as a merge blocker until the assessment comment is posted.

Critical

None.

Important

  1. A failed /reload may partially commit rejected settings and registry stateAgentSession.reload() mutates SettingsManager and refreshes the live ModelRegistry before checking registry errors or cross-source prompt conflicts (packages/coding-agent/src/core/agent-session.ts:3328-3339). A malformed models.json can therefore throw after the registry has dropped custom models, prompt metadata, and custom-provider API-key state, while valid concurrent settings.json edits are already live. The prompt and extension runtime remain intact, but subsequent model cycling, settings reads, or credential resolution can observe state from a reload reported as failed. Confidence: 90/100.

  2. Malformed models.json prompt settings may not be rejected when a model is activated — registry load failures install built-ins with an empty prompt sidecar, while _validateModelPromptSettings() only resolves that sidecar and settings (packages/coding-agent/src/core/model-registry.ts:377-409; packages/coding-agent/src/core/agent-session.ts:2199-2202). A built-in target whose override contains invalid prompt metadata could therefore activate without the intended prompt if the activation path does not separately surface ModelRegistry.getError(). Confidence: 86/100.

  3. Prompt metadata identity can collide when provider names contain / — the sidecar flattens (provider, modelId) to ${provider}/${modelId} (packages/coding-agent/src/core/model-registry.ts:174-176). The schema permits record keys such as provider team/proxy; that pair with model qwen collides with provider team and model proxy/qwen, allowing one prompt to overwrite or leak into the other and potentially creating a false cross-source conflict. Confidence: 92/100.

Suggestions

  1. The custom-over-built-in prompt precedence path lacks direct regression coverage — the merge deliberately deletes an overridden built-in prompt before applying a same-identity custom model's prompt or intentional absence (packages/coding-agent/src/core/model-registry.ts:390-398). Existing tests cover custom and built-in prompts separately, not this interaction, so a future merge-order regression could reintroduce stale instructions. Confidence: 89/100.

  2. The runtime typeof checks in validateModelPromptSettings() appear redundant — AJV schema validation already rejects non-string values before this module-private validator runs, while TypeScript narrows both fields to string | undefined; only the whitespace-only checks add behavior (packages/coding-agent/src/core/model-registry.ts:149-166). Removing the unreachable branches could simplify the validator. Confidence: 82/100.

Strengths

  • Prompt composition is centralized in AgentSession, preserving explicit replacement and append ordering.
  • Prompt metadata remains in a coding-agent sidecar rather than leaking into provider requests or RPC model state.
  • Switching, cycling, and restore paths preflight cross-source conflicts before mutating session/default state.
  • Tests cover both configuration surfaces, both prompt modes, same- and mixed-mode conflicts, switching paths, restore, successful reload changes/removals, and schema validation.
  • All required public documentation surfaces were updated, and the completeness review found every issue acceptance criterion represented.

Agents run: code-reviewer, error-auditor, test-reviewer, completeness-checker, simplifier


Reviewed by mach6

@m-aebrer

Copy link
Copy Markdown
Collaborator Author

Review Assessment

#485 (comment)

Classifications

Finding Classification Reasoning
finding 1 — failed /reload partially commits rejected state merge blocker Factual: reload() mutates live settings and registry state before checking registry errors and source conflicts. ModelRegistry.refresh() clears custom-provider API keys and replaces custom models/prompt metadata even when the subsequent check throws. Scope: this PR newly adds registry refresh to /reload; the approved plan requires invalid reload configuration to be rejected atomically and leave the running session usable. Practical: start on a custom model whose credential exists only in models.json, make the file invalid, then run /reload. The command loudly fails but the active session can no longer resolve that custom provider credential, so the supposedly preserved runtime cannot send its next request. Fixing it preserves the prior live settings/registry until staged configuration fully validates.
finding 2 — malformed prompt settings may not be rejected on activation false positive Factual: malformed models.json is surfaced at startup, when opening /model, and during /reload; direct selection and cycling do not independently reload the file. The empty sidecar only exists after an already-reported registry load error. Scope: loud rejection is required and occurs at the configuration-loading boundary. Practical: no separate reachable silent activation sequence was demonstrated beyond the partially committed failed-reload state already covered by finding 1.
finding 3 — flattened prompt identity can collide merge blocker Factual: both registry maps flatten two independently supplied strings to ${provider}/${modelId}. Provider team/proxy with model qwen and provider team with model proxy/qwen therefore share one key; the schema does not forbid / in provider names. Scope: the approved acceptance criteria require exact provider plus opaque model identity without ambiguous identity handling or cross-provider leakage. Practical: configuring those two otherwise valid custom identities with distinct prompts lets one overwrite the other, or lets a settings declaration for one produce a false source conflict for the other. The result is wrong behavioral instructions or inability to start a valid model configuration. Use a collision-free nested map or equivalent structured key.
finding 4 — custom-over-built-in precedence lacks direct coverage useful follow-up Factual: the explicit delete-then-set interaction is not directly tested. Scope: the custom-over-built-in behavior is relevant to the feature. Practical: current code implements the intended behavior correctly, and no present regression was identified; missing coverage alone is not a blocker. A focused test would protect the intentional absence-clears-override behavior.
finding 5 — redundant runtime typeof guards nitpick Factual: AJV rejects non-strings before the private validator, while the whitespace-only .trim() checks remain necessary. Scope: simplification is optional. Practical: the defensive branches are harmless and do not affect users or maintainability materially.

Action Plan

  1. Make /reload stage or snapshot settings and model-registry state, validate registry errors and active-model source conflicts, and commit only on success. Add an active custom-model test proving a failed reload preserves models.json-only credential resolution and registry contents.
  2. Replace flattened model-prompt keys with a collision-free representation of provider and opaque model ID. Add coverage for two distinct pairs that currently flatten to the same string and verify no prompt leakage or false cross-source conflict.

Assessment by mach6

@m-aebrer

Copy link
Copy Markdown
Collaborator Author

Maintainer Assessment Update

The maintainer reviewed the round-one assessment and adopted the independent assessor's practical classification: findings 1 and 3 are non-blocking nitpicks, not merge blockers. Finding 1 is a loud, recoverable reload-state imperfection whose core prompt/runtime guarantees remain intact; finding 3 requires a pathological colliding provider-name configuration and does not justify blocking this hotfix.

The PR is approved to proceed to publication as implemented.


Assessment updated by maintainer direction

@m-aebrer
m-aebrer merged commit b3ab73b into master Aug 20, 2026
3 checks passed
@m-aebrer
m-aebrer deleted the feature/issue-484-model-prompts-config-sources branch August 20, 2026 21:53
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.

Support model prompts in both models.json and settings.json

1 participant