feat(orders): competitiveness report for your open orders - #29
Conversation
`lofty orders competitiveness` (alias `comp`) reports, per property and side, how far your resting orders are from a fill and from the competition: - Buy: gap from your highest bid up to the lowest ask not yours (how far a seller must come down, % of that ask), and between the book's highest bid and the highest bid not yours. - Sell: gap from the highest bid not yours up to your lowest ask, and between the book's lowest ask and the lowest ask not yours. - `leadUsd` gives your distance ahead of (or behind) the best competing order. The public book is aggregated by price without order ids, so your share of a level is your open quantity at that price; the legacy per-order envelope is matched by order id. Partially filled orders count at their remaining quantity. Read-only; `--json` emits `orders-competitiveness/v1`. Also fixes `orders list --property-id`: it used the `?propertyId=` query, which reports a partially filled order's original quantity. It now filters the `all=true` list client-side, which reports what remains (the same approach `quote` already takes).
piekstra-dev
left a comment
There was a problem hiding this comment.
Automated PR Review
Reviewed commit: 560182b5f213
Profile: reviewer - Posting as: piekstra-dev
Summary
| Reviewer | Findings |
|---|---|
| rust:implementation-tests | 0 |
| policies:conventions | 0 |
| architecture:solid | 3 |
| documentation:docs | 0 |
| rust:implementation | 1 |
architecture:solid (3 findings)
Major - src/commands/competitiveness.rs:41
U-S1 / U-O1:
side()is a second, divergent implementation of the domain rule "the book with my own resting size removed".quote::market_from(src/commands/quote.rs:438-490) already computes exactly this; it backs the never-cross-the-market rail. The two implementations now disagree on mechanics. Price matching iscents()rounding here andabs() < 0.005there. The residual threshold is> 1e-9here and> 0.0there. On the legacy envelope, this module matches by orderid/orderId, while quote subtracts by price from the first matching entry. The PR's design note relies on that id matching. The result is thatorders competitivenessandquotecan report different values for "best ask not yours" on the same book, and the rail is the one that moves money. This diff also adds the 6th copy of the<path>/{bids,asks}vs<path>/{buyOrders,sellOrders}envelope fallback; the others are properties.rs:112, quote.rs:442, rewards.rs:238 and :374, and orders.rs:348. A third envelope shape would mean editing 6 sites. Suggested fix: add one pure module (for examplesrc/commands/book.rs) exposingfn levels(book: &Value, side: BookSide) -> Vec<Level { cents: i64, qty: f64, id: Option<String> }>andfn without_mine(levels: &[Level], mine: &[&Value]) -> Vec<Level>. Carry the id-aware attribution over from this PR. Have bothcompetitiveness::sideandquote::market_fromconsume it, and move the fixture-driven tests that live here onto it. Migrating rewards and properties can follow separately. If the two consumers are meant to differ, document why at both sites.
Minor - src/commands/orders.rs:117
U-S1: the rule "open orders come from the
all=truelist, filtered client-side tostatus == active(and optionally to one property), because?propertyId=reports original quantity after a partial fill" is now hand-copied in orders.rs:86-93, orders.rs:117-128, quote.rs:289-300, rewards.rs:168-174 and account.rs:178/207. This PR also writes the rule into AGENTS.md as prose. A manifest rule kept in 5+ copies by convention only takes one new command that uses?propertyId=to break it. Suggested fix: addfn open_orders(client: &LoftyClient, property_id: Option<&str>) -> Result<Vec<Value>, CliError>next toretain_property, or incommands/mod.rs, and call it fromCompetitivenessandquote::fetch_state. Point the AGENTS.md sentence at that function so the rule has one owner.
Minor - src/commands/orders.rs:250
U-T1:
orders list --property-idchanged behavior. It used to be the server-side?propertyId=query and is nowall=trueplus client-sideretain_property. Nothing in the diff tests it: the surface tests only check that--property-idparses.retain_propertyis pure and private, and other top-level fields are documented as kept unchanged, so it is cheap to pin. Suggested fix: add a unit test in the existingorders.rsmod teststhat runsretain_propertyon a two-property<path>payload. It should assert that only the target property's orders remain and that a sibling top-level field is untouched. Reusetests/fixtures/competitiveness/orders-open.json, which already holds two properties.
rust:implementation (1 finding)
Minor - src/commands/orders.rs:8
market_view'ssideclosure in orders.rs re-implements the same orderbook-level lookup (<path>/{bids,asks}falling back to the legacy<path>/{buyOrders,sellOrders}envelope) thatcompetitiveness::sidealready implements independently. A third book shape or a fix to the fallback logic now has to be made in two places, and they have already drifted once (competitiveness::side filtersquantity <= 0per level; market_view::side does not, since it only tracks min/max price). Factor the shared "find the levels array for this side" step into one function both call.
Reviewer Coverage
rust:implementation-tests— complete (broad); inspected 5 assigned files (13 inspected across reviewers):src/commands/competitiveness.rs,src/commands/mod.rs,src/commands/orders.rs,tests/cli_surface.rs,tests/fixture_shapes.rs; skipped: none; constraints: nonepolicies:conventions— complete (broad); inspected 5 assigned files (13 inspected across reviewers):AGENTS.md,README.md,src/commands/competitiveness.rs,src/commands/mod.rs,src/commands/orders.rs; skipped: none; constraints: Base-branch .codereview/agents/ guidance was not present for this review (dossier reports it missing). No ../cli-common or ../.github sibling convenience copies present in the workbench; relied on this repo's AGENTS.md as the source of truth for conventions.architecture:solid— complete (broad); inspected 7 assigned files (13 inspected across reviewers):src/commands/competitiveness.rs,src/commands/orders.rs,tests/fixtures/competitiveness/orderbook-alone-on-top.json,tests/fixtures/competitiveness/orderbook-bids-only.json,tests/fixtures/competitiveness/orderbook-legacy-ids.json,tests/fixtures/competitiveness/orderbook-shared-top.json,tests/fixtures/competitiveness/orders-open.json; skipped: none; constraints: AGENTS.md was read as the project manifest. It contains no rule IDs, so findings cite the universal U-* rules. git and cargo commands required interactive approval in this environment, so the base-vs-head diff was reconstructed from head files plus the change map, andmake verify/cargo testwere not run; build and test status is taken from the PR description only.documentation:docs— complete (broad); inspected 3 assigned files (13 inspected across reviewers):AGENTS.md,README.md,tests/fixtures/README.md; skipped: none; constraints: nonerust:implementation— complete (broad); inspected 3 assigned files (13 inspected across reviewers):src/commands/competitiveness.rs,src/commands/mod.rs,src/commands/orders.rs; skipped: none; constraints: none
Inspected files (13)
AGENTS.mdREADME.mdsrc/commands/competitiveness.rssrc/commands/mod.rssrc/commands/orders.rstests/cli_surface.rstests/fixture_shapes.rstests/fixtures/README.mdtests/fixtures/competitiveness/orderbook-alone-on-top.jsontests/fixtures/competitiveness/orderbook-bids-only.jsontests/fixtures/competitiveness/orderbook-legacy-ids.jsontests/fixtures/competitiveness/orderbook-shared-top.jsontests/fixtures/competitiveness/orders-open.json
0 PR discussion threads considered. 0 summarized; 0 resolved.
Completed in 4m 21s | $2.55 | claude-sonnet-5, claude-opus-5-5 | cr 0.10.314
| Field | Value |
|---|---|
| Model | claude-sonnet-5, claude-opus-5-5 |
| Reviewers | rust:implementation-tests, policies:conventions, architecture:solid, documentation:docs, rust:implementation |
| Engine | claude_cli · claude-sonnet-5, claude-opus-5-5 |
| Reviewed by | cr · piekstra-dev |
| Duration | 4m 21s wall · 7m 51s compute |
| Cost | $2.55 |
| Tokens | 122 in / 36.8k out |
Per-workstream usage
orchestrator-selection— claude-sonnet-5- In: 6
- Out: 3.5k
- Cache read: 138.6k
- Cache create: 18.8k
- Cost: $0.14
- Duration: 41s
rust:implementation-tests— claude-sonnet-5- In: 20
- Out: 7.6k
- Cache read: 670.1k
- Cache create: 42.5k
- Cost: $0.38
- Duration: 1m 42s
policies:conventions— claude-sonnet-5- In: 22
- Out: 3.1k
- Cache read: 620.2k
- Cache create: 23.2k
- Cost: $0.25
- Duration: 37s
architecture:solid— claude-opus-5-5- In: 28
- Out: 8.5k
- Cache read: 936.0k
- Cache create: 56.1k
- Cost: $0.81
- Duration: 1m 28s
documentation:docs— claude-sonnet-5- In: 28
- Out: 4.9k
- Cache read: 869.9k
- Cache create: 36.4k
- Cost: $0.37
- Duration: 1m 23s
rust:implementation— claude-sonnet-5- In: 12
- Out: 8.6k
- Cache read: 354.9k
- Cache create: 42.2k
- Cost: $0.33
- Duration: 1m 36s
orchestrator-rollup— claude-sonnet-5- In: 6
- Out: 592
- Cache read: 201.4k
- Cache create: 6.3k
- Cost: $0.28
- Duration: 22s
Review follow-up. - New `commands/book.rs` owns order-book parsing for both envelopes and the "book without my own orders" rule (id match on the legacy per-order envelope, quantity subtraction by cent on the aggregated one). The quote never-cross rail (`quote::market_from`), `orders competitiveness`, `orders create`'s market check, and `rewards` all read through it, so the rail and the report can no longer disagree about the market. - `orders::open_orders` is the single owner of "open orders come from the all=true list, filtered client-side"; competitiveness, quote, rewards eligibility, and account coverage call it. - Unit test pinning `retain_property` (`orders list --property-id`). Behaviour note: book levels with no positive quantity are no longer counted as resting liquidity anywhere (previously some readers included a zero-quantity level's price).
piekstra-dev
left a comment
There was a problem hiding this comment.
Automated PR Review
Reviewed commit: 9c77cc48e052
Profile: reviewer - Posting as: piekstra-dev
Summary
| Reviewer | Findings |
|---|---|
| rust:implementation-tests | 0 |
| policies:conventions | 0 |
| architecture:solid | 1 |
| documentation:docs | 0 |
| rust:implementation | 0 |
architecture:solid (1 finding)
Minor - src/commands/orders.rs:362
U-T1: the
orders createmarket check changes behavior here. The oldsideclosure counted any level that had a numericprice.book::levelsnow also requires a numericquantitygreater than 1e-9 (book.rs:67 and :73). A zero-quantity level no longer counts; the PR describes that as intended. A level with noquantityfield is now silently dropped as well, and the PR does not mention that case. For this check, dropping a level fails open: an ask the book does report becomes invisible to the 'AT OR ABOVE the best ask' objection. AGENTS.md says changes to money-moving checks are safety-critical, but no test pins either case. Thebook()helper in orders.rs tests always sets quantity 1, and book.rs tests use only positive-quantity fixtures. Risk today is low: tests/fixture_shapes.rs shows live levels carryquantity. The verdict would change to no finding if a test pinned both cases. Suggested fix: add a test to theorders.rsmod testswheremarket_viewgets a book whose best ask has quantity 0 and whose next ask is real, then assert the objection quotes the real ask. Decide explicitly how a level withoutquantityshould be treated. Failing closed means counting it as resting for best-price purposes, for exampleqty.unwrap_or(f64::INFINITY)or a separateprices()reader. Pin whichever choice is made in a book.rs test.
Reviewer Coverage
rust:implementation-tests— complete (constrained); inspected 9 assigned files (17 inspected across reviewers):src/commands/account.rs,src/commands/book.rs,src/commands/competitiveness.rs,src/commands/mod.rs,src/commands/orders.rs,src/commands/quote.rs,src/commands/rewards.rs,tests/cli_surface.rs,tests/fixture_shapes.rs; skipped: none; constraints: nonepolicies:conventions— complete (constrained); inspected 5 assigned files (17 inspected across reviewers):AGENTS.md,README.md,src/commands/competitiveness.rs,src/commands/mod.rs,src/commands/orders.rs; skipped: none; constraints: Base-branch .codereview/agents/ guidance was not present for this review (dossier reports it missing). No ../cli-common or ../.github sibling convenience copies present in the workbench; relied on this repo's AGENTS.md as the source of truth for conventions. src/commands/book.rs, account.rs, quote.rs, and rewards.rs are not in this reviewer's assigned file list; verified their consumption of the shared book module only to confirm the AGENTS.md claims in the assigned files, not reviewed line-by-line.architecture:solid— complete (constrained); inspected 7 assigned files (17 inspected across reviewers):src/commands/competitiveness.rs,src/commands/orders.rs,tests/fixtures/competitiveness/orderbook-alone-on-top.json,tests/fixtures/competitiveness/orderbook-bids-only.json,tests/fixtures/competitiveness/orderbook-legacy-ids.json,tests/fixtures/competitiveness/orderbook-shared-top.json,tests/fixtures/competitiveness/orders-open.json; skipped: none; constraints: All four earlier threads are resolved at this head: book.rs owns envelope parsing and own-size removal, open_orders owns the fetch rule, market_view uses book::levels, and retain_property has a unit test (orders.rs:546). git and cargo commands required interactive approval in this environment, somake verify/cargo testwere not run and the diff was judged from head files plus the change map; build and test status is taken from the PR description. src/commands/book.rs, quote.rs, rewards.rs and account.rs were read for context only; they are outside this reviewer's assigned files, so findings anchor on orders.rs.documentation:docs— complete (constrained); inspected 3 assigned files (17 inspected across reviewers):AGENTS.md,README.md,tests/fixtures/README.md; skipped: none; constraints: nonerust:implementation— complete (constrained); inspected 3 assigned files (17 inspected across reviewers):src/commands/competitiveness.rs,src/commands/mod.rs,src/commands/orders.rs; skipped: none; constraints: book.rs is the new home of the previously-duplicated orderbook parsing logic but is not in this reviewer's assigned file list, so it was read for context only; no findings were filed against it.
Inspected files (17)
AGENTS.mdREADME.mdsrc/commands/account.rssrc/commands/book.rssrc/commands/competitiveness.rssrc/commands/mod.rssrc/commands/orders.rssrc/commands/quote.rssrc/commands/rewards.rstests/cli_surface.rstests/fixture_shapes.rstests/fixtures/README.mdtests/fixtures/competitiveness/orderbook-alone-on-top.jsontests/fixtures/competitiveness/orderbook-bids-only.jsontests/fixtures/competitiveness/orderbook-legacy-ids.jsontests/fixtures/competitiveness/orderbook-shared-top.jsontests/fixtures/competitiveness/orders-open.json
0 PR discussion threads considered. 0 summarized; 0 resolved.
Completed in 1m 26s | $4.37 | claude-sonnet-5, claude-opus-5-5 | cr 0.10.314
| Field | Value |
|---|---|
| Model | claude-sonnet-5, claude-opus-5-5 |
| Reviewers | rust:implementation-tests, policies:conventions, architecture:solid, documentation:docs, rust:implementation |
| Engine | claude_cli · claude-sonnet-5, claude-opus-5-5 |
| Reviewed by | cr · piekstra-dev |
| Duration | 1m 26s wall · 4m 20s compute |
| Cost | $4.37 |
| Tokens | 94 in / 18.4k out |
Per-workstream usage
rust:implementation-tests— claude-sonnet-5- In: 22
- Out: 3.7k
- Cache read: 1.2M
- Cache create: 44.4k
- Cost: $0.84
- Duration: 1m 02s
policies:conventions— claude-sonnet-5- In: 20
- Out: 2.6k
- Cache read: 758.2k
- Cache create: 13.7k
- Cost: $0.48
- Duration: 29s
architecture:solid— claude-opus-5-5- In: 14
- Out: 5.2k
- Cache read: 789.7k
- Cache create: 40.8k
- Cost: $1.39
- Duration: 55s
documentation:docs— claude-sonnet-5- In: 20
- Out: 3.4k
- Cache read: 935.4k
- Cache create: 21.8k
- Cost: $0.68
- Duration: 1m 04s
rust:implementation— claude-sonnet-5- In: 12
- Out: 3.1k
- Cache read: 599.9k
- Cache create: 38.5k
- Cost: $0.63
- Duration: 37s
orchestrator-rollup— claude-sonnet-5- In: 6
- Out: 419
- Cache read: 219.2k
- Cache create: 5.0k
- Cost: $0.35
- Duration: 10s
Review follow-up. `book::levels` skipped a level whose quantity field was absent, which would hide a reported ask from the orders-create crossing check. Such a level is now kept with an unknown (infinite) size, so its price still counts and subtracting your own size can never remove it; a level reporting quantity 0 is still treated as empty. Reward scoring skips unknown-size levels. Both cases are pinned in book.rs and in the orders create market-check tests.
piekstra-dev
left a comment
There was a problem hiding this comment.
Automated PR Review
Reviewed commit: 92660a442646
Profile: reviewer - Posting as: piekstra-dev
Summary
| Reviewer | Findings |
|---|---|
| rust:implementation-tests | 0 |
| policies:conventions | 0 |
| architecture:solid | 0 |
| documentation:docs | 0 |
| rust:implementation | 0 |
Reviewer Coverage
rust:implementation-tests— complete (constrained); inspected 9 assigned files (18 inspected across reviewers):src/commands/account.rs,src/commands/book.rs,src/commands/competitiveness.rs,src/commands/mod.rs,src/commands/orders.rs,src/commands/quote.rs,src/commands/rewards.rs,tests/cli_surface.rs,tests/fixture_shapes.rs; skipped: none; constraints: nonepolicies:conventions— complete (constrained); inspected 5 assigned files (18 inspected across reviewers):AGENTS.md,README.md,src/commands/competitiveness.rs,src/commands/mod.rs,src/commands/orders.rs; skipped: none; constraints: Base-branch .codereview/agents/ guidance was not present for this review (dossier reports it missing). No ../cli-common or ../.github sibling convenience copies present in the workbench; relied on this repo's AGENTS.md as the source of truth for conventions. src/commands/book.rs and rewards.rs are not in this reviewer's assigned file list; checked only to confirm the fail-closed fix and its test coverage, not reviewed line-by-line.architecture:solid— complete (constrained); inspected 8 assigned files (18 inspected across reviewers):src/commands/competitiveness.rs,src/commands/orders.rs,tests/fixtures/book/orderbook-zero-and-missing-quantity.json,tests/fixtures/competitiveness/orderbook-alone-on-top.json,tests/fixtures/competitiveness/orderbook-bids-only.json,tests/fixtures/competitiveness/orderbook-legacy-ids.json,tests/fixtures/competitiveness/orderbook-shared-top.json,tests/fixtures/competitiveness/orders-open.json; skipped: none; constraints: All five earlier threads are resolved at this head. The orders.rs:362 thread is closed by book.rs:72-75 (no quantity field means infinite size, failing closed), the orders.rs:577 market_view test, and the book.rs:222 test, both over the new fixture. Not reported because book.rs is outside the assigned files: a level with a non-numeric quantity (for example a string) is still dropped, which fails open. Using an infinite quantity as an 'unknown size' marker is enforced only by the book.rs:55-62 doc and an is_finite() check in rewards.rs:353. git and cargo commands required interactive approval in this environment, somake verify/cargo testwere not run; build and test status is taken from the PR description.documentation:docs— complete (constrained); inspected 3 assigned files (18 inspected across reviewers):AGENTS.md,README.md,tests/fixtures/README.md; skipped: none; constraints: nonerust:implementation— complete (constrained); inspected 3 assigned files (18 inspected across reviewers):src/commands/competitiveness.rs,src/commands/mod.rs,src/commands/orders.rs; skipped: none; constraints: book.rs is not in this reviewer's assigned file list and was read for context only; no findings were filed against it.
Inspected files (18)
AGENTS.mdREADME.mdsrc/commands/account.rssrc/commands/book.rssrc/commands/competitiveness.rssrc/commands/mod.rssrc/commands/orders.rssrc/commands/quote.rssrc/commands/rewards.rstests/cli_surface.rstests/fixture_shapes.rstests/fixtures/README.mdtests/fixtures/book/orderbook-zero-and-missing-quantity.jsontests/fixtures/competitiveness/orderbook-alone-on-top.jsontests/fixtures/competitiveness/orderbook-bids-only.jsontests/fixtures/competitiveness/orderbook-legacy-ids.jsontests/fixtures/competitiveness/orderbook-shared-top.jsontests/fixtures/competitiveness/orders-open.json
0 PR discussion threads considered. 0 summarized; 0 resolved.
Completed in 1m 24s | $6.06 | claude-sonnet-5, claude-opus-5-5 | cr 0.10.314
| Field | Value |
|---|---|
| Model | claude-sonnet-5, claude-opus-5-5 |
| Reviewers | rust:implementation-tests, policies:conventions, architecture:solid, documentation:docs, rust:implementation |
| Engine | claude_cli · claude-sonnet-5, claude-opus-5-5 |
| Reviewed by | cr · piekstra-dev |
| Duration | 1m 24s wall · 3m 31s compute |
| Cost | $6.06 |
| Tokens | 80 in / 14.3k out |
Per-workstream usage
rust:implementation-tests— claude-sonnet-5- In: 20
- Out: 4.2k
- Cache read: 1.5M
- Cache create: 29.3k
- Cost: $1.29
- Duration: 1m 03s
policies:conventions— claude-sonnet-5- In: 14
- Out: 1.6k
- Cache read: 621.1k
- Cache create: 13.7k
- Cost: $0.67
- Duration: 19s
architecture:solid— claude-opus-5-5- In: 10
- Out: 3.4k
- Cache read: 699.5k
- Cache create: 18.4k
- Cost: $1.75
- Duration: 35s
documentation:docs— claude-sonnet-5- In: 22
- Out: 3.2k
- Cache read: 1.3M
- Cache create: 22.2k
- Cost: $1.05
- Duration: 1m 03s
rust:implementation— claude-sonnet-5- In: 8
- Out: 1.6k
- Cache read: 540.8k
- Cache create: 30.5k
- Cost: $0.88
- Duration: 20s
orchestrator-rollup— claude-sonnet-5- In: 6
- Out: 335
- Cache read: 233.4k
- Cache create: 4.2k
- Cost: $0.41
- Duration: 9s
What
New read-only command
lofty orders competitiveness [--property-id ID](aliascomp;--json→orders-competitiveness/v1). For each property you have open orders on, per side:Plus
leadUsd(signed distance ahead of the best competing order) andatTop.Design choices
bookblock./properties/{id}/orderbookis aggregated by price with no order ids. Your share of a level is your open quantity at that price, and the remainder belongs to someone else. A shared top level therefore has a competition gap of 0. The legacy per-order envelope (orderBook.buyOrders) is matched by order id.status: active. A partially filled order stays active at its remaining quantity in theall=truelist.Also fixed:
orders list --property-idorders list --property-idused the?propertyId=query, which reports a partially filled order's original quantity. Theall=truelist and the single-order GET report what remains (quotealready works around this). The command now filters theall=truelist client-side, and the response shape is unchanged. The new command uses the same path. Live check: the order set for one property is identical under both queries (no partial fills were open to compare quantities).Shared book and open-order rules
src/commands/book.rsnow owns order-book parsing for both envelopes and the "book without my own orders" rule. Thequotenever-cross rail,orders create's market check,rewards, and this command all read through it, so the rail and the report cannot disagree. Book levels with no positive quantity no longer count as resting liquidity anywhere.orders::open_ordersis the only place that fetches open orders (all=true, filtered client-side).quote,rewards eligibility,account coverage, and this command use it.Tests
src/commands/book.rs(both envelopes, quantity subtraction, id matching, side isolation, empty side), 1 forretain_property, and 8 insrc/commands/competitiveness.rsover synthetic fixtures intests/fixtures/competitiveness/: shared top level, alone on top, undercut ask (negative lead), legacy id envelope, empty opposite side, one-sided orders, grouping by property.orders --helplists it andorders comp --helprenders.quoteandrewardstests pass unchanged on the shared module.make verifygreen locally. Live read-only runs oforders comp,account coverage, andorders list --property-idexit 0.rewards eligibilityexits 4 (no open orders), the same result the released binary gives. Live read-only run: exit 0,orders-competitiveness/v1emitted (the account had no open orders at the time, so the per-property path was covered by fixtures). The live order book shape (orderbook.{bids,asks}, fractional quantities) matches what the parser reads.No orders are placed or cancelled.