Skip to content

fix(config): Apply shared extends files only once - #887

Open
JeanMertz wants to merge 7 commits into
mainfrom
system-prompt-delta
Open

fix(config): Apply shared extends files only once#887
JeanMertz wants to merge 7 commits into
mainfrom
system-prompt-delta

Conversation

@JeanMertz

@JeanMertz JeanMertz commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

A config that reaches the same file through two extends branches (a diamond, e.g. a -> b -> d and a -> c -> d) now applies d once instead of once per branch. Previously each branch loaded the file independently, so an append/prepend merge strategy in the shared file duplicated its contribution to the final value. The extends graph is now resolved fully before any file is loaded, then reduced to one entry per file, keeping each file's last position so declared extends order (and precedence) is preserved.

Merged strings gain the same dedup opt-out already available on merged vecs: an append or prepend whose value is already present as a whole separator-delimited block is skipped, so re-supplying the same --cfg or hitting the same file through an extends diamond does not duplicate prompt text. Dedup now defaults to true for both merged vecs and merged strings (previously vecs defaulted to no dedup); set dedup = false to keep duplicates. The flag is sticky across merges and survives a plain-string replacement. Strategies that combine two sources (append, prepend) deduplicate by default; a replace does not, since its contents are a single source's own data — free-form JsonValue arrays such as tool options and template values are merged through the same path with an implicit replace and must survive untouched.

assistant.system_prompt accepts dotted overrides for its merge metadata (--cfg assistant.system_prompt.dedup=false, .strategy, .separator, .value, .discard_when_merged), which previously failed with UnknownKey. A plain-string prompt supplied by a lower layer is promoted to the merged form with replace pinned, preserving the scalar's meaning, and a metadata-only override no longer suppresses the built-in prompt (it gap-fills the default's value instead of resolving to an empty string). dedup = "inherit" is accepted on every input channel and is a no-op for key-value assignments: since those mutate the accumulated partial, writing "no opinion" would erase a lower layer's explicit choice. Returning to the default takes an explicit dedup=true.

schematic_macros now treats an invalid #[setting]/#[schema] attribute as a hard error instead of silently discarding the field's other keys, and adds deserialize_with support so a partial field can use a custom deserializer while staying optional. The default argument is parsed with a dedicated helper that distinguishes a bare default (fall back to Default::default(), matching what the generated code already emitted) from default = <expr>.

The hard error surfaced four fields whose keys had been silently dropped, all of which now behave as declared:

  • AppConfig::inherit drops its #[setting(optional)], which never did anything.
  • AppConfig::config_load_paths drops the same key, which was aborting the parse of the merge/transform keys beside it. Load paths now accumulate across config layers instead of the highest layer replacing the rest. --cfg <name> resolution takes the first directory in the list that holds a matching file, so a lower layer's directory can now win a name collision it previously lost.
  • StdioConfig::arguments and variables (MCP stdio servers) now append across layers, as merge = append_vec always declared.
  • ParametersConfig::stop_words now appends across layers for the same reason.

ParametersConfig::other keeps its merge strategy but stays a nested [parameters.other] table rather than being flattened: stored conversation configs carry a literal other key, which a flattened map would capture as a provider parameter named other. Flattening needs a compat migration first, tracked separately.

Also fixed: string_with_strategy treated an unstated strategy as "no strategy" and dropped the value entirely when both sides had one, so --cfg 'assistant.system_prompt:{"value":"x"}' wiped an existing prompt. An unstated strategy now resolves to append, matching MergedStringStrategy's default and the vec merge's handling.

A config that reaches the same file through two `extends` branches (a
diamond, e.g. `a -> b -> d` and `a -> c -> d`) now applies `d` once
instead of once per branch. Previously each branch loaded the file
independently, so an `append`/`prepend` merge strategy in the shared
file duplicated its contribution to the final value. The `extends`
graph is now resolved fully before any file is loaded, then reduced to
one entry per file, keeping each file's last position so declared
`extends` order (and precedence) is preserved.

Merged strings gain the same `dedup` opt-out already available on
merged vecs: an `append` or `prepend` whose value is already present as
a whole `separator`-delimited block is skipped, so re-supplying the
same `--cfg` or hitting the same file through an `extends` diamond does
not duplicate prompt text. Dedup now defaults to `true` for both merged
vecs and merged strings (previously vecs defaulted to no dedup); set
`dedup = false` to keep duplicates. The flag stays sticky across
merges, matching the existing vec behavior.

`schematic_macros` now treats an invalid `#[setting]`/`#[schema]`
attribute as a hard error instead of silently discarding the field's
other keys, and adds `deserialize_with` support so a partial field can
use a custom deserializer while staying optional. The `default`
argument is parsed with a dedicated helper that distinguishes a bare
`default` (fall back to `Default::default()`) from `default = <expr>`.

`AppConfig::inherit` and `config_load_paths` drop their now-redundant
`#[setting(optional)]`, and `parameters.other` stays a nested
`[parameters.other]` table rather than being flattened, since stored
conversation configs already use a literal `other` key that flattening
would otherwise capture as a provider parameter.

Signed-off-by: Jean Mertz <git@jeanmertz.com>
Signed-off-by: Jean Mertz <git@jeanmertz.com>
Signed-off-by: Jean Mertz <git@jeanmertz.com>
Signed-off-by: Jean Mertz <git@jeanmertz.com>
Signed-off-by: Jean Mertz <git@jeanmertz.com>
Signed-off-by: Jean Mertz <git@jeanmertz.com>
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