Skip to content

DAH-2506 - Add browse-filter parity to the SDK and CLI - #97

Open
fortunelucky777 wants to merge 4 commits into
mainfrom
feat/2506-filter-parity
Open

DAH-2506 - Add browse-filter parity to the SDK and CLI#97
fortunelucky777 wants to merge 4 commits into
mainfrom
feat/2506-filter-parity

Conversation

@fortunelucky777

Copy link
Copy Markdown
Collaborator

Summary

Task Link

DAH-2506

Give the lium SDK and CLI the same node filters the web browse page gained in DAH-2506, and hold the two implementations to a shared fixture so they cannot drift apart.

Problem

The web browse page and the CLI answered the same question differently. lium ls could not filter on tier, reliability, uptime, VRAM, open ports or country at all, and its price filter compared against a different GPU count than the price it printed. A renter who found a node in the browser could not reproduce that result from the CLI.

There was also a live billing hazard in the selection path. lium up widens its search to splittable nodes, but the rent payload carried no gpu_count, and the backend defaults to the whole node when the field is absent (gpu_count = payload.gpu_count or len(available_gpus), priced as price_per_gpu * gpu_count). On a splittable 8-GPU node, lium up --count 1 would rent and bill all eight.

Solution

  • New lium/sdk/filters.py holds one predicate per dimension, mirroring browseFilterPredicates.ts in the frontend one-for-one.
  • Both sides are held to a byte-identical fixture. test/fixtures/filter_executors.json and the frontend's src/contexts/__fixtures__/filter-executors.json are the same file, and both test suites assert the same accepted-ID sets for the same inputs. A change on one side that is not mirrored fails the other side's tests.
  • Lium.up now sends gpu_count, the CLI threads --count into it, and the confirmation prompt states the split before the rental is created.

Changes

  • lium/sdk/filters.py (new) — display_gpu_count plus predicates for tier, reliability, price, uptime, VRAM per GPU, total VRAM, ports and country. _is_splittable_for_count moved here from client.py and re-exported there, so existing imports keep working.
  • ExecutorInfo gains reliability_score and uptime_in_minutes, plus vram_gb / vram_total_gb properties mirroring the frontend's getNodeVramMb. _dict_to_executor_info maps the two new fields — it drops unknown keys, so this is required, not cosmetic.
  • Lium.ls gains ten keyword arguments applied client-side; the docstring's "defaults to 8" claim is corrected.
  • lium ls gains nine flags with grouped help, a $/h column beside $/GPU·h, and available_gpu_count / reliability_score / uptime_in_minutes in --format json. A duplicate download sort key is removed — it shadowed the effective speed with a raw specs value.
  • lium up selects through a single widened Lium.ls() call instead of hand-rolled comparisons, and gains --min-cuda.
  • price_per_hour is now derived from available_gpu_count rather than the physical gpu_count, so the printed $/h agrees with what --max-price compares.
  • lium/cli/provider/_render.py — fixed a crash: keys.index(k) was called from inside keys.sort(), and list.sort() empties the list while it runs, so it raised ValueError. This broke 5 existing tests. Out of scope for the ticket, but CI now runs the full suite and would have landed red without it.
  • .github/workflows/ci.yml — the job runs pytest test/ instead of a single file. Note for whoever merges: the job is renamed from validate-binary-targets to test, so branch protection needs updating if the old name is a required check.
  • Version bumped to 0.0.28 in both pyproject.toml and lium/__about__.py.

Deployment Steps

Use GitHub Action. Publishing to PyPI is a manual workflow_dispatch as usual — this PR does not release.

Review Request

  • Just code review
  • QA – local test on reviewer side

Other PRs

Risks

  • Billing. The gpu_count fix changes what lium up actually rents. It is covered by tests asserting the exact rent payload, but it is the part of this PR worth the most review attention.
  • The parity contract only holds while the fixture stays identical on both sides. There is no CI job spanning the two repos — the guard is that both suites assert the same sets over the same bytes.
  • merge was used rather than rebase to bring main in, deliberately, so the DAH-2254 commits this branch stacks on are not rewritten. The merge conflicts were all additive: main added an enable_volume_encryption parameter exactly where this branch added gpu_count, and both survive.

