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

fix: Remove tiktoken references #98

Merged
merged 8 commits into from
Feb 21, 2025
Merged

fix: Remove tiktoken references #98

merged 8 commits into from
Feb 21, 2025

Conversation

nboyse
Copy link
Collaborator

@nboyse nboyse commented Feb 18, 2025

Context

Tiktoken is making direct calls to openAI which is causing a RateLimitError as we don't use openai anymore we shouldn't even be referencing it (security wise not great either)

Its only for metadata so doesn't impact anything properly but turning it off will reduce the amount of herrings and ensure our independence.

Changes proposed in this pull request

Replace tiktoken references with a custom tokeniser simply used to count token usage across codebase and minorly used for wikipedia/govuk search functions

Guidance to review

Please run it locally, check it hasn't broken any of the routes for you, also check you are happy with the approach or if you have any suggestions on what you'd rather use.

Relevant links

Things to check

  • I have added any new ENV vars in all deployed environments
  • I have tested any code added or changed
  • I have run integration tests

@nboyse nboyse marked this pull request as ready for review February 18, 2025 14:55
@nboyse nboyse changed the title WIP: fix - openai usage fix: Remove tiktoken references Feb 18, 2025
@larry6point6
Copy link
Contributor

As discussed think the implementation is all good but not sure if right, ultimately a question of how important are tokens being accurate per model basis and the impact either way, think it's for the team to decide/guide

Copy link
Contributor

@larry6point6 larry6point6 left a comment

Choose a reason for hiding this comment

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

Further discussion with the team, think we're good to merge

@nboyse nboyse merged commit d095d59 into dev Feb 21, 2025
8 checks passed
@nboyse nboyse deleted the bugfix/openai-usage branch February 21, 2025 12:29
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