fix(market): sort active offers by price - #4001
Conversation
Market offers were returned in database insertion order, so new offers always appeared at the bottom of the list regardless of their price. Add ORDER BY to the per-item active-offer query in IOMarket::getActiveOffers (itemId/tier overload, used for market browsing): buy offers highest price first, sell offers lowest price first, with the oldest offer breaking price ties. Also order own offers by creation time for consistency. The parameterless overload (used only by loadItemsPrice for max prices) is left unsorted to avoid unnecessary DB load. No schema change or migration is required: this is a read-only ordering change over columns that already exist.
📝 WalkthroughWalkthroughTwo SQL queries in the market I/O module now include ORDER BY clauses. ChangesMarket Query Result Ordering
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Infer (1.2.0)src/io/iomarket.cppsrc/io/iomarket.cpp:10:10: fatal error: 'io/iomarket.hpp' file not found ... [truncated 1058 characters] ... s/clang/install/lib/clang/18/include" 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 |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds deterministic ordering to market offer queries so results are returned sorted (by price/created for active offers, by created for own offers).
Changes:
- Sort active offers by
price(direction depends on buy vs sell) withcreatedas a tie-breaker. - Sort “own offers” by
createdascending.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces ordering to the market offer queries in src/io/iomarket.cpp to ensure active and own offers are returned in a sorted order. The feedback suggests refactoring the query construction to use fmt::format and the global g_database() helper instead of std::ostringstream and Database::getInstance() for improved performance, consistency, and readability, as well as using std::string_view for the price order variable.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/io/iomarket.cpp (1)
69-71: ⚡ Quick winAdd a focused regression check for market ordering semantics.
The SQL ordering changes look correct, but this C++ behavior change should be covered by a focused test (or the smallest practical query-level check) asserting: buy = highest price first, sell = lowest price first, tie-break = oldest
createdfirst, and own offers sorted bycreatedASC.As per coding guidelines, “For C++ changes, prefer focused tests or the smallest practical build/check that validates the touched code.”
Also applies to: 104-104
🤖 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 `@src/io/iomarket.cpp` around lines 69 - 71, Add a focused test that validates the market ordering semantics implemented around priceOrder and the built SQL in query: create test data in a transient/test DB with multiple offers for the same item/tier including different prices, same prices with different created timestamps, and offers owned by the same player; then assert for MARKETACTION_BUY the results return highest price first (price DESC) with ties resolved by created ASC, and for sell (non-BUY) lowest price first (price ASC) with ties resolved by created ASC and that a player's own offers are sorted by created ASC; the test should either execute the actual SQL string built by the code path that uses priceOrder and query or invoke the function that runs it so you verify real query ordering and not only string contents.Source: Coding guidelines
🤖 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 `@src/io/iomarket.cpp`:
- Around line 69-71: Add a focused test that validates the market ordering
semantics implemented around priceOrder and the built SQL in query: create test
data in a transient/test DB with multiple offers for the same item/tier
including different prices, same prices with different created timestamps, and
offers owned by the same player; then assert for MARKETACTION_BUY the results
return highest price first (price DESC) with ties resolved by created ASC, and
for sell (non-BUY) lowest price first (price ASC) with ties resolved by created
ASC and that a player's own offers are sorted by created ASC; the test should
either execute the actual SQL string built by the code path that uses priceOrder
and query or invoke the function that runs it so you verify real query ordering
and not only string contents.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 122f3ae4-27d7-43a4-8147-1662239c2319
📒 Files selected for processing (1)
src/io/iomarket.cpp
|
Description
Market offers were returned in database insertion order, so new offers always appeared at the bottom of the list regardless of their price.
Add ORDER BY to the per-item active-offer query in IOMarket::getActiveOffers (itemId/tier overload, used for market browsing): buy offers highest price first, sell offers lowest price first, with the oldest offer breaking price ties. Also order own offers by creation time for consistency. The parameterless overload (used only by loadItemsPrice for max prices) is left unsorted to avoid unnecessary DB load.
No schema change or migration is required: this is a read-only ordering change over columns that already exist.
List buy/purchase items by highest price and sell/sale items by lowest price.
Behaviour
Actual
Market offers are displayed in database insertion order. New offers always appear at the bottom of the list, regardless of whether they have a better price than existing ones.
Expected
Buy offers are sorted by highest price first, and sell offers by lowest price first. Ties are broken by the oldest creation time.
Fixes #issuenumber
Type of change
How Has This Been Tested
Tested manually using the game client to verify the visual ordering of the market board:
Test Configuration:
Checklist
Summary by CodeRabbit