Skip to content

Comments

fix: Use placeholder message to check tokens#41

Merged
baxen merged 1 commit intomainfrom
baxen/moderator-models
Sep 9, 2024
Merged

fix: Use placeholder message to check tokens#41
baxen merged 1 commit intomainfrom
baxen/moderator-models

Conversation

@baxen
Copy link
Collaborator

@baxen baxen commented Sep 9, 2024

This makes the moderators compatible with providers which don't support empty message lists

_system_token_exchange.generate()
last_system_prompt_token_count = self.system_prompt_token_count
self.system_prompt_token_count = _system_token_exchange.checkpoint_data.total_token_count
self.system_prompt_token_count = _system_token_exchange.checkpoint_data.total_token_count - 1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a heads up, this isn't quite right (and depends on model) since the addition of a user message adds some extra tokens to indicate the start/end of the message for the role (user/assistant). But this estimate is within ~5 tokens which is fine and is the best we can do without building out a token counting module for this

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes that does make sense - i think that is alright though, because we're sort of redefining what "system_prompt_token_count" means by a bit - it is now both the system and the unavoidable overhead of sending the next message, which is still a reasonable thing to count towards the limit?

Message(role="user", content=[ToolResult(tool_use_id="7", output="2.418 x 10^14 Hz")]),
Message(role="assistant", content=[ToolUse(id="8", name="electron_mass", parameters={})]),
Message(role="user", content=[ToolResult(tool_use_id="8", output="9.10938356 x 10^-31 kg")]),
Message(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

who wrote these tests? I appreciate the constants :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol glad someone found them fun!

@zakiali
Copy link
Collaborator

zakiali commented Sep 9, 2024

Updated some tests to do a real check on the addition of "a"

Message(role="user", content=[ToolResult(tool_use_id="7", output="2.418 x 10^14 Hz")]),
Message(role="assistant", content=[ToolUse(id="8", name="electron_mass", parameters={})]),
Message(role="user", content=[ToolResult(tool_use_id="8", output="9.10938356 x 10^-31 kg")]),
Message(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol glad someone found them fun!

if not self.system_prompt_token_count or is_different_system_prompt:
# calculate the system prompt tokens (includes functions etc...)
# we use a placeholder message with one token, which we subtract later
# this ensures compatible with providers that require a user message
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: s/compatible/compatibility

This makes the moderators compatible with providers which
don't support empty message lists
@baxen baxen force-pushed the baxen/moderator-models branch from d48a1fc to 61dca81 Compare September 9, 2024 20:32
@baxen baxen merged commit 4678c9d into main Sep 9, 2024
lifeizhou-ap added a commit that referenced this pull request Sep 18, 2024
* main:
  feat: Rework error handling (#48)
  chore(release): release version 0.9.0 (#45)
  chore: add just command for releases and update pyproject for changelog (#43)
  feat: convert ollama provider to an openai configuration (#34)
  fix: Bedrock Provider request (#29)
  test: Update truncate and summarize tests to check for sytem prompt t… (#42)
  chore: update test_tools to read a file instead of get a password (#38)
  fix: Use placeholder message to check tokens (#41)
  feat: rewind to user message (#30)
  chore: Update LICENSE (#40)
  fix: shouldn't hardcode truncate to gpt4o mini (#35)
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.

3 participants