fix(ci): update actions/cache to v4 in benchmarks workflow - #224
Conversation
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
Dependency limit exceeded — report not shown. This pull request scan exceeded the 10,000-dependency limit applied to this scan, so the results are incomplete and may be inaccurate. To avoid reporting false positives, Socket has not posted a report. Upgrade your plan to raise the dependency limit and get complete reports, or view the partial scan in the dashboard. Socket is always free for open source. If this is a non-commercial open source project, contact us to request a free Team account. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05ebf569c8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| /// Simulates the core agent loop pattern that forge_api orchestrates. | ||
| struct AgentLoop { | ||
| providers: Vec<MockProvider>, | ||
| max_iterations: usize, | ||
| tool_executor: Box<dyn Fn(&ToolCallFull) -> String + Send + Sync>, |
There was a problem hiding this comment.
The agent-loop tests invoke this test-only AgentLoop, which independently implements provider fallback, iteration limits, context updates, and tool execution instead of calling the production ForgeApp::chat/Orchestrator::run path. Consequently, regressions in the real orchestration can leave every purported end-to-end scenario green; use mocked production service traits to drive the actual orchestrator rather than maintaining a parallel implementation in an integration-test file.
AGENTS.md reference: AGENTS.md:L40-L42
Useful? React with 👍 / 👎.
| for (i, answer, iterations) in results { | ||
| assert_eq!(answer, format!("Response for conversation {i}")); | ||
| assert_eq!(iterations, 1); |
There was a problem hiding this comment.
When Tokio schedules these spawned tasks in any order other than their creation order, each task removes the next item from one shared FIFO queue, so conversation i is not guaranteed to receive response i; sorting the completed tuples afterward cannot restore that association, and this assertion fails nondeterministically under concurrent scheduling. Give each request an isolated provider/response or assert only order-independent properties.
Useful? React with 👍 / 👎.
| provider.push_response(MockResponse::text_with_usage("Turn 1", 100)).await; | ||
| provider.push_response(MockResponse::text_with_usage("Turn 2", 200)).await; | ||
| provider.push_response(MockResponse::text_with_usage("Turn 3", 150)).await; |
There was a problem hiding this comment.
Each queued response is plain text with no tool calls, so AgentLoop::run returns immediately after consuming only Turn 1; the 200- and 150-token responses remain unused, and the weak > 0 assertion passes without testing accumulation across turns. Exercise all three responses through the same context and compare the resulting total with the handwritten expected value of 450 tokens.
AGENTS.md reference: AGENTS.md:L18-L22
Useful? React with 👍 / 👎.
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| struct AgentLoop { | ||
| providers: Vec<MockProvider>, | ||
| max_iterations: usize, | ||
| tool_executor: Box<dyn Fn(&ToolCallFull) -> String + Send + Sync>, | ||
| } |
There was a problem hiding this comment.
Suggestion: AgentLoop reimplements orchestration instead of calling production ForgeApp::chat, so these tests can pass while hooks, persistence, retries, limits, and lifecycle behavior are broken. [incomplete implementation]
Assessment: 🟠 Major · 🔁 Occurrence: Often
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/forge_api/tests/e2e_mock_provider.rs
**Line:** 180:184
**Comment:**
*Incomplete Implementation: `AgentLoop` reimplements orchestration instead of calling production `ForgeApp::chat`, so these tests can pass while hooks, persistence, retries, limits, and lifecycle behavior are broken.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| if let Some(result) = stream.next().await { | ||
| match result { | ||
| Ok(msg) => return Ok(msg), | ||
| Err(e) => { |
There was a problem hiding this comment.
Suggestion: call_provider reads only the first stream item, so later content or tool-call chunks are discarded and streaming aggregation is never tested. [logic error]
Assessment: 🟠 Major · 🔁 Occurrence: Often
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/forge_api/tests/e2e_mock_provider.rs
**Line:** 267:270
**Comment:**
*Logic Error: `call_provider` reads only the first stream item, so later content or tool-call chunks are discarded and streaming aggregation is never tested.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| let args = call.arguments.parse().unwrap_or_default(); | ||
| let cmd = args.get("command").and_then(|v| v.as_str()).unwrap_or("echo 'no command'"); | ||
| format!("Mock shell output for: {cmd}") |
There was a problem hiding this comment.
Suggestion: Malformed tool arguments become harmless defaults and every execution is marked successful, so invalid-argument failures and retry or interruption behavior cannot be detected. [error handling]
Assessment: 🟠 Major · 🔁 Occurrence: Often
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/forge_api/tests/e2e_mock_provider.rs
**Line:** 295:297
**Comment:**
*Error Handling: Malformed tool arguments become harmless defaults and every execution is marked successful, so invalid-argument failures and retry or interruption behavior cannot be detected.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| let provider = MockProvider::new("concurrent"); | ||
| for i in 0..5 { | ||
| provider.push_response(MockResponse::text(format!("Response for conversation {i}"))).await; |
There was a problem hiding this comment.
Suggestion: Concurrent tasks share one response queue without request identity, so scheduling can give conversation i another conversation's response and make this test fail nondeterministically. [race condition]
Assessment: 🟠 Major · 🔁 Occurrence: Sometimes
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/forge_api/tests/e2e_mock_provider.rs
**Line:** 488:490
**Comment:**
*Race Condition: Concurrent tasks share one response queue without request identity, so scheduling can give conversation `i` another conversation's response and make this test fail nondeterministically.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| let provider = MockProvider::new("usage-accum"); | ||
| provider.push_response(MockResponse::text_with_usage("Turn 1", 100)).await; | ||
| provider.push_response(MockResponse::text_with_usage("Turn 2", 200)).await; |
There was a problem hiding this comment.
Suggestion: The usage test queues three responses, but the first text response ends AgentLoop::run; it never verifies accumulation across multiple turns. [possible bug]
Assessment: 🟠 Major · 🔁 Occurrence: Often
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/forge_api/tests/e2e_mock_provider.rs
**Line:** 632:634
**Comment:**
*Possible Bug: The usage test queues three responses, but the first text response ends `AgentLoop::run`; it never verifies accumulation across multiple turns.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
User description
GitHub deprecated actions/cache v1/v2 and auto-fails workflows using them. Update the SHA to actions/cache@v4.
CodeAnt-AI Description
Add end-to-end agent-loop coverage and restore benchmark caching
What Changed
Impact
✅ Fewer agent-loop regressions✅ Reliable provider fallback tests✅ Benchmark workflows continue using build caches💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.