feat: integrate amp and ampcode configurations - #390
Conversation
Summary of ChangesHello @shunkakinoki, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request integrates the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis pull request updates model provider configurations, modifies package dependencies, and restructures Linux package management. It removes Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
🧰 Additional context used📓 Path-based instructions (5)**/*.{js,ts,jsx,tsx,json}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{json,yaml,yml,toml}📄 CodeRabbit inference engine (.cursor/rules/formatting.mdc)
Files:
**/*.{js,jsx,ts,tsx,json}📄 CodeRabbit inference engine (.cursor/rules/formatting.mdc)
Files:
**/*.{js,jsx,ts,tsx,json,jsonc,md}📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Files:
**/*.{yaml,yml,toml}📄 CodeRabbit inference engine (.cursor/rules/formatting.mdc)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Mesa DescriptionTL;DRIntegrates Sourcegraph AMP for AI proxy functionality, enables What changed?
Description generated by Mesa. Update settings |
There was a problem hiding this comment.
Code Review
This pull request integrates @sourcegraph/amp for AI proxy functionality. The changes correctly add the new dependency, configure it in cliproxyapi, and update the model provider configurations. My review focuses on two main points: the use of a direct git dependency for @sourcegraph/amp, which poses a stability and security risk, and a potentially incorrect provider name in the custom models configuration. Addressing these points will improve the robustness and maintainability of the project.
| "@github/copilot": "^0.0.366", | ||
| "@google/jules": "^0.1.40", | ||
| "@nanocollective/nanocoder": "^1.16.3", | ||
| "@sourcegraph/amp": "^0.0.1764893126-gb1ffc3", |
There was a problem hiding this comment.
The @sourcegraph/amp package is being added with a version that points to a specific git commit (^0.0.1764893126-gb1ffc3). Using direct git dependencies can introduce instability and security risks, as they may not have gone through the same vetting process as packages published to a registry. This also affects the bun.lock file. It is recommended to use a stable, published version from a package registry (like npm) if one is available to ensure better dependency management and security.
| "model": "gemini-3-pro-preview", | ||
| "base_url": "http://127.0.0.1:8317/v1", | ||
| "api_key": "sk-dummy", | ||
| "provider": "openai" | ||
| "provider": "antigravity" |
There was a problem hiding this comment.
The provider for the gemini-3-pro-preview model is set to antigravity. This is an unconventional provider name and might be a placeholder. Using a placeholder or incorrect provider name can lead to runtime errors when this model is invoked. This should be updated to the correct provider identifier if antigravity is not the intended final value.
There was a problem hiding this comment.
Pull request overview
This PR integrates the Sourcegraph AMP (Autonomous Model Proxy) package into the project by adding the @sourcegraph/amp npm dependency and configuring the ampcode service. The changes transition from a standalone CLI tool (amp-cli in Nix) to an npm package-based integration, while also reorganizing the custom model configurations in the factory config.
- Added
@sourcegraph/ampnpm package dependency with version^0.0.1764893126-gb1ffc3 - Removed
amp-clifrom Nix packages (replaced by npm package) - Reordered custom model configurations and updated provider associations
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| package.json | Added @sourcegraph/amp dependency to both dependencies and trustedDependencies arrays |
| home-manager/packages/default.nix | Removed amp-cli package (superseded by npm package) |
| config/factory/config.json | Reordered custom models and updated provider mappings (Claude models moved to top, gemini model provider changed to "antigravity") |
| config/cliproxyapi/config.yaml | Added blank line for formatting in ampcode configuration section |
| bun.lock | Updated lock file with @sourcegraph/amp and its dependencies (@napi-rs/keyring with platform-specific binaries) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "base_url": "http://127.0.0.1:8317/v1", | ||
| "api_key": "sk-dummy", | ||
| "provider": "openai" | ||
| "provider": "antigravity" |
There was a problem hiding this comment.
The provider "antigravity" is not a valid provider. Based on the config.yaml file which lists valid protocols as "openai", "gemini", "claude", and "codex", this should likely be "gemini" for a model named "gemini-3-pro-preview".
| "provider": "antigravity" | |
| "provider": "gemini" |
There was a problem hiding this comment.
Performed full review of 52e0242...b064884
Analysis
-
Incomplete AMP Integration: The AMP configuration appears partially implemented without clear documentation on authentication flow, whether it uses OpenAI-compatible endpoints or dedicated routes, and missing AMP configuration in the custom models list.
-
Unclear Model Priority/Routing Logic: The reordering of models (prioritizing Anthropic over OpenAI) and changing providers (introducing "antigravity") lacks documentation about intent, impact on selection behavior, and whether "antigravity" is a valid provider.
-
Configuration/Secret Management Complexity: Multiple secret injection points exist across the system (runtime shell scripts, environment variables, local files) rather than using Nix's age/sops integration, creating unnecessary complexity.
-
Tight System Coupling: Hardcoded localhost:8317 endpoints across all tools, tight coupling between model names and provider types, and binary location assumptions violate Nix principles and limit deployment flexibility.
-
Documentation Gaps: Lack of explanation about AMP's role in the system, factory's purpose (development/testing/production), and the model selection/fallback behavior makes the architecture difficult to understand and maintain.
Tip
Help
Slash Commands:
/review- Request a full code review/review latest- Review only changes since the last review/describe- Generate PR description. This will update the PR body or issue comment depending on your configuration/help- Get help with Mesa commands and configuration options
5 files reviewed | 0 comments | Edit Agent Settings • Read Docs
Add @sourcegraph/amp to package dependencies for AI proxy integration, configure ampcode in cliproxyapi service, and update custom models and providers configuration
Summary by cubic
Integrates Sourcegraph AMP into the AI proxy and enables ampcode routing. Updates model/provider mappings and removes the legacy amp-cli.
Dependencies
Configuration
Written for commit 54e45f9. Summary will update automatically on new commits.