Skip to content

feat(config): Let every map state how it merges - #1131

Open
JeanMertz wants to merge 16 commits into
mainfrom
config-map-strategies
Open

JeanMertz wants to merge 16 commits into
mainfrom
config-map-strategies

Conversation

@JeanMertz

Copy link
Copy Markdown
Collaborator

A map-valued setting can say how it combines, the same way a list
can. Ten fields gain it: providers.mcp, conversation.tools,
providers.llm.aliases, plugins.command, template.values, and a
tool's parameters, questions, options and properties.

Removing an entry from any of them reaches the conversation, which an
entry-wise merge could not express: the conversation kept the tool or
server the user had deleted, and recomputed the same empty delta every
turn.

A conversation's own snapshot is emitted key by key rather than as a
replacement, so a server, tool or alias the workspace config gained
after the conversation was created still reaches it.

Also fixes how a generic type is named in the schema.
MergeableMap<ToolConfig> and MergeableMap<ToolParameterConfig> both
answered MergeableMap, so a consumer resolving a reference by name
walked a value against whichever it met first. That reached users
through stored conversations: either valid keys were deleted, or a stale
one survived for typed deserialization to reject, which discards the
whole stored config rather than the key. The arguments are appended to
the name.

merge_nested_indexmap and the four strategy-less delta helpers are
deleted; nothing needs to guess any more.

@JeanMertz
JeanMertz force-pushed the config-map-strategies branch from e4804a1 to a5e22fb Compare September 8, 2026 08:09
Base automatically changed from config-list-strategies to main September 18, 2026 19:43
`conversation.labels` computed its own delta inline. The rule it
encoded is not specific to labels: an entry both states share carries
its own delta, an entry only the next state has is carried whole, and a
key the previous state had and the next one does not was dropped, which
entries cannot spell -- so the whole map carries `replace` rather than
letting a deep merge resurrect the key.

`delta_mergeable_map` states that once, alongside `delta_mergeable_vec`.
It also drops an entry whose own delta comes out empty, which the inline
version did not: a missing entry already means "unchanged", so an empty
one reads as a change that is not there and makes the enclosing partial
look non-empty. That is the same noise the map delta for
`providers.mcp` was fixed for.

Behaviour is otherwise unchanged; the helper is what the remaining
`IndexMap` fields need as they gain a strategy of their own.

Signed-off-by: Jean Mertz <git@jeanmertz.com>
Removing an MCP server, a tool, a model alias, a plugin, a template
value, a tool parameter, question or option from a config file now
reaches the conversation. Until now the removal was computed, found to
be unexpressible, and dropped: the conversation kept starting the
server the user had deleted, and recomputed the same non-delta on every
turn.

Map entries merge by key, which is what lets a server added to the
workspace config reach a conversation created before it existed. The
same property means no value a delta carries can take an entry away:
the key survives from the previous layer. The entry's own path is
reported in the delta's `unsets` instead, and the fold removes it
before merging, which `unset` already supported.

Every map in the configuration is covered, including the ones nested
inside a tool, and clearing a field of the `conversation.tools.'*'`
defaults block is recorded too. That block resolves through its own
type, which had no path-reporting delta, so a cleared `enable` or
`style.error.inline_results` there went unrecorded.

A map whose values are plain rather than nested partials gets the same
treatment through `delta_value_map_with_unsets`, replacing three copies
of the same inline entry-comparison loop.

Signed-off-by: Jean Mertz <git@jeanmertz.com>
`conversation.compaction.rules` is the one list field whose delta still
compares elements rather than saying `replace`, and the reason recorded
next to it was wrong. It named the built-in defaults' `discard_when_merged`
marker, which is not what stops the field.

What stops it is that the field's partial is a bare `MergeableVec`, so an
empty one cannot say whether the user asked for no rules or said nothing
about them. Every sparse partial that reaches the delta carries the empty
one, and replacing with it would record zero rules the user never asked
for, which makes a later `jp conversation compact` do nothing.

Found by routing the field through the shared helper and reading what
broke: `replace` with an empty list written into 37 conversation
snapshots, and a config event appended where the suppression path should
have left none. Reaching the field needs its partial to become an
`Option<MergeableVec<_>>`, the shape every converted list field has,
where `None` is absent and `Some([])` is a deliberate empty.

Signed-off-by: Jean Mertz <git@jeanmertz.com>
`providers.mcp` accepts a strategy the way every other collection field
does, so a workspace can drop the servers an outer layer configured
rather than merging with them:

```toml
[providers.mcp]
value = { bookworm = { type = "stdio", command = "just" } }
strategy = "replace"
```

The default is unchanged: entries merge by key, so a server added to a
later layer joins the ones an earlier layer set rather than replacing
them. That is what lets a server added to the workspace config reach a
conversation created before it existed, and there is now a test holding
that property.

Recording a removed server no longer needs a reported path. The map
states `replace` and carries the servers the user is left with, which
the fold applies without help, so `unsets` is left to the fields that
genuinely cannot speak for themselves.

The conversation's snapshot of the map merges per key rather than
stating `replace`, through the new `map_to_partial_per_key`. Stating
`replace` there would drop every server the config files declare and
the conversation does not, which is the behaviour the test above
catches. Merging per key is safe because each server's own lists
already state their strategies, so re-merging the snapshot over the
layer it came from reproduces it rather than doubling its arguments.

Signed-off-by: Jean Mertz <git@jeanmertz.com>
`plugins.command`, `providers.llm.aliases`, and a tool's `parameters`,
`questions` and nested `properties` accept a strategy the way every
other collection field does:

```toml
[providers.llm.aliases]
value = { opus = "anthropic/claude-opus-4" }
strategy = "replace"
```

The default is unchanged: entries merge by key, so an alias, plugin or
parameter configured in a later layer joins the ones an earlier layer
set. That is what lets a workspace config gain an entry and have it
reach a conversation created before it existed.

Recording a removed entry no longer needs a reported path. Each map
states `replace` and carries the entries the user is left with, which
the fold applies on its own. `unsets` is left to the fields that
genuinely cannot speak for themselves, and `PartialToolParameterConfig`
no longer needs a path-reporting delta at all.

Each conversation snapshot merges per key rather than stating
`replace`, so an entry the files declare and the conversation does not
survives the layering.

Signed-off-by: Jean Mertz <git@jeanmertz.com>
`template.values` and a tool's `options` accept a strategy the way
every other collection field does:

```toml
[template.values]
value = { branch = "main" }
strategy = "replace"
```

The default is unchanged: entries merge by key, so a value set in a
later layer joins the ones an earlier layer set.

Both maps hold free-form JSON rather than nested config, so their
entries have no partial to diff and are compared and carried whole
through `delta_mergeable_value_map`. A removed entry travels as a
`replace` with the entries the user is left with, so neither map needs
a reported path any more. `delta_value_map` and its unset-reporting
sibling are gone with them.

Every map in the configuration except `conversation.tools` itself now
states its own strategy, and each conversation snapshot merges per key
so an entry the files declare and the conversation does not survives
the layering.

Signed-off-by: Jean Mertz <git@jeanmertz.com>
`conversation.tools` accepts a strategy, which completes the set: every
collection field in the configuration now states how it merges.

```toml
[conversation.tools]
strategy = "replace"

[conversation.tools.value.my_tool]
source = "builtin"
```

The default is unchanged: tools merge by key, so a tool configured in a
later layer joins the ones an earlier layer set. Tool entries are
flattened to sit directly under `conversation.tools`, and the wrapper
needs both `value` and `strategy` to be recognised, so a tool may still
be named `value` or `strategy` on its own. Two tests hold both halves of
that.

`delta_map` and `delta_map_with_unsets` are gone. Every map states its
own strategy, so a removed entry travels in the value as a `replace` and
no map needs a reported path. What remains of `unsets` is what only it
can express: a scalar that went away.

Signed-off-by: Jean Mertz <git@jeanmertz.com>
`merge_nested_indexmap` merged two maps per key with no way for a config
to ask for anything else. Every map field now carries a `MergeableMap`,
whose `map_with_strategy` does the same per-key merge by default and
honours a declared `deep_merge`, `merge`, `keep` or `replace`, so the
older function has no callers left.

Signed-off-by: Jean Mertz <git@jeanmertz.com>
A schema names each type it expands and refers back to that name
wherever the type appears again, so a name has to identify one type. A
generic named after its base alone does not: `MergeableMap<ToolConfig>`
and `MergeableMap<ToolParameterConfig>` both answered `MergeableMap`,
and a consumer resolving a reference by name walked a value against
whichever of them it met first.

That reached users through stored conversations. Stripping a stored
config walks it against the schema to drop keys a newer release wrote,
and resolving a tool's `parameters` to the map of tools instead walks
each parameter against `ToolConfig` — deleting valid keys, or leaving a
stale one behind for typed deserialization to reject, which discards the
whole stored config rather than the key.

The arguments are appended, so the two are `MergeableMap_ToolConfig` and
`MergeableMap_ToolParameterConfig`. An argument with no name of its own
contributes nothing, which leaves the base name for a type generic only
over primitives.

Signed-off-by: Jean Mertz <git@jeanmertz.com>
Stripping a stored config walks it against the schema and removes the
keys the schema has no field for, so a key an older release does not
know is dropped rather than left to fail typed deserialization, which
would discard the whole stored config.

A map that can state its own merge strategy is described as the plain
map beside the wrapper holding it under `value`. Both are objects on the
wire, so shape alone left the union ambiguous and the walk stopped:
every key inside a tool, server, alias or plugin went unvisited. The
variants are now told apart the way the wrapper's own deserializer does
it, by whether the value carries `value` and `strategy` together, so a
tool called `value` is still a tool.

A flattened map is resolved the same way, which is what lets the entries
of `conversation.tools` be walked at all.

Signed-off-by: Jean Mertz <git@jeanmertz.com>
A tool the workspace config gained after a conversation was created
still reaches that conversation, as it did before the map could state
its own merge strategy. Filling took the conversation's map whole, so a
tool added later was invisible to every conversation that predated it.

A map that states a strategy is left alone instead: its owner said how
it combines, and filling gaps into it would answer differently. Only the
styles of the tools it holds are filled, which is what carries a single
`[conversation.tools.'*'.style]` key to each of them.

Signed-off-by: Jean Mertz <git@jeanmertz.com>
@JeanMertz
JeanMertz force-pushed the config-map-strategies branch from a5e22fb to e835ab0 Compare September 18, 2026 20:00
Restores a test-only `now` whose `cfg` attribute a structural merge
detached, and carries the `MergeableMap` hand-off into the plugin
dispatch path main added since.

The schema probe stops inventing strings below a collection, so a tool's
`source` is left out of a document probing something else, and
`MergeableMap` commits to the wrapper once a table carries both `value`
and `strategy`, so a misspelled strategy is an error rather than an
entry by that name.

Signed-off-by: Jean Mertz <git@jeanmertz.com>
Removing one of a conversation's tools stored the map the user was left
with, stamped `replace`. Reading that conversation back emptied the map,
so the replacement took every tool rather than the one removed, and the
conversation loaded with no tools at all.

Schema-aware stripping walks a flattened map's leftover keys against the
map's entry type, since those keys are entry names. A map stating a
strategy puts `value` and `strategy` where entry names otherwise sit, so
`value` was walked as a tool and every key inside it was removed as
unknown.

Told apart the way the map's own deserializer does it: a table carrying
both keys is the wrapper, and its `value` holds the map whose entries
are walked. The metadata beside it is left alone.

Signed-off-by: Jean Mertz <git@jeanmertz.com>
A server, tool, alias or plugin removed from a conversation came back on
the next invocation. The conversation layer is filled from the config
files, and filling reads a key the conversation does not hold as one it
never mentioned rather than one it removed, so the workspace put it
back.

A dropped key now reports its dotted path alongside the `replace` the
delta already carried. Both readers need it: the value is what the
conversation's own fold applies, and the path is what stops the layer
above from restoring the key after filling.

An entry both maps hold is diffed with its own path too, so a field
cleared inside a surviving entry says where it lives. Clearing an MCP
server's `checksum`, or a parameter's `enum`, was computed and dropped;
the tool config's path-reporting deltas were unreachable through the map
entirely.

Signed-off-by: Jean Mertz <git@jeanmertz.com>
…merge

Signed-off-by: Jean Mertz <git@jeanmertz.com>
Writing `--cfg 'conversation.tools.cargo_check.options:={"value":{…},
"strategy":"replace"}'` created two options literally named `value` and
`strategy`, and the tool ran with neither the setting the user asked for
nor a replaced map. The command reported success.

A whole-map assignment distributes the object's keys across entries,
which is right for a map of values and wrong for the wrapper that states
how the map merges. The two are told apart the way the map's own
deserializer does it, so the form the field documentation shows means
the same thing on the command line as in a config file.

An entry named `value` needs the sibling `strategy` before it reads as
the wrapper, so a tool called `value` stays assignable, and naming a
single entry still leaves whatever wrapper the map already carries
alone.

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