Proper token count based on model #2

oleander · 2024-05-17T22:13:40Z

No description provided.

src/hook.rs

…tency

Change the trigger types for the continuous delivery pipeline in the GitHub Actions workflow file (cd.yml). Restrict triggers to the 'closed' event type rather than 'opened', 'synchronize', 'reopened', and 'closed'. Also, add 'workflow_dispatch' to allow manual running of the workflow.

…e it accordingly in commit.rs

The main function now has a better error handling mechanism using the standard process exit call. In the event of an error, the program prints the error and exits with a status code of 1. Also, debug log messages have been added to give additional insight into the execution duration of the program whether it ends successfully or encounters an error.

This commit adds a debug log to the Args implementation, making it easier to monitor if a commit message has already been provided. In addition, it also handles invalid sources, ensuring program stability by returning Ok(()) in such cases.

Copilot

Pull Request Overview

This PR introduces proper token counting based on specific AI models by adding a new Model enum and related infrastructure. The changes replace the generic string-based model configuration with a type-safe model system that includes model-specific token counting capabilities using the tiktoken-rs library.

Key changes:

Added model-aware token counting and text truncation functionality
Refactored OpenAI API integration to use structured request/response types
Updated configuration system to use optional fields with sensible defaults

Reviewed Changes

Copilot reviewed 13 out of 15 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/model.rs	New module defining Model enum with token counting and truncation methods
src/openai.rs	New OpenAI API client with structured Request/Response types
src/hook.rs	Updated diff processing to use model-specific token counting and improved truncation logic
src/commit.rs	Simplified commit generation by delegating to new openai module
src/config.rs	Changed configuration fields to Optional with default value handling
src/lib.rs	Added new model and openai modules to library exports
Cargo.toml	Added tiktoken-rs dependency for token counting
tools/demo.sh	Removed demo script file
scripts/integration-tests	Added comprehensive integration test script
Dockerfile	Simplified Docker configuration for testing
Justfile	Added integration test recipe
.github/workflows/ci.yml	Removed CI workflow file
.github/workflows/cd.yml	Simplified conditional expressions in CD workflow

Copilot · 2025-10-06T06:07:41Z

src/openai.rs

+  let choise = chat
+    .choices
+    .first()
+    .context(format!("Failed to get response: {:?}", chat))?;
+
+  let response = choise


Corrected spelling of 'choise' to 'choice'.

Suggested change

let choise = chat

.choices

.first()

.context(format!("Failed to get response: {:?}", chat))?;

let response = choise

let choice = chat

.choices

.first()

.context(format!("Failed to get response: {:?}", chat))?;

let response = choice

Copilot · 2025-10-06T06:07:41Z

src/hook.rs

-        }).context("Failed to print diff")?;
-
-    Ok(patch_acc.to_utf8())
+  // TODO: Grouo arguments


Corrected spelling of 'Grouo' to 'Group'.

Suggested change

// TODO: Grouo arguments

// TODO: Group arguments

Copilot · 2025-10-06T06:07:42Z

src/hook.rs

+    let mut diffs: Vec<_> = files.values().collect();
+
+    // TODO: No unwrap
+    diffs.sort_by_key(|diff| model.count_tokens(diff).unwrap());


Using unwrap() can cause panic if token counting fails. Consider using unwrap_or_default() or proper error handling.

Suggested change

diffs.sort_by_key(|diff| model.count_tokens(diff).unwrap());

diffs.sort_by_key(|diff| model.count_tokens(diff).unwrap_or_default());

Copilot · 2025-10-06T06:07:42Z

src/hook.rs

