Repository navigation
Conversation
datvo06
reviewed
Sep 30, 2026
| if not isinstance(schema, dict): | ||
| return False | ||
| if schema.get("type") == "object" and ( | ||
| schema.get("additionalProperties") is not False |
Contributor
There was a problem hiding this comment.
Can/should we add this schema.get("additionalProperties") not in (None, False)?
Because right now any object schema without additionalProperties: false will be counted as open.
Contributor
There was a problem hiding this comment.
If that's the intended design then it's fine.
datvo06
approved these changes
Sep 30, 2026
eb8680
force-pushed
the
eb-tool-serialization
branch
2 times, most recently
from
October 1, 2026 13:26
7f3afb7 to
2c8d1e8
Compare
eb8680
removed this pull request from stack #808
October 1, 2026 13:29
eb8680
changed the base branch from
master
to
worktree-issue-762-empty-content-blocks
October 1, 2026 13:29
eb8680
added this pull request to stack #777
October 1, 2026 13:29
eb8680
force-pushed
the
eb-tool-serialization
branch
from
October 1, 2026 13:37
2c8d1e8 to
436c29d
Compare
eb8680
removed this pull request from stack #777
October 1, 2026 13:40
eb8680
changed the base branch from
worktree-issue-762-empty-content-blocks
to
eb-mapping-encoding
October 1, 2026 13:40
eb8680
added this pull request to stack #812
October 1, 2026 13:41
Port from eb-mcp-client: give each parameter a positional field aliased to its name so any parameter name round-trips, keep defaults, map **kwargs to extra fields, and drop strict mode when the schema cannot satisfy it. Update the acp plan test and PlanStep docstring, whose optional fields are no longer required.
An object schema that omits additionalProperties can still be closed for strict generation, so only an explicit open schema disables strict mode. This keeps tools whose parameters are nested dataclasses or models strict. Dump tool results by alias, matching the aliased parameter encoding.
With mappings written as key-value pairs, a tool taking a dict has a strict schema, so it moves to the strict cases. Keyword arguments collected by **kwargs are encoded one keyword at a time.
A named tool is advertised through the encoding of its tool's type, so a Tool subclass changes how it is advertised by registering an encoding. Tools as values never reached the old source-code encoding: the harness encodes values by nested_type, which gives Operation, not Tool.
eb8680
removed this pull request from stack #812
October 3, 2026 17:48
eb8680
force-pushed
the
eb-tool-serialization
branch
from
October 3, 2026 17:48
436c29d to
19b74fc
Compare
eb8680
added this pull request to stack #814
October 3, 2026 17:48
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Blocked by #802 which includes some preliminary fixes.
This PR includes some fixes for edge cases of tool encoding which came up in adding MCP support to the harness: give each parameter a positional field aliased to its name so any parameter name round-trips, keep defaults, map **kwargs to extra fields, and drop strict mode when the schema cannot satisfy it. Update the acp plan test and PlanStep docstring, whose optional fields are no longer required.