Repository navigation
feat: distinguish failed/partial TWAP orders from expired - #203
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
Disabled knowledge base sources:
📝 WalkthroughWalkthroughAdds two TWAP statuses ('failed', 'partiallyFilled'), updates types/schema across client and server, refactors TWAP status derivation into exported helper functions, updates UI/status config, and adds fixtures and extensive tests for status and filled-part computation. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AgenticServer
participant CoW_API
participant OrderStore
Client->>AgenticServer: getTwapOrders(request with status filter)
AgenticServer->>OrderStore: fetch active TWAP summaries
AgenticServer->>CoW_API: fetch CoW orders for TWAP parts
CoW_API-->>AgenticServer: cow orders (or failure)
AgenticServer->>AgenticServer: getTwapFilledPartCount(order, cowOrders)
AgenticServer->>AgenticServer: deriveTwapStatus(order, cowOrders, isActive, now, cowApiFailed)
AgenticServer-->>Client: registry orders with derived status / numParts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
premiumjibles
force-pushed
the
feat/twap-failed-status
branch
from
March 9, 2026 03:06
1b6707e to
3845225
Compare
premiumjibles
marked this pull request as ready for review
March 9, 2026 03:25
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/agentic-server/src/tools/twap/getTwapOrders.ts (1)
81-102:⚠️ Potential issue | 🟠 MajorTighten child-order matching before using this count for terminal status.
On Lines 88-99,
filledCountis inferred fromtotalSell / numPartsplus pair/time-window matching. That can miscount in at least two cases: a TWAP whose total sell amount is not evenly divisible bynumPartswill drop the remainder and undercount one part, and two overlapping TWAPs on the same Safe with the same pair/part size can both match the same CoW child orders. Either case will surface the wrongfailed/partiallyFilled/fulfilledstatus.Please anchor the match to a parent-order identifier or metadata field if one exists, rather than relying on exact per-part amount plus timestamps alone.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/agentic-server/src/tools/twap/getTwapOrders.ts` around lines 81 - 102, getTwapFilledPartCount currently matches child orders only by token pair, exact per-part amount (expectedPartAmount) and timestamps which can undercount when totalSell % numParts != 0 or mis-attribute child orders across overlapping TWAPs; update getTwapFilledPartCount to first try anchoring child orders to a parent identifier/metadata (check co.parentOrderId, co.metadata?.parentOrderId, co.twapOrderId or similar) and match it to order.id or order.parentId, and only fall back to amount/timestamp heuristics if no parent id exists; additionally, when falling back avoid strict equality to expectedPartAmount—compute filled parts by summing toBigInt(co.executedSellAmount) for matching child orders and deriving count = min(numParts, floor(sumExecuted / expectedPartAmount) + (sumExecuted % expectedPartAmount ? 1 : 0)) so remainders count as a filled part rather than being dropped (use variables order, expectedPartAmount, cowOrders, matchingFilledParts and return that adjusted count).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/agentic-server/src/tools/twap/getTwapOrders.ts`:
- Around line 81-102: getTwapFilledPartCount currently matches child orders only
by token pair, exact per-part amount (expectedPartAmount) and timestamps which
can undercount when totalSell % numParts != 0 or mis-attribute child orders
across overlapping TWAPs; update getTwapFilledPartCount to first try anchoring
child orders to a parent identifier/metadata (check co.parentOrderId,
co.metadata?.parentOrderId, co.twapOrderId or similar) and match it to order.id
or order.parentId, and only fall back to amount/timestamp heuristics if no
parent id exists; additionally, when falling back avoid strict equality to
expectedPartAmount—compute filled parts by summing
toBigInt(co.executedSellAmount) for matching child orders and deriving count =
min(numParts, floor(sumExecuted / expectedPartAmount) + (sumExecuted %
expectedPartAmount ? 1 : 0)) so remainders count as a filled part rather than
being dropped (use variables order, expectedPartAmount, cowOrders,
matchingFilledParts and return that adjusted count).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5c341aef-83fe-4bfb-8d2b-ded7e9aea096
📒 Files selected for processing (9)
.tickets/sa-ppkj.mdapps/agentic-chat/src/components/tools/GetTwapOrdersUI.tsxapps/agentic-chat/src/stores/orderStore.tsapps/agentic-server/src/lib/cow/types.tsapps/agentic-server/src/routes/chat.tsapps/agentic-server/src/tools/twap/__tests__/fixtures.tsapps/agentic-server/src/tools/twap/__tests__/getTwapOrders.test.tsapps/agentic-server/src/tools/twap/getTwapOrders.tsapps/agentic-server/src/utils/walletContextSimple.ts
TWAP orders that expire now show granular end states based on fill count: - "failed" (0 fills) - solvers ignored all sub-orders - "partiallyFilled" (some fills) - incomplete execution - "fulfilled" (all fills) - unchanged behavior Adds UI badges, system prompt guidance, and type updates across the stack. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
numParts was being stripped by Zod validation, causing all expired TWAPs to show as 'expired' instead of 'failed'/'partiallyFilled'. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
premiumjibles
force-pushed
the
feat/twap-failed-status
branch
from
March 9, 2026 03:33
2bfd4f3 to
e7c8f28
Compare
This branch was successfully deployed
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
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
isTwapFulfilled→getTwapFilledPartCountto return fill count instead of booleanfulfilled(all parts filled),partiallyFilled(some parts filled),failed(zero parts filled)failedandpartiallyFilledto type unions across server and chat appTest plan
bunx tsc --noEmit— type-check passesfailedandpartiallyFilledstatuses works🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests