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

chore: serialize tool calls messages with non-empty content #622

Merged
merged 4 commits into from
Nov 28, 2024

Conversation

mmikita95
Copy link
Contributor

Enable serialization for messages that contain tool_calls but have non-empty content – in cases when model generates both content and tool_calls without separation.

src/writer/ai.py Outdated Show resolved Hide resolved
@mmikita95
Copy link
Contributor Author

mmikita95 commented Nov 18, 2024

I've decided to go for a simpler approach – as we need to display all messages with proper content, it should be the main "reason" behind the decision to include it into serialization. And, as long as there is no proper content, we shouldn't really care about other properties of the message, it doesn't get displayed either way.

UPD: as we can see in _serialize_message below, on line 1833, messages get "stripped" of all properties other than content, role and actions – so even if we allow serializing messages with tool_calls or tool_call_id, those properties are still not propagated to the frontend.

src/writer/ai.py Outdated Show resolved Hide resolved
@ramedina86 ramedina86 merged commit c789209 into writer:dev Nov 28, 2024
15 checks passed
ramedina86 added a commit that referenced this pull request Nov 29, 2024
commit 0f8a653
Merge: c6bd286 5ea2448
Author: Ramiro Medina <[email protected]>
Date:   Thu Nov 28 17:54:43 2024 +0100

    Merge pull request #638 from madeindjs/WF-45

    feat(ui): use `BuilderSelect` for handlers + implement Design System - WF-45

commit c6bd286
Merge: c789209 4998ace
Author: Ramiro Medina <[email protected]>
Date:   Thu Nov 28 10:03:36 2024 +0100

    Merge pull request #654 from mmikita95/fix-pytest-warnings

    fix: register `set_token` mark to avoid pytest warnings

commit c789209
Merge: 8774f9b 93a4f6e
Author: Ramiro Medina <[email protected]>
Date:   Thu Nov 28 10:01:53 2024 +0100

    Merge pull request #622 from mmikita95/chore-serialize-non-empty-content

    chore: serialize tool calls messages with non-empty content

commit 8774f9b
Merge: ec53f21 a8f42de
Author: Ramiro Medina <[email protected]>
Date:   Wed Nov 27 09:30:02 2024 +0100

    Merge pull request #646 from writer/WF-123-fix-examples-versions

    fix(WF-123): template apps have incorrect version after generation

commit 4998ace
Author: mmikita95 <[email protected]>
Date:   Wed Nov 27 10:20:04 2024 +0300

    fix: register `set_token` mark to avoid pytest warnings

commit 5ea2448
Author: Alexandre Rousseau <[email protected]>
Date:   Mon Nov 18 21:33:27 2024 +0100

    feat(ui): use `BuilderSelect` for handlers + implement Design System - WF-45

commit a8f42de
Author: Mateusz Russak <[email protected]>
Date:   Fri Nov 22 23:24:36 2024 +0100

    fix: tests

commit efcb814
Author: Mateusz Russak <[email protected]>
Date:   Fri Nov 22 23:10:25 2024 +0100

    fix: linter error

commit a93b102
Author: Mateusz Russak <[email protected]>
Date:   Fri Nov 22 22:16:46 2024 +0100

    fix(WF-123): Fix writer versions in examples

commit 93a4f6e
Author: Mikita Makiej <[email protected]>
Date:   Mon Nov 18 12:47:27 2024 +0300

    fix: also include empty non-`None` content as failing condition

commit bb3683a
Author: Mikita Makiej <[email protected]>
Date:   Mon Nov 18 11:46:52 2024 +0300

    fix: unfinished docstring

commit 2f96977
Author: mmikita95 <[email protected]>
Date:   Mon Nov 18 10:51:13 2024 +0300

    chore: update `_is_serialized`

commit d646732
Author: mmikita95 <[email protected]>
Date:   Fri Nov 8 17:34:02 2024 +0400

    chore: serialize tool calls messages with non-empty content
@mmikita95 mmikita95 deleted the chore-serialize-non-empty-content branch December 2, 2024 08:39
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.

4 participants