Conversation
…eping calls Ports the 5 short-circuit categories from free-claude-code (quota-check mock, prefix detection, title-generation skip, suggestion-mode skip, filepath extraction mock) so Claude-Code-CLI internal bookkeeping requests are intercepted locally instead of burning free-tier quota. Returns a non-streaming Messages JSON body (matching the reference's response shape) rather than an SSE stream -- these CLI-internal calls are not sent with stream:true and expect a plain JSON response; the rest of this proxy is unconditionally SSE, so this is a deliberate exception.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a configurable short-circuit module for selected Anthropic-style message requests. Matching quota, command, title, suggestion, and filepath requests receive generated JSON responses; unmatched requests continue through existing upstream forwarding. ChangesShort-circuit request handling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant v1_messages_handler
participant try_short_circuit
participant UpstreamProxy
Client->>v1_messages_handler: JSON message request
v1_messages_handler->>try_short_circuit: Body and configured flags
alt Match
try_short_circuit-->>v1_messages_handler: Generated JSON response
v1_messages_handler-->>Client: Short-circuited response
else NoMatch
try_short_circuit-->>v1_messages_handler: NoMatch
v1_messages_handler->>UpstreamProxy: Existing proxy request
UpstreamProxy-->>Client: Upstream response
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
crates/flare-proxy/src/shortcircuit.rs (2)
145-196: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRedundant
<filepaths>check and duplicateto_lowercase()call.
"<filepaths>"is always a substring of"filepaths"'s match target once lowercased, socontent.to_lowercase().contains("<filepaths>")can never be true when the first check is false ("<filepaths>"itself contains"filepaths") — the second condition is dead. It also allocates a second lowercase copy ofcontentunnecessarily, on a path reached by every user message shaped likeCommand:/Output:.♻️ Proposed cleanup
- let user_has_filepaths = content.to_lowercase().contains("filepaths") - || content.to_lowercase().contains("<filepaths>"); + let content_lower = content.to_lowercase(); + let user_has_filepaths = content_lower.contains("filepaths");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/flare-proxy/src/shortcircuit.rs` around lines 145 - 196, In detect_filepath_extraction, compute content.to_lowercase() once and reuse it for the filepath detection. Remove the redundant "<filepaths>" condition, leaving the check for the "filepaths" substring while preserving the existing gating behavior.
507-552: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd dispatch-level tests for the remaining
try_short_circuitbranches.Only the "all disabled" and "quota enabled" cases exercise
try_short_circuitdirectly; the prefix/title/suggestion/filepath branches are only tested via their standalone detector functions, not through the actual dispatcher (config flag gating +build_messageshape together). Since this dispatcher gates quota-bypassing behavior, end-to-end coverage of each branch would catch future regressions where a flag check or return value is wired incorrectly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/flare-proxy/src/shortcircuit.rs` around lines 507 - 552, Add dispatch-level tests alongside test_short_circuit_all_disabled and test_short_circuit_quota_mock_enabled for the fast-prefix, title-generation, suggestion-mode, and filepath-extraction branches of try_short_circuit. Enable only the corresponding ShortCircuitConfig flag in each test, provide a body that triggers that detector, and assert the returned Match payload from build_message, including its distinguishing content and metadata.crates/flare-proxy/src/lib.rs (1)
57-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider caching
ShortCircuitConfiginAppStateinstead of reloading per request.
ProviderConfigis computed once inrouter()and stored inAppState, butShortCircuitConfig::from_env()is re-read from the environment on every request here. Not a functional bug (env vars are static at runtime), but it's an inconsistent pattern with the existing config-loading approach in this same handler.♻️ Proposed refactor
- let sc_config = shortcircuit::ShortCircuitConfig::from_env(); - match shortcircuit::try_short_circuit(&body, &sc_config) { + match shortcircuit::try_short_circuit(&body, &state.sc_config) {This requires adding the field to
AppStateand populating it inrouter():struct AppState { config: ProviderConfig, client: reqwest::Client, sc_config: shortcircuit::ShortCircuitConfig, }.with_state(AppState { config: ProviderConfig::from_env(), client, sc_config: shortcircuit::ShortCircuitConfig::from_env(), })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/flare-proxy/src/lib.rs` around lines 57 - 58, Cache ShortCircuitConfig in AppState alongside ProviderConfig and initialize it once in router() with ShortCircuitConfig::from_env(). Update the request handler’s shortcircuit::try_short_circuit call to use the state-held configuration instead of reloading the environment per request.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/flare-proxy/src/lib.rs`:
- Around line 57-58: Cache ShortCircuitConfig in AppState alongside
ProviderConfig and initialize it once in router() with
ShortCircuitConfig::from_env(). Update the request handler’s
shortcircuit::try_short_circuit call to use the state-held configuration instead
of reloading the environment per request.
In `@crates/flare-proxy/src/shortcircuit.rs`:
- Around line 145-196: In detect_filepath_extraction, compute
content.to_lowercase() once and reuse it for the filepath detection. Remove the
redundant "<filepaths>" condition, leaving the check for the "filepaths"
substring while preserving the existing gating behavior.
- Around line 507-552: Add dispatch-level tests alongside
test_short_circuit_all_disabled and test_short_circuit_quota_mock_enabled for
the fast-prefix, title-generation, suggestion-mode, and filepath-extraction
branches of try_short_circuit. Enable only the corresponding ShortCircuitConfig
flag in each test, provide a body that triggers that detector, and assert the
returned Match payload from build_message, including its distinguishing content
and metadata.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3d87bb52-b733-43ac-b4b2-2f50c47c9b6a
📒 Files selected for processing (2)
crates/flare-proxy/src/lib.rscrates/flare-proxy/src/shortcircuit.rs
- drop redundant <filepaths> substring check (always implied by filepaths) - cache ShortCircuitConfig in AppState instead of reading env per request - add try_short_circuit dispatch coverage for prefix/title/suggestion/filepath
1 participant
Summary
stream: truefor them — an SSE reply would have broken CLI parsing. Replaced the SSE builders with onebuild_message()JSON helper and switched the handler toaxum::Json(..).into_response().-D warningsfailure (Iterator::laston aDoubleEndedIterator).Test plan
cargo fmt --all -- --checkcargo clippy -p flare-proxy --all-features --all-targets -- -D warningscargo test -p flare-proxy --all-features(46 passed)cargo build --workspace --all-featuresSummary by CodeRabbit