Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,34 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

## [0.6.2] - 2026-06-21

### Fixed — Token Usage Tracking

- **`tokens_used` is no longer always `None` in review and scan responses.**
- `parse_review_response` and `parse_scan_response` previously discarded the `usage` object returned by the LLM API, hardcoding `Ok((..., None))`. Token counts and cost estimates were silently dropped.
- `chat_completion` now returns `(content, Option<Usage>)` and the parse functions thread `usage` through as `TokenUsage`. All call sites updated.
- `ReviewResponse.tokens_used` and `ScanResponse.tokens_used` now report real values when the provider supplies them.

- **`cora review --stream` now collects token usage.**
- The streaming path (`chat_completion_stream`) previously only accumulated `delta.content` and ignored the `usage` field. It now sends `stream_options: { include_usage: true }` and parses `usage` from the final SSE chunk (top-level or nested in `choices[0].delta.usage`).
- Token counts are now reported correctly for both streaming and non-streaming review.

- **`cora scan` multi-batch token accumulation.**
- When scanning multiple batches, `total_tokens` was overwritten by each batch instead of accumulated. Only the last successful batch's tokens were reported.
- Token usage now accumulates across all batches (`input_tokens`, `output_tokens`, and `estimated_cost_usd` are summed).

### Changed — Code Quality

- **Extracted magic numbers into named constants.**
- `scan.rs`: the hardcoded batch size fallback `20` and token budget `60_000` are now `DEFAULT_MAX_FILES_PER_BATCH` and `DEFAULT_BATCH_TOKEN_BUDGET`.
- **`Usage` struct now accepts camelCase aliases.**
- Some providers (e.g. Azure OpenAI, certain third-party gateways) return `promptTokens` / `completionTokens` / `totalTokens` instead of snake_case. Both forms are now accepted via `#[serde(alias = ...)]`.

### Tests

- Added 4 regression tests for token usage threading: `parse_review_preserves_usage_when_provided`, `parse_review_returns_none_usage_when_not_provided`, `parse_scan_preserves_usage_when_provided`, `usage_to_token_usage_maps_fields_correctly`.

## [0.6.1] - 2026-06-17

### Fixed — Scan
Expand Down
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "cora-cli"
version = "0.6.1"
version = "0.6.2"
edition = "2024"
description = "CLI-first AI code review — BYOK, diff/scan/branch, pre-commit hooks"
license = "MIT"
Expand Down
28 changes: 28 additions & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,34 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

## [0.6.2] - 2026-06-21

### Fixed — Token Usage Tracking

- **`tokens_used` is no longer always `None` in review and scan responses.**
- `parse_review_response` and `parse_scan_response` previously discarded the `usage` object returned by the LLM API, hardcoding `Ok((..., None))`. Token counts and cost estimates were silently dropped.
- `chat_completion` now returns `(content, Option<Usage>)` and the parse functions thread `usage` through as `TokenUsage`. All call sites updated.
- `ReviewResponse.tokens_used` and `ScanResponse.tokens_used` now report real values when the provider supplies them.

- **`cora review --stream` now collects token usage.**
- The streaming path (`chat_completion_stream`) previously only accumulated `delta.content` and ignored the `usage` field. It now sends `stream_options: { include_usage: true }` and parses `usage` from the final SSE chunk (top-level or nested in `choices[0].delta.usage`).
- Token counts are now reported correctly for both streaming and non-streaming review.

- **`cora scan` multi-batch token accumulation.**
- When scanning multiple batches, `total_tokens` was overwritten by each batch instead of accumulated. Only the last successful batch's tokens were reported.
- Token usage now accumulates across all batches (`input_tokens`, `output_tokens`, and `estimated_cost_usd` are summed).

### Changed — Code Quality

- **Extracted magic numbers into named constants.**
- `scan.rs`: the hardcoded batch size fallback `20` and token budget `60_000` are now `DEFAULT_MAX_FILES_PER_BATCH` and `DEFAULT_BATCH_TOKEN_BUDGET`.
- **`Usage` struct now accepts camelCase aliases.**
- Some providers (e.g. Azure OpenAI, certain third-party gateways) return `promptTokens` / `completionTokens` / `totalTokens` instead of snake_case. Both forms are now accepted via `#[serde(alias = ...)]`.

### Tests

- Added 4 regression tests for token usage threading.

## [0.6.1] - 2026-06-17

### Fixed — Scan
Expand Down
81 changes: 81 additions & 0 deletions docs/fix-plan-v0.6.2.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
# Fix Plan — v0.6.2 Patch Release

Branch: `fix/token-usage-tracking-and-hardcode-cleanup`
Base: `develop` @ f1027ba

## Executive Summary

Investigasi menemukan **3 bug fungsional** (token usage selalu hilang) dan
**2 masalah hardcode/optimasi**. Semua lolos dari compiler & 586 tests karena
tidak ada test yang meng-assert `tokens_used`.

---

## Bug Fixes

### BUG-1: `tokens_used` selalu `None` di `parse_review_response` / `parse_scan_response`

**Root cause:** Kedua fungsi parse hardcoded `Ok((..., None))` sebagai return
value untuk `tokens_used`, meskipun `chat_completion()` sudah menerima dan
membuang objek `Usage` dari API response.

**Impact:**
- `--progress` events selalu report `tokens: {input: 0, output: 0}`
- `ScanResponse.tokens_used` selalu `None` di output JSON/SARIF
- Cost estimation (`estimated_cost_usd`) tidak pernah terisi
- Debt tracker tidak bisa track token cost dari review ke review

**Fix:**
1. `chat_completion()` return tuple `(String, Option<Usage>)` bukan `String`
2. `parse_review_response` / `parse_scan_response` terima parameter `Option<&Usage>`
3. Konversi `Usage` → `TokenUsage` via helper `usage_to_token_usage()`
4. Update semua call sites

### BUG-2: Streaming response tidak collect `usage`

**Root cause:** `chat_completion_stream()` & `review_diff_stream()` hanya
mengumpulkan `delta.content`, mengabaikan field `usage` yang dikirim provider
(via `stream_options: {include_usage: true}` atau di chunk terakhir).

**Fix:**
1. Tambah `stream_options: {include_usage: true}` ke request body
2. Parse `usage` dari SSE chunk (banyak provider kirim di chunk terakhir)
3. Return `(String, Option<TokenUsage>)` dari streaming path

### BUG-3: Scan multi-batch `total_tokens` di-overwrite, bukan diakumulasi

**Root cause:** `scan.rs:148` — `total_tokens = tokens` menimpa nilai batch
sebelumnya. Hanya batch terakhir yang sukses yang dilaporkan.

**Fix:** Akumulasi via helper `accumulate_token_usage()`.

---

## Hardcode & Optimasi

### HARD-1: Magic number `20` dan `60_000` di `scan.rs`

`max_files_per_batch` fallback `20` dan token-budget `60_000` di-hardcode.
Ekstrak ke konstanta bermakna.

### HARD-2: `Usage.prompt_tokens` / `completion_tokens` diduplikasi field

`Usage` struct punya `#[allow(dead_code)]` untuk `prompt_tokens` dan
`completion_tokens` — sekarang akan dipakai untuk konversi ke `TokenUsage`,
jadi `allow` bisa dihapus.

---

## Testing

- Tambah test untuk BUG-1: assert `tokens_used.is_some()` ketika API kasih usage
- Tambah test untuk BUG-3: assert akumulasi 2 batch → total = jumlah
- Tambah test untuk `usage_to_token_usage()` konversi
- Pastikan semua 586 test existing masih pass

---

## Version Bump & Changelog

- `Cargo.toml`: `0.6.1` → `0.6.2`
- `CHANGELOG.md`: tambah entry `[0.6.2]` dengan Fixed section
28 changes: 22 additions & 6 deletions src/commands/scan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,17 @@ use tracing::debug;

use crate::config::schema::Config;
use crate::engine::scanner::{batch_files, format_batch_for_prompt, walk_project};
use crate::engine::types::TokenUsage;
use crate::formatters::{OutputFormat, formatter_for};

/// Default maximum files per LLM batch when `--batch-files` is not specified.
/// Lower this to work around provider token limits or rate-limit errors.
const DEFAULT_MAX_FILES_PER_BATCH: usize = 20;

/// Approximate token budget per batch. Used by the scanner to split large file
/// sets into review-sized chunks that fit within typical model context windows.
const DEFAULT_BATCH_TOKEN_BUDGET: usize = 60_000;

/// Scan command options.
pub struct ScanOptions {
/// Root directory to scan.
Expand Down Expand Up @@ -108,9 +117,9 @@ pub async fn execute_scan(
let max_files_per_batch = if opts.batch_files > 0 {
opts.batch_files
} else {
20
DEFAULT_MAX_FILES_PER_BATCH
};
let batches = batch_files(&files, 60_000, max_files_per_batch);
let batches = batch_files(&files, DEFAULT_BATCH_TOKEN_BUDGET, max_files_per_batch);
debug!(
batches = batches.len(),
max_files = max_files_per_batch,
Expand All @@ -119,7 +128,7 @@ pub async fn execute_scan(

// 4. Process batches and collect issues
let mut all_issues = Vec::new();
let mut total_tokens = None;
let mut total_tokens: Option<TokenUsage> = None;
let mut skipped_batches: Vec<(usize, Vec<String>, String)> = Vec::new();

for (batch_idx, batch) in batches.iter().enumerate() {
Expand All @@ -144,9 +153,16 @@ pub async fn execute_scan(
{
Ok((issues, _summary, tokens)) => {
all_issues.extend(issues);
if tokens.is_some() {
total_tokens = tokens;
}
total_tokens = match (total_tokens, tokens) {
(Some(mut acc), Some(t)) => {
acc.input_tokens += t.input_tokens;
acc.output_tokens += t.output_tokens;
acc.estimated_cost_usd += t.estimated_cost_usd;
Some(acc)
}
(None, Some(t)) => Some(t),
(acc, None) => acc,
};
}
Err(err) => {
let file_list: Vec<String> =
Expand Down
Loading
Loading