feat(rules): list --sort active-first - #270
Conversation
Forwards `ruleset_list(sort='active_first')` — the hunt page's order, rulesets with a running live hunt first (as recorded by the server's live-hunt link, the same one Livescan Id renders from), newest first within each block. Server-side like the filters: the list is keyset-paginated, so a local sort would only ever reorder one page. The option is a closed click.Choice (hyphenated CLI spelling, underscored server token) and is forwarded only when given, so the unsorted default request is unchanged. The autospec tests double as the signature check against the installed SDK, alone and combined with the filters.
…ort=) The floor names the SDK version that adds the keyword `rules list --sort` forwards (specs/05 §Current floor follows the pin). Mergeable once the SDK's develop declares 4.5.0; releasable once that version is on PyPI — the SDK releases first. CI resolves the SDK from source by branch name, falling back to develop, so the paired SDK branch must carry the identical name.
…n Live Hunt Id The help text said the order came from "the same one Live Hunt Id renders from". The server ranks on the raw link and renders the id under a stricter predicate, so a legacy row whose hunt was stopped without clearing the link leads the list with an empty Live Hunt Id. Someone reading the list top-down for what is running would have stopped at that row.
…y Live Hunt Id `rules list` walks every page and exposes no limit, which makes it exactly the consumer the SDK puts the dedupe obligation on. Under --sort active-first the ordering key is the live-hunt link, so a ruleset whose hunt stops between two page fetches drops below the cursor and the server serves it again: the run printed it twice and any script counting the output double-counted it. Rows are now emitted at most once per run. The dedupe is unconditional — the id is unique under either order and one set of ids costs nothing next to the rendered rows. The symmetric case cannot be repaired from here and is documented rather than hidden: a hunt STARTED mid-walk moves its row above the cursor and it never reaches this client until the next run. Two doc corrections in the same push. The --sort help promised that a stale-link row "leads the list with an empty Live Hunt Id", but the formatter gates the whole Live Hunt Id pair on a truthy value, so such a row prints no such line at all and reads as idle. And the commands spec called that row "stopped-but-unlinked", the inverse of the state it means: it is stopped and STILL linked, which is why it leads a list ranked on the link being present.
Review — against
|
The two dedupe tests repeated the same row wholesale and compared rows that differed in every field, so an implementation keyed on the name — or on the rendered block — passed both. Ruleset names are not unique, so that variant would swallow a real row from any inventory listing. The new case is the one that separates them: two distinct rulesets sharing a name must both render. Re-keying the dedupe on the name fails it and nothing else.
|
Reviewed against 1. The dedupe keeps the stale copy — and that is the copy whose The scenario the dedupe exists for is a hunt that stops mid-walk. Page 1 renders that ruleset while it is still linked — That is unavoidable for a streaming printer (the first copy is already on stdout), so this is a docs fix rather than a code one — but it undercuts the escape hatch the 2. The dedupe test pins the wrong duplicate shape (
3. The dedupe rule is documented only in the command catalogue ( "The obligation the SDK puts on a multi-page consumer of this mutable key" is an SDK-consumption invariant, and 4.
One item for the release step, not for this PR: the dedupe is unconditional, so plain Unverifiable from here, worth confirming before merge: that the SDK's |
…y so Round two of review. The dedupe keeps the first copy — the only choice a streaming printer has, since that copy is already on stdout when the second arrives — and the first copy carries the values from BEFORE the transition. So the one row the dedupe acts on prints the Live Hunt Id of a hunt that has already stopped. The help said to read the field rather than the position; under a mid-walk move neither is authoritative, and the help, the command docstring and the commands catalogue now say that instead of promising it. The dedupe test repeated a row wholesale, which is not the shape a re-serve takes: the two copies differ in exactly the field the sort ranks on, because that is why the row moved. It uses that shape now, and a new case pins first-wins as a decision — a last-wins rewrite fails it and nothing else. The invariant itself moved to specs/05 §Consuming the SDK correctly, beside the other SDK-consumption rules, where the next command that walks a mutably ordered endpoint will find it; the commands catalogue points at it.
|
Reviewed against 1. 2. Merge subject — the branch name must not land. Non-blocking observation: the |
…e duplicated prose Seven tests about sort forwarding and mid-walk dedupe had accumulated inside a class whose docstring promised only the zero-argument call, so nobody landing on the name would expect the dedupe pins to be there. They are their own class now, with a docstring that states both contracts it holds; the module docstring enumerates the new one alongside the others. The same ~150 words about stale Live Hunt Id values were restated in the help, the commands catalogue and the SDK-contract spec. The help keeps them — the user reading --help is the one who acts on a stale field — and the catalogue cell, the copy most likely to drift, is back to the one-line claim plus its existing pointer.
|
Reviewed against AGENTS.md, Clean against the documented conventions — no correctness, spec-drift, contract, coverage or gitflow issues found. Checks that mattered, for the record:
One nit, in the PR description only (not the code): the "The dedupe" section has a garbled duplicate — |
The inverse of --favorites-only, which the server refuses to combine with it. It exists for a client that renders the favorites as their own list: leaving them in the paginated list too makes a page repeat a row or come back short. A False flag is not a filter here either, so an unflagged invocation sends nothing new.
|
Review — dedupe design, 1.
This repo already has the pattern for exactly this: Also the docstring opener, 2. Floor attribution —
The pinned value is right; the attribution is wrong in three places. Against a 4.4.0 SDK 3. Missing test case No coverage for the refused pair — 4. Gitflow Base |
|
Approving. Nothing below blocks the merge. The one MODERATE is an edge whose mechanism predates this branch and whose fix lands in the SDK, and the rest is documentation — land any of it here or in a follow-up, whichever you prefer. SummaryAdds an opt-in active-first order to the ruleset list — rulesets with a live hunt ahead of idle ones — plus an Severity: 0 HIGH · 1 MODERATE · 5 LOW (set-wide). Prior feedback: 12 checked · 7 open (set-wide).
Fixes are proposed, not applied; nothing was run; no independent fix review in this run. Cross-repo coordination
Private companion repos are referred to by category here rather than by name, per this repo's Merge order: the server-side API PR → polyswarm-api#324 → this PR (§14: the API is the contract). This is the one merge in the set that genuinely breaks if taken out of order —
Fixes are independent; F1 is the only one spanning members, and its own fix names both sides. Findings (round 1)Every finding below is work for this change set; each entry's [MODERATE] F1. A ruleset walk whose last page empties exits 1 as "no results"Lands in: polyswarm-api#324 · Also touches: What happens:
Proposed fix (untested): No code change here. The fix is in polyswarm-api#324 — wrap
elsewhere: F4, F5 → polyswarm/polyswarm-api#324 · F2, F3 → the two internal repos (AI-attribution trailers on their commits and PR bodies — this repo and polyswarm-api are clean, because Outstanding review feedback
Addressed and not printed: the missing Standards conformitySet-level. §14 delivery order is satisfied — one externally-facing capability, API + SDK + CLI in one change set, no UI ahead of it, exercised through the SDK against a running stack. Rule 6 name identity holds byte-for-byte across all four repos. Project-level (non-clean rows only).
polyswarm-cli — Clean: §14, §15. Not applicable: §2–§13, §17. Checked and clean, so not reported: |
sbneto
left a comment
sbneto
left a comment
There was a problem hiding this comment.
Reviewed. Nothing blocking. One LOW on this PR: both specs place --exclude-favorites on the 4.4.0 SDK floor, but the keyword only exists in 4.5.0 — the pin is right, the attribution is not. The set's one MODERATE surfaces in rules list (the walk exits 1 when its last page empties) but its fix lands in polyswarm-api#324; the matching change here is a doc qualification. Merge last, and squash with a clean subject so the branch name stays out of public history. Details in the review comment.
sbneto
left a comment
sbneto
left a comment
There was a problem hiding this comment.
Reviewed as part of the DN-8445 change set alongside polyswarm/portal#2535. No findings land in this PR; the contracts it produces (the active-first sort, the two-directional cursor guard, and exclude_favorites) were re-derived against the consumer and are correct.
2 participants
What
polyswarm rules list --sort active-first— surfaces the API's opt-in active-first rulesetorder, where rulesets carrying a live hunt link list first. Without the flag nothing changes.
The flag maps to the SDK's
ruleset_list(sort='active_first'); the CLI adds no ordering ofits own, because the list is keyset-paginated and a client-side sort would reorder one page
and misrepresent the rest.
The help text is explicit that the rank is the stored link rather than what Live Hunt Id
renders from: a legacy row stopped while still linked leads the list and renders no Live Hunt
Id at all, reading exactly like an idle one. Read the field, never the position.
The dedupe
This command walks every page and exposes no limit, which makes it the consumer the SDK puts
the dedupe obligation on. The active-first key is the live-hunt link itself, so a ruleset
whose hunt stops between two page fetches drops below the cursor and the server serves it
again — the run printed it twice, and a script counting the output double-counted it. Rows
are now emitted at most once per run, keyed on id, unconditionally: the id is unique under
either order, and a gate would be a second place to update when another mutable order appears.
That leaves two costs. Neither is hidden: both are in the
--sorthelp, the commanddocstring, and specs/05 §"A mutable order makes the walk the caller's problem".
printer, and that copy carries the values from before the transition — so the one row the
dedupe acts on prints the Live Hunt Id of a hunt that has already stopped. Under this order
a moved row is authoritative in neither its position nor its fields; a fresh run shows the
settled state.
at all. Nothing client-side can repair that. A re-run lists it.
Follow-up for the release PR
The dedupe is unconditional, so plain
polyswarm rules listchanges behaviour too. This repo has no changelog, so per specs/05 §"Behaviour changes to existing invocations" that belongs on thedevelop → masterPR at cutoff, not here.--exclude-favoritesThe inverse of
--favorites-only, which the server refuses to combine with it. For a client thatlists the favorites separately. A False flag is not a filter here either, so an unflagged
invocation sends nothing new.
Requires
ruleset_list(sort=), released there as 4.5.0.This PR raises the floor to
polyswarm_api>=4.5.0. Merge precondition: the SDK'sdevelopmust DECLARE 4.5.0, clean of any
.devNsuffix, or CI reinstalls the SDK from the packageindex over the archive build. Until then CI resolves it from source — it installs
$POLYSWARM_API_ARCHIVE/$CI_COMMIT_BRANCH.zip, and that branch is pushed in the SDK repounder this identical name.
Verification
Full suite green (195). Four cases hold the dedupe, each failing against a different wrong implementation: a row served twice prints once; the default order is not narrowed by it; two distinct rulesets sharing a name both render (pins the key as the id — names are not unique); and the surviving copy is the first one, stale values and all (pins first-wins, so a last-wins rewrite fails). The duplicate the tests use differs in the field the sort ranks on, which is the shape a real re-serve takes.