Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tool auto generated Invoke reflection names are too long #323

Closed
StephenHodgson opened this issue Jun 10, 2024 · 5 comments · Fixed by #375 or RageAgainstThePixel/com.openai.unity#303
Assignees
Labels
enhancement New feature or request

Comments

@StephenHodgson
Copy link
Member

          @StephenHodgson I've found that Assistant v2 doesn't always understand well when type.FullName is included, because the namespaces I use are long and not relevant/confusing to the AI context.

I found that removing this and only including the method.Name solved it.

I realize it isn't the ideal scenario in case two types contain the same method name, but it would still be helpful with an omit namespace option if possible. For now I've been removing it manually here in each update :)

Originally posted by @Bodekaer in RageAgainstThePixel/com.openai.unity#235 (comment)

Proposed fix:

Converting long namespace names to hash file string guid.

@StephenHodgson StephenHodgson self-assigned this Jun 10, 2024
@StephenHodgson StephenHodgson added the enhancement New feature or request label Jun 10, 2024
@StephenHodgson StephenHodgson added help wanted Extra attention is needed good first issue Good for newcomers labels Aug 17, 2024
@sagos95
Copy link

sagos95 commented Nov 10, 2024

I faced the same issue.

I found that removing this and only including the method.Name solved it.

But how did you do that? I didn't see any accessible ways to specify function name by myself.
By the way that could be a fix too - to allow users manually specify function names. As for me it will be not the best solution to use hashes or guids.

@StephenHodgson
Copy link
Member Author

StephenHodgson commented Nov 10, 2024

Using hashes or guids would make it a bit more difficult to debug.

Part of the reason I haven't implemented it yet

@StephenHodgson
Copy link
Member Author

By the way that could be a fix too - to allow users manually specify function names.

Needs to be unique, and ideally not something that is super configurable, also the cache uses the names as keys on the backend.

@sagos95
Copy link

sagos95 commented Nov 11, 2024

also the cache uses the names as keys on the backend

Maybe it makes sense to separate Tools for different chats. For example, we could create a kind of Tool pool or Tool container. Making this class static provides a simpler way to interact with it, but this approach limits the flexibility of using Tools in scaled production apps.

@StephenHodgson
Copy link
Member Author

StephenHodgson commented Nov 11, 2024

You don't have to use the built-in tool serialization convince methods. You can always serialize and pass your own. That is off topic from the issue and not productive to solving the original problem defined in the title. Open a discussion about it if you'd like to talk about proposals like this.

The reason it is done this way is to provide convenient way to invoke those same tools if they happen to have locally defined methods you can execute in known libraries. Without mapping these things correctly it'd never work.

@RageAgainstThePixel RageAgainstThePixel locked as off-topic and limited conversation to collaborators Nov 11, 2024
@StephenHodgson StephenHodgson removed help wanted Extra attention is needed good first issue Good for newcomers labels Nov 14, 2024
StephenHodgson added a commit that referenced this issue Nov 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
2 participants