Test Cases

Test Case 1 — Splitting bills the requested count

Actions lium up --count 1 against a splittable 8-GPU node.

Expected Output The rent payload carries gpu_count: 1 and the confirmation prompt states the split. Before this change the backend defaulted to all 8 GPUs and billed accordingly.

Test Case 2 — CLI and web agree

Actions Apply the same filter combination on the browse page and via lium ls flags.

Expected Output The same node set. This is asserted in CI on both sides against the shared fixture.

Test Case 3 — Printed price matches the price filter

Actions lium ls --max-price-total N on a partly-rented node.

Expected Output The $/h column never disagrees with the filter — both use the rentable count.

Automated: pytest test/ — 367 passed, including 30 new parity tests. The fixture was confirmed byte-identical to the frontend copy.

Checklist

  • Proper labels added (ready-review, require-qa, breaking-changes if applicable)
  • PR description is clear and complete
  • Risks are documented
  • Tests are described

- ExecutorInfo: add min_gpu_count_for_rental and available_gpu_count fields
- _dict_to_executor_info: wire both new fields from API dict
- _is_splittable_for_count: module-level predicate helper (parity with FE)
- Lium.ls(): add widen_for_splitting kwarg (default False per D2a)
- decorators.py: clarifying comment — @machine stays strict
- cli/ls/actions.py: pass widen_for_splitting=True (CLI opt-in only)
- cli/ls/command.py: update --count help string
- cli/ls/display.py: append '↯ from N' in _cfg, add field to compact_executor
- test/fixtures/splittable_executors.json: byte-identical twin of frontend fixture
- test/test_client_ls_widen.py: 5 tests covering widen truth table, default-off,
  compact_executor field round-trip, and _cfg '↯ from N' marker
Give `lium ls` the same filter dimensions the browse page has, and route
`lium up` through the same call so the two commands stop offering disjoint
candidate sets.

- SDK: ExecutorInfo gains reliability_score, uptime_in_minutes and the
  vram_gb / vram_total_gb properties; Lium.ls gains tier, min_reliability
  (+include_unscored), min_uptime_minutes, min_vram_gb, min_vram_total_gb,
  min_ports, countries, max_price_total and max_price_per_gpu.
- New lium/sdk/filters.py holds the predicates. Max price (both modes), tier
  and min reliability are held to a fixture mirrored from the web repo, so a
  predicate that drifts on one side fails on the other.
- max_price_total prices the split --count would actually rent, not the whole
  node, so a $2.50/GPU splittable node survives a $5 budget.
- Lium.up gains gpu_count and sends it in the rent payload. Without it the API
  rents every available GPU, so widening `up` to splittable nodes would have
  handed over — and billed — all eight GPUs to someone who asked for one. The
  confirmation prompt now quotes the split, not the node.
- CLI: nine new `ls` flags with grouped help, a $/h column next to $/GPU·h,
  available_gpu_count / reliability_score / uptime_in_minutes in --format json,
  and a fix for the duplicate "download" sort key that shadowed the effective
  download speed.
- `lium up` now selects via one Lium.ls() call (widened for splitting) instead
  of comparing gpu_count by hand, and reuses that result instead of re-fetching.
- CI runs the whole test suite instead of one file. That surfaced a real
  pre-existing crash in the provider generic table renderer: it called
  keys.index() from inside keys.sort(), and list.sort() empties the list while
  it runs. list.sort() is stable, so the tie-breaker was redundant anyway.
@fortunelucky777
fortunelucky777 force-pushed the feat/2506-filter-parity branch from 197bc3c to 39fd7fa Compare August 5, 2026 13:51
0.0.29 is already published to PyPI, so the filter-parity work in this PR
needs its own version to ship. Both files must stay identical — release.yml
validates the dispatched version against each of them before building.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants