-
Notifications
You must be signed in to change notification settings - Fork 25
Add configurable model support for all agents #126
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
Merged
Merged
+1,432
−285
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
## Summary - Introduces a new function `filterOpencodeToolCallLines` to clean up the output from the OpenCode CLI by removing JSON lines that represent tool calls, which are not needed for the review process. - Adds a helper function `isOpencodeToolCallLine` to identify these JSON lines. ## Changes - Updated the `Review` method to utilize the new filtering function on the command output before returning the result. ## Test plan - [ ] Verify that the output from `OpenCodeAgent.Review` no longer includes tool-call JSON lines. Generated with [Cursor](https://cursor.com)
- Require exactly 2 keys (name, arguments) to identify tool calls - Use TrimRight instead of TrimSpace to preserve leading indentation - Add tests for JSON examples with extra keys and indented content Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add model configuration support to allow specifying which model opencode should use. This is useful when the default model doesn't support certain features or when you want to use a specific model for reviews. Changes: - Add `opencode_model` to config.toml for global default - Add `opencode_model` to .roborev.toml for per-repo override - Add `--model` flag to run, review, and refine commands - Add `WithModel(model string)` to Agent interface - Pass `--model` flag to opencode when configured - Add `model` column to review_jobs table with migration The model format for opencode is "provider/model", e.g., "anthropic/claude-sonnet-4-20250514". Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Extend model configuration from opencode-only to all agents (codex, claude, gemini, copilot). Each agent now properly stores and passes the model flag to its CLI (-m for codex/gemini, --model for claude/ copilot). Renamed config from opencode_model to model/default_model for generality. Added tests for ResolveModel config resolution and agent model persistence through WithReasoning/WithAgentic method chains. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…docs WithReasoning was returning the same instance instead of a copy, which could cause issues with method chaining. Now returns a copy that preserves all fields including Model. Also improved the filterOpencodeToolCallLines comment to document that it filters the standard LLM tool call format that may occasionally leak through when streaming, not OpenCode-specific output. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Addresses review finding about testing gap: the model flag plumbing was not covered for Copilot and OpenCode. Added integration tests that verify --model is passed to both agents. Also clarified ResolveModel comment to document which config file uses which key (default_model in config.toml, model in .roborev.toml). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Update CLI --model help text to be generic (not opencode-specific) - Add model column to Postgres schema with v1->v2 migration - Include model in UpsertJob, PulledJob, and PullJobs for sync - Add model to SyncableJob and GetJobsToSync for push sync - Include model in ListJobs and GetJobByID for API responses Addresses review findings #2478 and #2479. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add COALESCE for model in UpsertJob (postgres.go) and UpsertPulledJob (sync.go) ON CONFLICT clauses so existing jobs get model backfilled when the incoming row has one - Add integration test for v1→v2 Postgres migration that verifies: - Schema version advances from 1 to 2 - Model column is added to review_jobs table - Existing jobs remain accessible with NULL model Addresses review findings #2483. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Use schema-qualified table names (roborev.schema_version, etc.) in MigratesV1ToV2 test instead of SET search_path, which only affects one connection in a pool - Add TestIntegration_UpsertJob_BackfillsModel to verify Postgres COALESCE behavior: existing NULL model gets backfilled, and empty model doesn't clear existing value - Add TestUpsertPulledJob_BackfillsModel for SQLite path with same verification Addresses review findings #2487. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add internal/storage/schemas/postgres_v1.sql and postgres_v2.sql - v1→v2 migration test now loads schema from embedded SQL file - Makes schema differences between versions easy to diff - Simplifies test by removing inline schema definitions Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- postgres.go now embeds schemas/postgres_v2.sql instead of defining schema as Go string slice - pgSchemaStatements() parses the SQL file into individual statements - SQL files use schema-qualified names (roborev.table_name) for clarity - Removes ~100 lines of inline schema definition from postgres.go - Single source of truth: edit the SQL file to change schema Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Drop existing schema at start of test so it runs regardless of prior state - Fix SQL parsing to handle statements with leading comments (match pgSchemaStatements() logic) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…tion - Add test job insertion before v1→v2 migration to verify data survives - Verify pre-existing job has model=NULL after migration - Add CREATE SCHEMA assertion to TestPgSchemaStatementsContainsRequiredTables - Document that v1 SQL is the sync schema (completed jobs only) Addresses review findings #2495 and #2496. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Fixes FK error when sequence isn't reset in reused test database. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
EnqueueJob and EnqueueRangeJob now require model parameter. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Adds
--modelflag support to all agents (codex, claude-code, gemini, copilot, opencode), allowing users to specify which model an agent should use. This lays the groundwork for the more comprehensive agent/model-per-reasoning-level configuration described in #125.Changes
CLI
--modelflag toreview,refine, andruncommandsprovider/model)Configuration
default_modelin~/.roborev/config.toml- global default modelmodelin.roborev.toml- per-repo model overrideAgent Interface
WithModel(model string) Agent--model,-m, etc.)Database & Sync
modelcolumn to Postgres schema (v1→v2 migration)Schema Management
internal/storage/schemas/*.sqlfilesBug Fixes
OpenCodeAgent.WithReasoningto preserve Model fieldRelationship to #125
This PR provides the foundation for the model-per-reasoning-level feature proposed in #125:
WithModel()on all agentsdefault_model/modelconfigmodelcolumn in databaseResolveModel()functionResolveReviewModel(reasoning, ...)The database schema does not need further changes - the single
modelcolumn stores whichever model was resolved at enqueue time, regardless of selection mechanism.Supersedes
Supersedes #124 (OpenCode tool-call filtering) - that fix is included here along with improved documentation.
Test Plan
ResolveModel()config resolutionWithModel()persistence across all agentsbuildArgs()