Skip to content

Fixes #46#45

Merged
jph00 merged 10 commits intoAnswerDotAI:mainfrom
dangoldner:main
Dec 13, 2025
Merged

Fixes #46#45
jph00 merged 10 commits intoAnswerDotAI:mainfrom
dangoldner:main

Conversation

@dangoldner
Copy link
Copy Markdown
Contributor

@dangoldner dangoldner commented Dec 2, 2025

datetime.date arguments can't be inferred by inspection so must be handled as custom types.

@jph00
Copy link
Copy Markdown
Collaborator

jph00 commented Dec 5, 2025

I don't quite follow this PR, sorry! It seems that dateArg is defined twice, but never used, IIUC?

@dangoldner dangoldner marked this pull request as draft December 5, 2025 02:25
@dangoldner
Copy link
Copy Markdown
Contributor Author

dangoldner commented Dec 5, 2025

  • dateArg is now defined once (as you might guess, I had both the nbdev and solveit export flags set)
  • Like PathArg, dateArg is not called by the code directly anywhere, but it shows up in the schema to convert the json (string) argument to the correct custom type. The pre-existing notebook section on Path explains this for experts, or see this dialogue for an explanation for newcomers.
  • It was cool to learn what I needed to add this, but should it be added? Tools with date-type arguments could easily be rewritten to take string arguments and convert them inside the tool code. You may or may not want a burgeoning set of custom types (this PR would double the number, to two). Of course the same is true for Path, and Path is in here. No hard feelings either way :)
    See below

@dangoldner dangoldner marked this pull request as ready for review December 5, 2025 05:39
@dangoldner dangoldner marked this pull request as draft December 5, 2025 14:02
@dangoldner
Copy link
Copy Markdown
Contributor Author

There's something I still don't understand so pulling this back to draft for now ...

@dangoldner dangoldner changed the title Add datetime.date argument support to get_schema Convert custom types from string before passing to call_func Dec 5, 2025
@dangoldner
Copy link
Copy Markdown
Contributor Author

dangoldner commented Dec 5, 2025

This PR started out to allow date-type parameters in tool functions. But I learned that the pre-existing support for Path-type parameters was incomplete:

  • get_schema was correctly including information needed to convert paths from json strings to Path objects, but
  • custom param types (e.g. PathArg) were not being added to the custom_types watch list, and
  • no conversion was happening prior to the actual tool call.

So now, this PR simply fixes the situation for Path types.

I will wait to submit a separate PR for dates until Paths are merged and working correctly.

@dangoldner
Copy link
Copy Markdown
Contributor Author

Ok, git PRs and nbdev both new to me so apologies for the scattered commits, but I think we're now good to go.

@dangoldner dangoldner marked this pull request as ready for review December 5, 2025 20:23
@dangoldner dangoldner changed the title Convert custom types from string before passing to call_func Fixes #46 Dec 6, 2025
@jph00 jph00 merged commit be3da52 into AnswerDotAI:main Dec 13, 2025
@jph00
Copy link
Copy Markdown
Collaborator

jph00 commented Dec 13, 2025

Thanks!

@jph00 jph00 mentioned this pull request Dec 13, 2025
@jph00
Copy link
Copy Markdown
Collaborator

jph00 commented Dec 13, 2025

Sorry I actually reverted it, since on reflection fixing this properly required a bit more machinery, and a proper e2e test. Path should work fine now I believe.

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.

2 participants