Skip to content

fix(utils): preserve header-named parameter descriptions - #771

Open
fallenmi wants to merge 1 commit into
ollama:mainfrom
fallenmi:codex/tool-docstring-section-names-oct19
Open

fallenmi wants to merge 1 commit into
ollama:mainfrom
fallenmi:codex/tool-docstring-section-names-oct19

Conversation

@fallenmi

Copy link
Copy Markdown

Summary

Keep documented tool parameters named raises, returns, or yields from being mistaken for docstring section headers.

On current main, an entry such as raises: Whether to raise on HTTP errors inside Args: ends the section early. Both that parameter and later parameters lose their descriptions in the generated tool schema, including the actual Client.chat(tools=[func]) / async request payload.

The parser now uses the function's exact parameter names, the Args: indentation, and nonempty inline description text to recognize parameter entries. Real section headers, including previously accepted over-indented bare headers, still end the section. The public callable-conversion API is unchanged; the private parser's new argument is optional and adds no dependency.

Verification

  • Original main + the first new regression test: 122 passed / 1 failed, with the raises description empty.
  • Final patch: 124 repository tests/doctests + 25 independent cases passed on HTTPX 0.27.0 and 0.28.1, Python 3.11.11 / Pydantic 2.10.6.
  • Independent cases cover header-named parameters across indentation/tabs, poisoned real section boundaries, existing doc formats, callable non-mutation, three legacy over-indented headers, and real sync/async chat serialization through HTTPX MockTransport.
  • The first proposed fix was rejected: it passed the original suite but introduced three independently reproduced legacy-header regressions. The refined patch passes those cases too.
  • Ruff 0.17.1 all-project lint and formatting checks (46 files), git diff --check, and Python 3.8 AST syntax check passed.

Tests ran with a clean environment, private-home/external-network denial and process resource limits. No Ollama server, model, or cloud calls were used in tests. The full Hatch multi-Python/OS matrix and Python 3.8 runtime were not run; lockfiles and dependency declarations are unchanged.

Scope and adjacent work

Bare empty-description entries and flat header-named entries retain their existing ambiguous behavior. A deeper lowercase inline header whose exact name is also a real parameter is interpreted as parameter documentation; this is the intentional ambiguity resolution, not a claim of identical behavior for every possible docstring.

This does not implement the separate raw-section/dictionary-key collision in #765/#766 or colon-containing continuation support in #747. Those PRs touch the same parser, so whichever lands later may need a rebase. The default-parameter/schema work in other open PRs is also unchanged.

AI assistance: Claude prepared and refined the proposal. Codex independently reproduced the base failure, found the first proposal's compatibility regression, integrated the refined change, and ran the checks above.

This branch has not been deployed

No deployments
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