+          let truncated_diff = if file_token_count > *file_allocated_tokens {
+            model.truncate(diff, *file_allocated_tokens)
+          } else {
+            Ok((*diff).clone().to_owned()) // TODO: Better way?


The double cloning ((*diff).clone().to_owned()) is redundant. Use either diff.to_string() or (*diff).to_owned().

Suggested change

Ok((*diff).clone().to_owned()) // TODO: Better way?

Ok((*diff).to_owned()) // Fixed: removed redundant clone()

Copilot · 2025-10-06T06:07:42Z

src/commit.rs

-    .build()?;
+pub async fn generate(diff: String, max_tokens: usize, model: Model) -> Result<openai::Response> {
+  if max_tokens == 0 {
+    bail!("Max can't be zero (2)")


The error message 'Max can't be zero (2)' is unclear. The '(2)' suffix appears to be a debugging artifact and should be removed or explained.

Suggested change

bail!("Max can't be zero (2)")

bail!("Max can't be zero")

oleander added 4 commits May 17, 2024 23:33

Add .rustfmt.toml for formatting and update dependencies

103161b

Update gitignore to exclude log files

31606a4

Add functions for HTTP request handling

d900ee9

Fix potential buffer overflow in input parsing

d5d69c3

oleander commented May 17, 2024

View reviewed changes

src/hook.rs Outdated Show resolved Hide resolved

oleander marked this pull request as ready for review May 17, 2024 22:31

oleander added 24 commits May 24, 2024 13:21

Merge branch 'main' into feature/tokens

c507120

Update dependencies and token handling logic

5d561c6

Add model parameters to commit and diff generation functions

6ffaca5

Refactor token counting error context messages

52f84f1

Add GPT4Turbo variant to Model enum and related implementations

6f89035

Switch to stable Rust, refactor model structure for serialization

e5d9e06

Refactor model import and usage across modules

597d40a

Refactor Model enum to models.rs for better organization

13858d4

Refactor model type in config.rs to use String instead of Model

0789453

Refactor Model conversion from String implementation

5cad11e

Refactor model string parsing logic

920d5d9

Fix Display impl formatting in Model

85f91f3

Refactor Model parsing to use parse_model function

df18b8c

Refactor Model parsing to simplify error handling

57850d2

Refactor Default implementation for Model

4193822

Remove comment from 'test' configuration in Cargo.toml

1e2b588

Switch Rust toolchain channel to nightly

5249d0b

Remove unused import from config.rs

8ffd524

Remove commented-out default model setting in config.rs

886fac8

Delete deprecated examples.rs file

0023522

Remove examples command from CLI interface

b0a9d8d

Refactor: merge models.rs into model.rs for clarity and import consis…

3ab6bd6

…tency

Smarter token calc

15d9ddf

Refactor code to remove redundant cloning in config.rs

1315eea

oleander added 14 commits May 28, 2024 18:58

Merge branch 'main' into feature/tokens

7c952e4

Remove unnecessary braces from conditional statements in workflow file

a365aa6

Fix redundant struct fields and cleanup main function in hook.rs

6df5c76

Remove unnecessary tree existence check in hook.rs

263bae5

Add new hook.rs file under src/bin directory

6b209af

Remove duplicate logging statement in Model implementation

09f8974

Remove template.txt file

e480301

Merge branch 'main' into feature/tokens

4be9a16

Update action event types in cd.yml workflow

0ee27bd

Update max_commit_length field to be optional in App struct and handl…

997f901

…e it accordingly in commit.rs

Remove duplicate case in match condition

13f5b01

oleander merged commit 81e0d59 into main May 28, 2024

oleander deleted the feature/tokens branch May 28, 2024 20:50

oleander mentioned this pull request Oct 5, 2025

[Refactor] Standardize type names across codebase #62

Open

8 tasks

Copilot AI mentioned this pull request Oct 5, 2025

[Refactor] Standardize type names: App → AppConfig, remove Settings alias #72

Merged

8 tasks

oleander requested a review from Copilot October 6, 2025 06:06

Copilot AI reviewed Oct 6, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Proper token count based on model #2

Proper token count based on model #2

Uh oh!

oleander commented May 17, 2024

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Oct 6, 2025

Uh oh!

Copilot AI Oct 6, 2025

Uh oh!

Copilot AI Oct 6, 2025

Uh oh!

Copilot AI Oct 6, 2025

Uh oh!

Copilot AI Oct 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	diffs.sort_by_key(\|diff\| model.count_tokens(diff).unwrap());
	diffs.sort_by_key(\|diff\| model.count_tokens(diff).unwrap_or_default());

	Ok((*diff).clone().to_owned()) // TODO: Better way?
	Ok((*diff).to_owned()) // Fixed: removed redundant clone()

Proper token count based on model #2

Proper token count based on model #2

Uh oh!

Conversation

oleander commented May 17, 2024

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants