From 68ffe4059c386316d34e13036fecfda093721ae0 Mon Sep 17 00:00:00 2001 From: Divyang Chauhan Date: Wed, 30 Sep 2026 16:51:01 +0000 Subject: [PATCH] Add token-based review chunks and usage reporting --- README.md | 55 +++++++++++++++++++- docs/eval-running.md | 15 ++++++ docs/review-quality-improvements.md | 10 ++-- go.mod | 2 + go.sum | 4 ++ internal/cli/cli.go | 7 ++- internal/config/config.go | 29 +++++++++-- internal/config/config_test.go | 32 ++++++++++++ internal/gitdiff/gitdiff.go | 70 ++++++++++++++++++++------ internal/gitdiff/token_chunks_test.go | 59 ++++++++++++++++++++++ internal/model/model.go | 23 +++++++++ internal/provider/api.go | 28 ++++++++++- internal/provider/provider.go | 7 +-- internal/provider/subscription_test.go | 12 ++++- internal/render/render.go | 41 +++++++++++++++ internal/render/render_test.go | 22 ++++++++ internal/review/review.go | 57 ++++++++++++++++++++- internal/tokens/tokens.go | 20 ++++++++ internal/tokens/tokens_test.go | 21 ++++++++ 19 files changed, 481 insertions(+), 33 deletions(-) create mode 100644 internal/gitdiff/token_chunks_test.go create mode 100644 internal/tokens/tokens.go create mode 100644 internal/tokens/tokens_test.go diff --git a/README.md b/README.md index c5f1f93..643b7c9 100644 --- a/README.md +++ b/README.md @@ -110,7 +110,7 @@ Native API and subscription reviews return command output, exit status, and erro follow-up checks. Each command can run for up to 120 seconds and returns up to 64 KiB of output, with truncation reported explicitly. File reads support paging. Each patch chunk allows up to 32 tool rounds or 128 tool calls, followed by a -final review that records unfinished checks. A 10-minute deadline covers the +final review that records unfinished checks. A 20-minute deadline covers the entire chunk; exceeding it fails the review. Other common scopes: @@ -145,6 +145,59 @@ Repository-specific rules can be committed in `.diffvouch.yml`. DiffVouch reads that policy from the trusted base commit so a change cannot suppress its own review. A repository cannot select billable API transport or enable publishing. +### Token limits and usage + +Diff chunks default to **50,000 tokens**, counted locally before any model call. +Configure the limit in the trusted `.diffvouch.yml`: + +```yaml +version: 1 +review: + chunk_tokens: 50000 +``` + +Or override it for a review: + +```bash +diffvouch review --provider codex --transport subscription --model gpt-5.6-sol --chunk-tokens 50000 +``` + +This replaces the previous 180,000-byte default, not an exact conversion of it. +Existing configurations with `chunk_bytes` keep their byte-based behavior. +Choose one configuration field; `--chunk-tokens` overrides either. The separate +`max_diff_bytes` collection safety limit remains in bytes. + +The tokenizer runs offline with bundled vocabularies. Recognized models use the +encoding mapped by the tokenizer library. Unknown models, including newer model +names and Claude, use `o200k_base` with an explicit estimate label. Counts are +exact for that local encoding, not a guarantee of the selected model's accounting. + +Terminal output and the JSON `tokens` field report each chunk's diff and initial +prompt text counts. Native OpenAI API and ChatGPT subscription reviews also +record input, cached input, output, and reasoning tokens for each model request, +when the provider returns usage. Progress goes to stderr, preserving JSON stdout. +External CLI and Anthropic usage is currently unavailable. Missing usage is +reported as unavailable, not zero. + +For context planning, use the **largest input count for a single request**, then +leave room for reasoning and the answer. The cumulative input count across tool +rounds measures usage and includes repeated history; it is not the context size. +Cached tokens are already part of input, and reasoning tokens are already part +of output. Do not add those subtotals twice. + +A chunk limit covers only the diff. Instructions, tool definitions, the output +schema, and context gathered during review need additional room. For example, +a chosen 128,000-token working budget might allocate 50,000 to the diff, 46,000 +to instructions and retrieved context, and 32,000 to reasoning and output. This +is a planning example, not a measured model quality threshold. DiffVouch reports +usage after requests; it does not yet enforce a total context budget or compact +history. Establish the useful working budget through review-quality evals rather +than assuming the model's maximum context is its best operating range. + +OpenAI documents the distinction between local text counts and full request +counts in its [token-counting guide](https://developers.openai.com/api/docs/guides/token-counting), +and explains context allocation in its [reasoning guide](https://developers.openai.com/api/docs/guides/reasoning). + Secret redaction is temporarily disabled because altering source lines produced false findings. Review the diff for credentials before running DiffVouch: the complete reviewable patch is sent unchanged to the selected AI provider. diff --git a/docs/eval-running.md b/docs/eval-running.md index e88f668..e9c83d8 100644 --- a/docs/eval-running.md +++ b/docs/eval-running.md @@ -55,3 +55,18 @@ diffvouch eval compare --before /path/to/earlier/run --after /path/to/later/run ``` A nonzero repeat selects the same deterministic approximately 20% subset for independent runs. `compare` requires the same manifest and writes observations on shared completed cases to `comparison.json`. Use it for code/prompt comparisons and repeat variation. Changes to the judge also change the measurement and should be assessed separately. No command enforces a regression threshold or CI quality gate. + +## Deferred task: evaluate chunk token limits + +Recorded 2026-09-21 at the user's request. Status: deferred; do not start until evaluations are explicitly resumed. Keep the current chunking implementation and configured value unchanged for now. + +Find the chunk token limit that gives the best review quality for the available subscription budget. Test sizes without changing how files or hunks are grouped, adding a synthesis pass, or modifying the prompt between size variants. + +- Freeze the reviewer implementation, the original prompt selected on 2026-09-30, model, effort, tools, judge/adjudicator settings, and case revisions. The completed prompt experiments remain separate from this deferred chunk-size experiment; do not attribute their differences to chunk size. +- Preselect a representative development cohort before inspecting outcomes. Include enough PRs whose actual token counts exceed the smaller limits, large individual files, changes involving multiple files, and varied languages/repositories. Small PRs that remain a single chunk under every setting are a control, not evidence of an optimal limit. Expand the development collection if the existing 30 personal PRs lack enough large cases. Preserve untouched confirmation cases and keep benchmark labels and competitor output out of reviewer prompts. +- Start with candidate limits of 20k, 35k, 50k, 75k, and 100k tokens, adjusted before the run if model/transport context constraints require it. Include a whole-diff condition only where sufficient context remains for instructions, tools, and reasoning/output. These are experiment values, not new defaults. +- Before running, ensure the eval runner can pass an explicit chunk limit through to the reviewer and record it in the run identity/configuration. Record the tokenizer and whether local counts are estimates. Keep the byte collection safety limit and other per-chunk limits fixed, and record chunk count and actual usage. Different chunk counts can change the total available tool calls and review time; report that effect rather than treating total compute as equal. +- Run paired cases for every setting, repeat a fixed subset to measure model variation, and retain failures and unfinished cases. Report reference coverage/precision/F2 where available, finding validity, concrete maintainability benefits, unresolved claims, completion rate, peak request input, cumulative usage, and elapsed time. Audit whether the prompt led to context inspection, execution of relevant existing tests/analyzers, candidate verification, and accurate reporting when evidence was unavailable. If saved output is insufficient to establish a behavior, mark it unknown rather than assume compliance. +- Compare paired results with uncertainty and repository breakdowns. Select the development setting based on quality and usage, then confirm on untouched cases before recommending a default. Do not add regression enforcement. Retain the current default if the results do not establish a useful improvement. + +Use the native ChatGPT subscription transport, stop when its allowance is exhausted, and never fall back to billable API calls. Use archived/published competitor results only; do not run competitor products for this experiment. diff --git a/docs/review-quality-improvements.md b/docs/review-quality-improvements.md index f8e1f2f..11f84bc 100644 --- a/docs/review-quality-improvements.md +++ b/docs/review-quality-improvements.md @@ -1,10 +1,12 @@ # Review quality improvements beyond prompt changes -Research date: 2026-09-21. These are proposed changes; this note does not implement them or resume evaluations. +Research date: 2026-09-21. The architectural options below are research proposals, not the current implementation plan. The original code observations refer to commit `86fb943`. The larger evaluation campaign remains paused; partial prompt experiments are reported separately. ## Recommendation -Build a staged review pipeline around the existing model call. The first priority is deterministic context and candidate coverage. The second is evidence-based verification and PR-level synthesis. Static analysis, tests, semantic deduplication, and adaptive compute should feed those stages. Explicit orchestration can enforce context selection and review coverage that the current model-directed tool use does not guarantee. +Decision on 2026-09-30: keep the original prompt. The experimental expanded prompt was removed from the working implementation at the user's request. Across the same eight completed personal PRs, the original produced 17 accepted claims, the installation-restricted variant produced 16 after the false-positive audit, and the corrected expanded variant produced 15 with one unresolved claim. These small, historical comparisons do not establish that either prompt is generally better. The supporting prompt evaluation and Tarpan retry reports remain local, uncommitted artifacts. + +The original instructions in [`buildPrompt`](../internal/review/review.go) permit repository inspection and relevant commands and tests when useful. The expanded workflow remains preserved in frozen experiment sources and reports. No architectural changes are selected here. The [deferred chunk-size experiment](eval-running.md#deferred-task-evaluate-chunk-token-limits) remains paused and will use the selected original prompt unless a later decision changes it. The ten-PR diagnostic supports this priority but does not prove it. Repository tools raised reference F2 from 41.4% to 47.2%, while the paired 95% interval was [-5.6, +15.2] points. The tool-enabled version still missed 17 of 32 benchmark issues, while the model judge marked only one of its 32 findings invalid. Unmatched claims are benchmark false positives even when the judge finds a real additional issue. These counts make coverage the first hypothesis to test. The small sample and model judge do not establish the cause, and they do not support blanket filtering. No version produced a valid maintainability finding. See [the paired report](eval-main-vs-tools-10.md#main-versus-repository-access-ten-prs). @@ -17,7 +19,7 @@ Research gives useful design precedents, not a guaranteed score increase: First-party product accounts report similar structures, but they are not controlled comparisons and their quality claims should not be treated as proof. Cursor describes an early eight-pass, majority-vote, validator pipeline, then says its larger gains came from dynamic context and tool use. Anthropic describes parallel detection, verification, severity ranking, and compute that scales with PR complexity. These accounts justify testing those components separately. They do not justify copying a fixed number of passes or filtering every single-pass candidate. [Cursor Bugbot account](https://cursor.com/blog/building-bugbot), [Anthropic Code Review account](https://claude.com/blog/code-review) -## What the current code does +## What the original research baseline did - `gitdiff.ChunkPatch` fills byte-limited chunks with whole file sections, then individual hunks when a file is too large. It has no dependency or symbol boundary model. [internal/gitdiff/gitdiff.go lines 383-424](../internal/gitdiff/gitdiff.go#L383-L424) - `review.Perform` starts a fresh review for each chunk and concatenates the results. There is no final model pass that reasons across chunk findings or recovers a dependency split between chunks. [internal/review/review.go lines 111-123](../internal/review/review.go#L111-L123) @@ -28,7 +30,7 @@ First-party product accounts report similar structures, but they are not control The paused full run `f003bd8b42da24a9` also shows why reliability needs its own workstream. Its local native records contain 81 completed cases and 30 errors: 19 checkout/dataset-patch mismatches, 9 incomplete subscription responses, 1 merge-base reconstruction failure, and 1 stream-read failure. Fixture preparation errors and provider transport errors should not be counted as review misses or mixed into an architectural quality conclusion. -## Ranked implementation plan +## Architectural options deferred for later evaluation | Rank | Change | Expected effect | Main risk and control | |---:|---|---|---| diff --git a/go.mod b/go.mod index aa98055..8eafe90 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.24.0 require ( github.com/gofrs/flock v0.12.1 github.com/spf13/cobra v1.10.1 + github.com/tiktoken-go/tokenizer v0.7.0 github.com/zalando/go-keyring v0.2.6 golang.org/x/term v0.35.0 gopkg.in/yaml.v3 v3.0.1 @@ -13,6 +14,7 @@ require ( require ( al.essio.dev/pkg/shellescape v1.5.1 // indirect github.com/danieljoos/wincred v1.2.2 // indirect + github.com/dlclark/regexp2 v1.11.5 // indirect github.com/godbus/dbus/v5 v5.1.0 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/spf13/pflag v1.0.9 // indirect diff --git a/go.sum b/go.sum index 88624aa..d51014e 100644 --- a/go.sum +++ b/go.sum @@ -5,6 +5,8 @@ github.com/danieljoos/wincred v1.2.2 h1:774zMFJrqaeYCK2W57BgAem/MLi6mtSE47MB6BOJ github.com/danieljoos/wincred v1.2.2/go.mod h1:w7w4Utbrz8lqeMbDAK0lkNJUv5sAOkFi7nd/ogr0Uh8= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/dlclark/regexp2 v1.11.5 h1:Q/sSnsKerHeCkc/jSTNq1oCm7KiVgUMZRDUoRu0JQZQ= +github.com/dlclark/regexp2 v1.11.5/go.mod h1:DHkYz0B9wPfa6wondMfaivmHpzrQ3v9q8cnmRbL6yW8= github.com/godbus/dbus/v5 v5.1.0 h1:4KLkAxT3aOY8Li4FRJe/KvhoNFFxo0m6fNuFUO8QJUk= github.com/godbus/dbus/v5 v5.1.0/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= github.com/gofrs/flock v0.12.1 h1:MTLVXXHf8ekldpJk3AKicLij9MdwOWkZ+a/jHHZby9E= @@ -24,6 +26,8 @@ github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +github.com/tiktoken-go/tokenizer v0.7.0 h1:VMu6MPT0bXFDHr7UPh9uii7CNItVt3X9K90omxL54vw= +github.com/tiktoken-go/tokenizer v0.7.0/go.mod h1:6UCYI/DtOallbmL7sSy30p6YQv60qNyU/4aVigPOx6w= github.com/zalando/go-keyring v0.2.6 h1:r7Yc3+H+Ux0+M72zacZoItR3UDxeWfKTcabvkI8ua9s= github.com/zalando/go-keyring v0.2.6/go.mod h1:2TCrxYrbUNYfNS/Kgy/LSrkSQzZ5UPVH85RwfczwvcI= golang.org/x/sys v0.36.0 h1:KVRy2GtZBrk1cBYA7MKu5bEZFxQk4NIDV6RLVcC8o0k= diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 8ac5575..49d94c2 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -43,6 +43,7 @@ type reviewFlags struct { excludes []string failBelow float64 maxDiffBytes int + chunkTokens int } func reviewCommand() *cobra.Command { @@ -68,6 +69,7 @@ func reviewCommand() *cobra.Command { command.Flags().Float64Var(&flags.failBelow, "fail-below", 0, "fail when rating is below this value") command.Flags().StringVar(&flags.failSeverity, "fail-on-severity", "", "fail at or above this severity") command.Flags().IntVar(&flags.maxDiffBytes, "max-diff-bytes", 0, "override diff safety limit") + command.Flags().IntVar(&flags.chunkTokens, "chunk-tokens", 0, "override diff chunk token limit (default 50000; excludes prompt and tool context)") command.Flags().StringVar(&flags.configPath, "config", "", "explicit repository config path") command.Flags().BoolVar(&flags.noColor, "no-color", false, "disable color output") command.Flags().BoolVar(&flags.verbose, "verbose", false, "enable verbose diagnostics") @@ -100,6 +102,9 @@ func runReview(command *cobra.Command, flags *reviewFlags) error { if flags.maxDiffBytes != 0 && flags.maxDiffBytes < 10_000 { return dv.New(dv.ExitArguments, "--max-diff-bytes must be at least 10000") } + if command.Flags().Changed("chunk-tokens") && flags.chunkTokens < 1000 { + return dv.New(dv.ExitArguments, "--chunk-tokens must be at least 1000") + } var failBelow *float64 if command.Flags().Changed("fail-below") { if flags.failBelow < 1 || flags.failBelow > 5 { @@ -150,7 +155,7 @@ func runReview(command *cobra.Command, flags *reviewFlags) error { ProviderName: flags.provider, Transport: flags.transport, Model: flags.model, Effort: flags.effort, Base: base, CommittedOnly: flags.committedOnly || flags.pr > 0 || flags.publish, StagedOnly: flags.stagedOnly, Excludes: flags.excludes, ConfigPath: flags.configPath, - MaxDiffBytes: flags.maxDiffBytes, FailBelow: failBelow, FailOnSeverity: flags.failSeverity, + MaxDiffBytes: flags.maxDiffBytes, ChunkTokens: flags.chunkTokens, FailBelow: failBelow, FailOnSeverity: flags.failSeverity, PublicationRequested: flags.publish, Root: repoRoot, Diagnostics: command.ErrOrStderr(), }) if err != nil { diff --git a/internal/config/config.go b/internal/config/config.go index 9393f43..8a838ec 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -31,6 +31,7 @@ type Review struct { Exclude []string `yaml:"exclude"` MaxDiffBytes int `yaml:"max_diff_bytes"` ChunkBytes int `yaml:"chunk_bytes"` + ChunkTokens int `yaml:"chunk_tokens"` } type QualityGate struct { @@ -55,7 +56,7 @@ func Defaults() Repository { "testing": 15, "scope": 10, }, MaxDiffBytes: 500_000, - ChunkBytes: 180_000, + ChunkTokens: 50_000, }, } } @@ -107,6 +108,22 @@ func LoadRepository(repo, explicit, trustedRef string) (Repository, error) { if err := decoder.Decode(&value); err != nil { return value, fmt.Errorf("invalid DiffVouch config %s: %w", source, err) } + // Preserve explicitly configured byte limits during migration. New configs + // use tokens; an explicit choice of both units is ambiguous. + var fields struct { + Review map[string]yaml.Node `yaml:"review"` + } + if err := yaml.Unmarshal(raw, &fields); err != nil { + return value, err + } + _, hasBytes := fields.Review["chunk_bytes"] + _, hasTokens := fields.Review["chunk_tokens"] + if hasBytes && hasTokens { + return value, errors.New("configure chunk_tokens or legacy chunk_bytes, not both") + } + if hasBytes { + value.Review.ChunkTokens = 0 + } if err := validate(value); err != nil { return value, fmt.Errorf("invalid DiffVouch config %s: %w", source, err) } @@ -120,8 +137,14 @@ func validate(value Repository) error { if value.Provider.DefaultTransport != "" && value.Provider.DefaultTransport != "cli" { return errors.New("repository config cannot select API transport") } - if value.Review.MaxDiffBytes < 10_000 || value.Review.ChunkBytes < 10_000 { - return errors.New("diff and chunk limits must be at least 10000 bytes") + if value.Review.MaxDiffBytes < 10_000 { + return errors.New("diff safety limit must be at least 10000 bytes") + } + if value.Review.ChunkTokens == 0 && value.Review.ChunkBytes < 10_000 { + return errors.New("chunk_tokens must be at least 1000, or legacy chunk_bytes at least 10000") + } + if value.Review.ChunkTokens != 0 && value.Review.ChunkTokens < 1000 { + return errors.New("chunk_tokens must be at least 1000") } total := 0 for _, name := range []string{"correctness", "security", "maintainability", "testing", "scope"} { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index d26e269..1f89944 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -66,6 +66,38 @@ func TestExternalExplicitConfigLoadsFromFilesystem(t *testing.T) { } } +func TestChunkLimitConfigurationMigration(t *testing.T) { + repo := configRepository(t) + for _, test := range []struct { + name, fields string + tokens, bytes int + invalid bool + }{ + {"default", "", 50000, 0, false}, + {"tokens", " chunk_tokens: 32000\n", 32000, 0, false}, + {"legacy", " chunk_bytes: 180000\n", 0, 180000, false}, + {"ambiguous", " chunk_tokens: 32000\n chunk_bytes: 180000\n", 0, 0, true}, + {"too small", " chunk_tokens: 999\n", 0, 0, true}, + {"zero", " chunk_tokens: 0\n", 0, 0, true}, + {"negative", " chunk_tokens: -1\n", 0, 0, true}, + {"legacy zero", " chunk_bytes: 0\n", 0, 0, true}, + } { + t.Run(test.name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "review.yml") + if err := os.WriteFile(path, []byte("version: 1\nreview:\n"+test.fields), 0o600); err != nil { + t.Fatal(err) + } + value, err := LoadRepository(repo, path, "HEAD") + if (err != nil) != test.invalid { + t.Fatalf("unexpected validation: %v", err) + } + if err == nil && (value.Review.ChunkTokens != test.tokens || value.Review.ChunkBytes != test.bytes) { + t.Fatalf("wrong limits: %#v", value.Review) + } + }) + } +} + func TestRepositoryConfigCannotSelectAPI(t *testing.T) { repo := configRepository(t) path := filepath.Join(repo, "unsafe.yml") diff --git a/internal/gitdiff/gitdiff.go b/internal/gitdiff/gitdiff.go index a379669..582a6ab 100644 --- a/internal/gitdiff/gitdiff.go +++ b/internal/gitdiff/gitdiff.go @@ -380,24 +380,60 @@ func uniqueSorted(values []string) []string { return result } +// ChunkPatch preserves the legacy byte-based limit. func ChunkPatch(patch string, limit int) ([]string, error) { - if len([]byte(patch)) <= limit { + return chunkPatch(patch, limit, "bytes", func(s string) (int, error) { return len(s), nil }) +} + +// ChunkPatchTokens counts complete candidates, including repeated file headers, +// so tokenizer merges at boundaries cannot make a chunk exceed its limit. +func ChunkPatchTokens(patch string, limit int, count func(string) (int, error)) ([]string, error) { + return chunkPatch(patch, limit, "tokens", count) +} + +func chunkPatch(patch string, limit int, unit string, count func(string) (int, error)) ([]string, error) { + if limit <= 0 { + return nil, dv.New(dv.ExitArguments, "chunk limit must be positive") + } + size, err := count(patch) + if err != nil { + return nil, err + } + if size <= limit { return []string{patch}, nil } var chunks []string - var current strings.Builder + var current string + appendPiece := func(piece string) error { + size, err := count(current + piece) + if err != nil { + return err + } + if current != "" && size > limit { + chunks = append(chunks, current) + current = "" + } + current += piece + return nil + } for _, section := range SplitFilePatches(patch) { - if current.Len() > 0 && current.Len()+len(section) > limit { - chunks = append(chunks, current.String()) - current.Reset() + size, err := count(section) + if err != nil { + return nil, err } - if len(section) <= limit { - current.WriteString(section) + if size <= limit { + if err := appendPiece(section); err != nil { + return nil, err + } continue } + if current != "" { + chunks = append(chunks, current) + current = "" + } hunkLocation := hunkHeader.FindStringIndex(section) if hunkLocation == nil { - return nil, dv.New(dv.ExitCoverage, "a single changed file exceeds the chunk limit") + return nil, dv.New(dv.ExitCoverage, fmt.Sprintf("a single changed file exceeds the chunk limit of %d %s", limit, unit)) } hunkAt := hunkLocation[0] header := section[:hunkAt] @@ -408,18 +444,20 @@ func ChunkPatch(patch string, limit int) ([]string, error) { end = hunkStarts[index+1][0] } piece := header + section[hunkAt+location[0]:hunkAt+end] - if len(piece) > limit { - return nil, dv.New(dv.ExitCoverage, "a single diff hunk exceeds the chunk limit") + size, err := count(piece) + if err != nil { + return nil, err + } + if size > limit { + return nil, dv.New(dv.ExitCoverage, fmt.Sprintf("a single diff hunk exceeds the chunk limit of %d %s", limit, unit)) } - if current.Len() > 0 && current.Len()+len(piece) > limit { - chunks = append(chunks, current.String()) - current.Reset() + if err := appendPiece(piece); err != nil { + return nil, err } - current.WriteString(piece) } } - if current.Len() > 0 { - chunks = append(chunks, current.String()) + if current != "" { + chunks = append(chunks, current) } return chunks, nil } diff --git a/internal/gitdiff/token_chunks_test.go b/internal/gitdiff/token_chunks_test.go new file mode 100644 index 0000000..45db9cd --- /dev/null +++ b/internal/gitdiff/token_chunks_test.go @@ -0,0 +1,59 @@ +package gitdiff + +import ( + "errors" + "strings" + "testing" + + "github.com/divyangchauhan/DiffVouch/internal/tokens" +) + +func TestTokenChunksPreservePatchAndRespectLimit(t *testing.T) { + counter, err := tokens.New("gpt-5") + if err != nil { + t.Fatal(err) + } + header := "diff --git a/a.go b/a.go\n--- a/a.go\n+++ b/a.go\n" + first := "@@ -1 +1 @@\n-old\n+" + strings.Repeat("こんにちは世界 ", 40) + "\n" + second := "@@ -20 +20 @@\n-old2\n+" + strings.Repeat("你好世界 ", 40) + "\n" + a, _ := counter.Count(header + first) + b, _ := counter.Count(header + second) + limit := max(a, b) + chunks, err := ChunkPatchTokens(header+first+second, limit, counter.Count) + if err != nil { + t.Fatal(err) + } + if len(chunks) != 2 || chunks[0] != header+first || chunks[1] != header+second { + t.Fatalf("hunks or repeated file headers lost: %#v", chunks) + } + for _, chunk := range chunks { + count, err := counter.Count(chunk) + if err != nil || count > limit { + t.Fatalf("chunk exceeds token budget: %d > %d: %v", count, limit, err) + } + } + // With enough space, retain one whole patch without duplicating its header. + whole := header + first + second + count, _ := counter.Count(whole) + chunks, err = ChunkPatchTokens(whole, count, counter.Count) + if err != nil || len(chunks) != 1 || chunks[0] != whole { + t.Fatalf("whole-patch boundary failed: %#v %v", chunks, err) + } + // Whole file boundaries are preserved when packing multiple files as well. + files := header + first + strings.ReplaceAll(header, "a.go", "b.go") + second + chunks, err = ChunkPatchTokens(files, limit+5, counter.Count) + if err != nil || len(chunks) != 2 || strings.Join(chunks, "") != files { + t.Fatalf("file packing lost patch content: %#v %v", chunks, err) + } + if _, err := ChunkPatchTokens(header+first, a-1, counter.Count); err == nil || !strings.Contains(err.Error(), "tokens") { + t.Fatalf("oversized hunk should fail explicitly: %v", err) + } +} + +func TestTokenChunkCounterFailureIsNotIgnored(t *testing.T) { + want := errors.New("tokenization failed") + _, err := ChunkPatchTokens("patch", 100, func(string) (int, error) { return 0, want }) + if !errors.Is(err, want) { + t.Fatalf("lost tokenization error: %v", err) + } +} diff --git a/internal/model/model.go b/internal/model/model.go index ef95771..0092e7f 100644 --- a/internal/model/model.go +++ b/internal/model/model.go @@ -103,6 +103,29 @@ type ReviewResult struct { Files FilesSummary `json:"files"` Gate Gate `json:"gate"` Publication Publication `json:"publication"` + Tokens *TokenReport `json:"tokens,omitempty"` +} + +// TokenUsage comes from the provider, never from the review model's prose. +// Cached input is part of input; reasoning is part of output, not extra tokens. +type TokenUsage struct { + InputTokens int `json:"inputTokens"` + OutputTokens int `json:"outputTokens"` + CachedInputTokens int `json:"cachedInputTokens"` + ReasoningTokens int `json:"reasoningTokens"` +} + +type ChunkTokens struct { + DiffTokens int `json:"diffTokens"` + PromptTextTokens int `json:"promptTextTokens"` + Requests []*TokenUsage `json:"requests"` +} + +type TokenReport struct { + Tokenizer string `json:"tokenizer"` + Estimated bool `json:"estimated"` + ChunkLimitTokens int `json:"chunkLimitTokens,omitempty"` + Chunks []ChunkTokens `json:"chunks"` } func NewReviewID() string { diff --git a/internal/provider/api.go b/internal/provider/api.go index 4799f5c..1b58569 100644 --- a/internal/provider/api.go +++ b/internal/provider/api.go @@ -27,6 +27,7 @@ type apiAdapter struct { client *http.Client subscription bool beforeCall func(context.Context) error + onUsage func(*model.TokenUsage) } func (a *apiAdapter) Model() string { return a.model } @@ -34,7 +35,7 @@ func (a *apiAdapter) Model() string { return a.model } func (a *apiAdapter) Review(prompt Prompt) (model.ProviderReview, error) { interruptCtx, stop := signal.NotifyContext(context.Background(), os.Interrupt) defer stop() - ctx, cancel := context.WithTimeout(interruptCtx, 10*time.Minute) + ctx, cancel := context.WithTimeout(interruptCtx, 20*time.Minute) defer cancel() if a.name == "codex" { return a.reviewOpenAI(ctx, prompt) @@ -58,6 +59,30 @@ type apiBlock struct { type openAIResponse struct { Status string `json:"status"` Output []json.RawMessage `json:"output"` + Usage *openAIUsage `json:"usage"` +} + +type openAIUsage struct { + InputTokens int `json:"input_tokens"` + OutputTokens int `json:"output_tokens"` + InputDetails struct { + CachedTokens int `json:"cached_tokens"` + } `json:"input_tokens_details"` + OutputDetails struct { + ReasoningTokens int `json:"reasoning_tokens"` + } `json:"output_tokens_details"` +} + +func (a *apiAdapter) reportUsage(usage *openAIUsage) { + if a.onUsage == nil { + return + } + if usage == nil { + a.onUsage(nil) + return + } + a.onUsage(&model.TokenUsage{InputTokens: usage.InputTokens, OutputTokens: usage.OutputTokens, + CachedInputTokens: usage.InputDetails.CachedTokens, ReasoningTokens: usage.OutputDetails.ReasoningTokens}) } func (a *apiAdapter) reviewOpenAI(ctx context.Context, prompt Prompt) (model.ProviderReview, error) { @@ -114,6 +139,7 @@ func (a *apiAdapter) generateOpenAI(ctx context.Context, prompt Prompt, outputSc if err != nil { return nil, err } + a.reportUsage(response.Usage) if response.Status != "completed" { return nil, dv.New(dv.ExitProvider, "OpenAI response did not complete") } diff --git a/internal/provider/provider.go b/internal/provider/provider.go index 476c11b..377a279 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -36,6 +36,7 @@ type Options struct { Effort string Root string BeforeCall func(context.Context) error + OnUsage func(*model.TokenUsage) } func New(options Options) (Adapter, error) { @@ -48,12 +49,12 @@ func New(options Options) (Adapter, error) { if options.Model == "" { return nil, dv.New(dv.ExitProvider, "OpenAI API transport requires --model or a trusted repository model") } - return &apiAdapter{name: "codex", model: options.Model, effort: options.Effort, root: options.Root}, nil + return &apiAdapter{name: "codex", model: options.Model, effort: options.Effort, root: options.Root, onUsage: options.OnUsage}, nil case "codex/subscription": if options.Model == "" { return nil, dv.New(dv.ExitProvider, "ChatGPT subscription transport requires --model or a trusted repository model") } - return &apiAdapter{name: "codex", subscription: true, model: options.Model, effort: options.Effort, root: options.Root, beforeCall: options.BeforeCall}, nil + return &apiAdapter{name: "codex", subscription: true, model: options.Model, effort: options.Effort, root: options.Root, beforeCall: options.BeforeCall, onUsage: options.OnUsage}, nil case "claude/api": if options.Model == "" { return nil, dv.New(dv.ExitProvider, "Anthropic API transport requires --model or a trusted repository model") @@ -342,6 +343,6 @@ func GenerateJSON(ctx context.Context, options Options, prompt Prompt, outputSch } ctx, cancel := context.WithTimeout(ctx, 10*time.Minute) defer cancel() - a := &apiAdapter{name: "codex", subscription: true, model: options.Model, effort: options.Effort, root: options.Root, beforeCall: options.BeforeCall} + a := &apiAdapter{name: "codex", subscription: true, model: options.Model, effort: options.Effort, root: options.Root, beforeCall: options.BeforeCall, onUsage: options.OnUsage} return a.generateOpenAI(ctx, prompt, outputSchema) } diff --git a/internal/provider/subscription_test.go b/internal/provider/subscription_test.go index 238d60e..a406d26 100644 --- a/internal/provider/subscription_test.go +++ b/internal/provider/subscription_test.go @@ -16,6 +16,7 @@ import ( "github.com/divyangchauhan/DiffVouch/internal/chatgpt" "github.com/divyangchauhan/DiffVouch/internal/config" + "github.com/divyangchauhan/DiffVouch/internal/model" "github.com/divyangchauhan/DiffVouch/internal/secret" ) @@ -52,7 +53,9 @@ func TestSubscriptionReviewUsesNativeToolsAndOpaqueHistory(t *testing.T) { if err := os.WriteFile(filepath.Join(repo, "context.txt"), []byte("repository evidence"), 0o600); err != nil { t.Fatal(err) } - adapter, err := New(Options{Name: "codex", Transport: "subscription", Root: repo, Model: "subscription-model", Effort: "high"}) + var usage []*model.TokenUsage + adapter, err := New(Options{Name: "codex", Transport: "subscription", Root: repo, Model: "subscription-model", Effort: "high", + OnUsage: func(value *model.TokenUsage) { usage = append(usage, value) }}) if err != nil { t.Fatal(err) } @@ -74,7 +77,9 @@ func TestSubscriptionReviewUsesNativeToolsAndOpaqueHistory(t *testing.T) { return streamResponse( map[string]any{"type": "response.output_item.done", "output_index": 0, "item": map[string]any{"type": "reasoning", "encrypted_content": "opaque-context", "summary": []any{}}}, map[string]any{"type": "response.output_item.done", "output_index": 1, "item": toolCall("codex", "read-1", "read_file", map[string]any{"path": "context.txt", "offset": 0, "max_bytes": 100})}, - map[string]any{"type": "response.completed", "response": map[string]any{"status": "completed"}}, + map[string]any{"type": "response.completed", "response": map[string]any{"status": "completed", "usage": map[string]any{ + "input_tokens": 100, "output_tokens": 10, "input_tokens_details": map[string]int{"cached_tokens": 60}, "output_tokens_details": map[string]int{"reasoning_tokens": 4}, + }}}, ), nil } if calls != 2 { @@ -90,6 +95,9 @@ func TestSubscriptionReviewUsesNativeToolsAndOpaqueHistory(t *testing.T) { if err != nil || result.Summary != "No issues." || calls != 2 { t.Fatalf("native subscription review failed: %v", err) } + if len(usage) != 2 || usage[0] == nil || *usage[0] != (model.TokenUsage{InputTokens: 100, OutputTokens: 10, CachedInputTokens: 60, ReasoningTokens: 4}) || usage[1] != nil { + t.Fatalf("lost stream usage or reported missing usage as zero: %#v", usage) + } } func TestSubscriptionRefreshesOnceAfter401(t *testing.T) { diff --git a/internal/render/render.go b/internal/render/render.go index 4c7f5a6..908c06c 100644 --- a/internal/render/render.go +++ b/internal/render/render.go @@ -49,6 +49,9 @@ func Terminal(result model.ReviewResult) string { fmt.Fprintf(&output, " using %s", result.Provider.Model) } output.WriteString(".\n") + if result.Tokens != nil { + writeTokens(&output, result.Tokens) + } if result.Files.Redactions > 0 { fmt.Fprintf(&output, "%d potential secret(s) were redacted before provider submission.\n", result.Files.Redactions) } @@ -71,6 +74,44 @@ func Terminal(result model.ReviewResult) string { return output.String() } +func writeTokens(output *strings.Builder, report *model.TokenReport) { + label := "local text count" + if report.Estimated { + label = "estimated for this model" + } + fmt.Fprintf(output, "Tokens (%s, %s):\n", sanitize.TerminalText(report.Tokenizer), label) + if report.ChunkLimitTokens > 0 { + fmt.Fprintf(output, " Diff chunk limit: %d tokens.\n", report.ChunkLimitTokens) + } + var input, cached, generated, reasoning, peak, reported, unknown int + for i, chunk := range report.Chunks { + fmt.Fprintf(output, " Chunk %d: %d diff / %d prompt text tokens.\n", i+1, chunk.DiffTokens, chunk.PromptTextTokens) + if len(chunk.Requests) == 0 { + unknown++ + } + for _, usage := range chunk.Requests { + if usage == nil { + unknown++ + continue + } + reported++ + input += usage.InputTokens + cached += usage.CachedInputTokens + generated += usage.OutputTokens + reasoning += usage.ReasoningTokens + peak = max(peak, usage.InputTokens) + } + } + output.WriteString(" Prompt text excludes tool definitions, output schema, message framing, and later tool context.\n") + if reported > 0 { + fmt.Fprintf(output, " Provider-reported peak input: %d tokens. Leave additional room for reasoning and output.\n", peak) + fmt.Fprintf(output, " Usage across %d reported requests: %d input (%d cached), %d output (%d reasoning included).\n", reported, input, cached, generated, reasoning) + } + if unknown > 0 { + output.WriteString(" Provider usage is unavailable for some or all requests; reported totals and peak may be incomplete.\n") + } +} + func filter(findings []model.Finding, blocking bool) []model.Finding { var result []model.Finding for _, finding := range findings { diff --git a/internal/render/render_test.go b/internal/render/render_test.go index 99973d2..75c6d90 100644 --- a/internal/render/render_test.go +++ b/internal/render/render_test.go @@ -33,6 +33,28 @@ func TestTerminalEscapesFindingPathControls(t *testing.T) { } } +func TestTokenReportSeparatesContextPeakFromCumulativeUsage(t *testing.T) { + result := model.ReviewResult{Tokens: &model.TokenReport{Tokenizer: "o200k_base", Estimated: true, ChunkLimitTokens: 50000, + Chunks: []model.ChunkTokens{ + {DiffTokens: 30000, PromptTextTokens: 32000, Requests: []*model.TokenUsage{ + {InputTokens: 35000, OutputTokens: 1000, CachedInputTokens: 10000, ReasoningTokens: 600}, + {InputTokens: 45000, OutputTokens: 2000, CachedInputTokens: 30000, ReasoningTokens: 1500}, + }}, + {DiffTokens: 5000, PromptTextTokens: 7000, Requests: []*model.TokenUsage{nil}}, + }, + }} + output := Terminal(result) + for _, expected := range []string{"estimated for this model", "peak input: 45000", "80000 input (40000 cached)", "3000 output (2100 reasoning included)", "may be incomplete"} { + if !strings.Contains(output, expected) { + t.Fatalf("missing %q in %s", expected, output) + } + } + encoded, err := JSON(result) + if err != nil || !strings.Contains(encoded, `"requests": [`) || !strings.Contains(encoded, "null") { + t.Fatalf("usage missing from JSON: %s %v", encoded, err) + } +} + func TestTerminalEscapesProviderControlledText(t *testing.T) { result := model.ReviewResult{ Summary: "summary\nforged\x1b]52;c;clipboard\a", diff --git a/internal/review/review.go b/internal/review/review.go index b3a5ca2..f7ee741 100644 --- a/internal/review/review.go +++ b/internal/review/review.go @@ -17,6 +17,7 @@ import ( "github.com/divyangchauhan/DiffVouch/internal/model" "github.com/divyangchauhan/DiffVouch/internal/provider" "github.com/divyangchauhan/DiffVouch/internal/rating" + "github.com/divyangchauhan/DiffVouch/internal/tokens" ) type Options struct { @@ -30,6 +31,7 @@ type Options struct { Excludes []string ConfigPath string MaxDiffBytes int + ChunkTokens int FailBelow *float64 FailOnSeverity string PublicationRequested bool @@ -99,11 +101,38 @@ func Perform(options Options) (*model.ReviewResult, *model.FilesSummary, error) return nil, files, nil } providerPatch, redactions := prepareProviderPatch(collected.Patch) - chunks, err := gitdiff.ChunkPatch(providerPatch, repositoryConfig.Review.ChunkBytes) + counter, err := tokens.New(selectedModel) + if err != nil { + return nil, nil, dv.Wrap(dv.ExitProvider, "initialize token counter", err) + } + chunkLimit := repositoryConfig.Review.ChunkTokens + if options.ChunkTokens != 0 { + if options.ChunkTokens < 1000 { + return nil, nil, dv.New(dv.ExitArguments, "chunk-tokens must be at least 1000") + } + chunkLimit = options.ChunkTokens + } + var chunks []string + if chunkLimit > 0 { + chunks, err = gitdiff.ChunkPatchTokens(providerPatch, chunkLimit, counter.Count) + } else { + chunks, err = gitdiff.ChunkPatch(providerPatch, repositoryConfig.Review.ChunkBytes) + } if err != nil { return nil, nil, err } - adapter, err := provider.New(provider.Options{Name: options.ProviderName, Transport: transport, Model: selectedModel, Effort: options.Effort, Root: repo, BeforeCall: options.BeforeCall}) + tokenReport := &model.TokenReport{Tokenizer: counter.GetName(), Estimated: counter.Estimated, + ChunkLimitTokens: chunkLimit, Chunks: make([]model.ChunkTokens, len(chunks))} + chunkIndex := 0 + onUsage := func(usage *model.TokenUsage) { + stats := &tokenReport.Chunks[chunkIndex] + stats.Requests = append(stats.Requests, usage) + if options.Diagnostics != nil && usage != nil { + fmt.Fprintf(options.Diagnostics, "Chunk %d/%d request %d: %d input tokens, %d output tokens (includes %d reasoning), %d cached input.\n", + chunkIndex+1, len(chunks), len(stats.Requests), usage.InputTokens, usage.OutputTokens, usage.ReasoningTokens, usage.CachedInputTokens) + } + } + adapter, err := provider.New(provider.Options{Name: options.ProviderName, Transport: transport, Model: selectedModel, Effort: options.Effort, Root: repo, BeforeCall: options.BeforeCall, OnUsage: onUsage}) if err != nil { return nil, nil, err } @@ -111,8 +140,31 @@ func Perform(options Options) (*model.ReviewResult, *model.FilesSummary, error) reviews := make([]model.ProviderReview, 0, len(chunks)) sizes := make([]int, 0, len(chunks)) for index, chunk := range chunks { + chunkIndex = index prompt := buildPrompt(chunk, index+1, len(chunks), repositoryConfig.Review.Rubric, repositoryConfig.Review.Instructions) prompt.System += fmt.Sprintf("\nReview scope: mode=%s, head=%s, committed-only=%t, staged-only=%t. For committed-only reviews, inspect files at the specified head with git show; working-tree edits are outside the review. For staged-only reviews, inspect index contents with git show :path; unstaged edits are outside the review. Otherwise inspect the working tree. Do not switch branches or overwrite local changes to inspect another revision.", collected.Mode, collected.HeadSHA, options.CommittedOnly, options.StagedOnly) + stats := &tokenReport.Chunks[index] + stats.DiffTokens, err = counter.Count(chunk) + if err != nil { + return nil, nil, err + } + systemTokens, countErr := counter.Count(prompt.System) + if countErr != nil { + return nil, nil, countErr + } + userTokens, countErr := counter.Count(prompt.User) + if countErr != nil { + return nil, nil, countErr + } + stats.PromptTextTokens = systemTokens + userTokens + stats.Requests = []*model.TokenUsage{} + if options.Diagnostics != nil { + fmt.Fprintf(options.Diagnostics, "Chunk %d/%d: %d diff tokens, %d prompt text tokens (%s; excludes tool/schema overhead and subsequent context).\n", + index+1, len(chunks), stats.DiffTokens, stats.PromptTextTokens, counter.GetName()) + if counter.Estimated { + fmt.Fprintln(options.Diagnostics, "Model tokenizer is unknown locally; these text counts are estimates for the selected model.") + } + } providerReview, reviewErr := adapter.Review(prompt) if reviewErr != nil { return nil, nil, reviewErr @@ -151,6 +203,7 @@ func Perform(options Options) (*model.ReviewResult, *model.FilesSummary, error) Files: model.FilesSummary{Reviewed: collected.ReviewedFiles, Excluded: collected.ExcludedFiles, Binary: collected.BinaryFiles, Omitted: []string{}, Redactions: redactions}, Gate: rating.Gate(calculatedRating, combined.Findings, failBelow, failSeverity), Publication: model.Publication{Requested: options.PublicationRequested}, + Tokens: tokenReport, } return result, nil, nil } diff --git a/internal/tokens/tokens.go b/internal/tokens/tokens.go new file mode 100644 index 0000000..56807d9 --- /dev/null +++ b/internal/tokens/tokens.go @@ -0,0 +1,20 @@ +// Package tokens counts text locally, without contacting a model provider. +package tokens + +import "github.com/tiktoken-go/tokenizer" + +type Counter struct { + tokenizer.Codec + Estimated bool +} + +// Unknown model tokenizers, including Claude, use an explicitly labeled +// o200k_base estimate. Provider usage remains authoritative for full requests. +func New(model string) (Counter, error) { + codec, err := tokenizer.ForModel(tokenizer.Model(model)) + if err == nil { + return Counter{Codec: codec}, nil + } + codec, err = tokenizer.Get(tokenizer.O200kBase) + return Counter{Codec: codec, Estimated: true}, err +} diff --git a/internal/tokens/tokens_test.go b/internal/tokens/tokens_test.go new file mode 100644 index 0000000..ec00a6a --- /dev/null +++ b/internal/tokens/tokens_test.go @@ -0,0 +1,21 @@ +package tokens + +import "testing" + +func TestKnownAndUnknownModelCounts(t *testing.T) { + for _, model := range []string{"gpt-5", "future-model", "claude-model", ""} { + counter, err := New(model) + if err != nil { + t.Fatal(err) + } + if counter.Estimated != (model != "gpt-5") { + t.Fatalf("model %q has incorrect estimate label", model) + } + for text, want := range map[string]int{"": 0, "hello world": 2, "Hello, world!": 4} { + got, err := counter.Count(text) + if err != nil || got != want { + t.Fatalf("%q: count %d, want %d, err %v", text, got, want, err) + } + } + } +}