Release 4.4.0 - #269
Merged
Merged
Release 4.4.0#269
Conversation
Two formatter legs, both getattr-guarded so the CLI still renders results parsed by an SDK release that predates the fields: - ruleset: Favorite / Favorited at, Rules in ruleset (absent when the server had no answer — never shown as 0), Historical hunts triggered, and New live results in window (only when the caller asked the list to include counts). - hunt: Source Ruleset Id, the source's last-modified at freeze time, and 'Source ruleset changed since this hunt froze it: yes/no' — the label names the reference point deliberately; unknown prints nothing.
The getattr guards (an old-SDK result without the attributes renders, new lines omitted), zero-distinct-from-absent for the counters, the truthy-only favorite leg, and the reference point in the changed-since-freeze label.
Maps to the server's include_counts so the 'New live results in window' formatter leg is reachable from the CLI (only live-hunting rulesets carry a count; the param is omitted unless asked).
The flag must reach ruleset_list(include_counts=True) and the unflagged run must omit the param entirely — the SDK drops None, and the exact wire value is load-bearing (the server only accepts '0'/'1'/'false'/ 'true').
…sources Review findings: - Blocking: the unconditional include_counts= kwarg made plain 'rules list' a hard dependency on an SDK newer than the pin's floor — 4.3.0's ruleset_list takes no arguments, so every unflagged run would TypeError against the published SDK (CI could not see it: the branch archive install picks up the new SDK). The kwargs are now built conditionally; only --include-counts requires the new SDK, matching the degradation claim the PR body makes. - The flag tests now autospec the mock, turning both assertions into signature checks against the installed SDK — the check that would have caught the above locally. - The rendering tests build REAL SDK resources from literal dicts, so an SDK attribute rename fails the test instead of silently dropping a line (the getattr guards convert mismatches into omission); they also pin that favorited_at/rule_modified arrive as parsed datetimes. SimpleNamespace remains only for the old-SDK degradation cases, where absent attributes are the point. - specs/02-commands.md documents the new flag and the floor-SDK constraint; specs/03-formatters.md records the non-obvious rendering semantics (0-vs-None, truthy-only favorite, the tri-state and its reference point). Dead 'is not None' half of the favorite guard dropped.
…ntract - rules favorite <id> [--unfavorite]: the CLI leg of the favorite capability (API, SDK and CLI land together as one change set). Renders the toggle state plus the server-owned 'Favorites used: N of M' budget, and converts the machine-readable FAVORITE_LIMIT refusal into a clean actionable message at exit 2 — the central mapping's server-refusal code (a ClickException would exit 1, the code reserved for no-results/not-found). Pinned end-to-end against a real recorded 400 (tests/cli_test.py::test_ruleset_favorite_limit_text), not just a hand-built mock, so a rename of the error shape on either side fails a test. - The command guards the SDK surface: ruleset_favorite ships in the paired SDK change and does not exist on the declared floor (published 4.3.0), so on a floor install the command fails with a clean upgrade message instead of an AttributeError traceback — the same only-the-new-surface-may-require-the-new-SDK principle as the withdrawn flag below. Every pre-existing command works unchanged on the floor; the floor itself cannot move until the SDK releases (specs/05 documents the exception and the follow-up bump). Every test touching the new surface is guarded on the narrowest dependency it actually needs (the method for command tests, the resource class for formatter fixture tests) so a rename on either side skips only the tests that need it, not the whole suite silently. - drop --include-counts: it wrapped a per-request server aggregate that is withdrawn (no count is computed on a request path; the badge is a stored, server-refreshed counter) — and it crashed on the declared floor SDK, which CI's branch-name SDK install could never surface. With it gone, list_rules is zero-argument again. - the new-results badge renders as 'New live results (last 24h)' — the fixed product window, since a caller can no longer choose one — with the new_results_counted_at staleness marker beside it. - specs: the sdk-contract floor header follows the pin (>=4.3.0, moved by #264; the header had lagged at 4.2.0), the imports table records RequestException/.request.errors as a real SDK dependency, and the command/formatter tables cover the favorite leg, its floor degradation, and the stored-counter render.
All ruleset/live/historical cassettes (and their click snapshots) re-recorded against a stack running the paired server branch: every ruleset body carries the four tracking keys, the LIST bodies additionally carry the stored new_results_count / new_results_counted_at pair (the detail serializer deliberately does not render the badge), livescan_id is a digit string on every surface, and the fixture ids are this recording run's own resources. New recordings cover the favorite toggle's happy path in both output formats, unfavorite, and a real FAVORITE_LIMIT refusal recorded against a genuinely full team budget.
A new OPTION may require the newer SDK; an existing INVOCATION may not. Passing an unknown keyword to an older SDK raises a bare TypeError that ExceptionHandlingGroup renders as a traceback plus 'Please contact support', so the guard fires only when the caller actually uses the new option and produces a clean upgrade message instead. Inspects the installed signature rather than catching TypeError, so a genuine argument error inside the SDK is never mistaken for a version mismatch. Two commands need it, which is what earns a helper over an inline check.
rules view renders a per-ruleset new-results badge and there was no way to list the results it counts: live feed gains --livescan-id, the badge's drill-down. It also gains --max-results, and rules list gains the four server-side filters the SDK exposes (--name, --status, --favorites-only, --has-new-results). The list is keyset-paginated, so filtering locally would mean walking every page. --since is documented in MINUTES to match the corrected wire unit; its 1440 default is unchanged and now means the 24h it was always meant to mean. 0 means no time filter at all. Every new option is forwarded only when passed, so an unfiltered rules list and a plain live feed still reach the floor SDK's own signatures untouched. specs/05 records all three floor-exceeding surfaces and why the floor itself does not move.
Covers the three decisions the plumbing now makes: an unfiltered list still calls a zero-argument ruleset_list(), a filtered one forwards exactly the filters given (a False flag must not become favorites_only=False, a filter the caller never asked for), and live feed forwards the two new kwargs only when passed. Each new surface is also pinned to degrade with a clean message at exit 2 against a stand-in carrying the floor signature, never a traceback.
The floor version was hardcoded in three places, so the follow-up bump that drops the guards had three chances to miss one. SDK_FLOOR states it once. --max-results takes IntRange(min=0): 0 is meaningful (no bound, matching the SDK) but a negative is a typo, and refusing it at the interface beats silently treating it as unbounded.
…lass The floor guards were keyed on the resource CLASS, but YaraRuleset and HistoricalHunt exist on the published floor — they simply do not parse the tracking and provenance keys. Resources declare attributes explicitly, so on a floor install the formatter's getattr guards return None and these tests FAIL rather than skip. test_ruleset_none_and_false_fields_are_omitted was worse: it asserts absence, so it passed vacuously there, looking like coverage while pinning nothing. Keyed on the attribute the fixture actually needs. specs/05's import row also claimed a bare `from polyswarm_api import exceptions` while rules.py aliases it — the drift the spec convention exists to prevent.
--max-results 0 is documented as the pre-existing unbounded behaviour, but it was forwarded to the SDK and therefore tripped the floor guard: a caller asking for exactly what the floor already does got 'requires a polyswarm-api release newer than 4.3.0'. That contradicts the rule specs/05 states as the reason the floor does not move — a new OPTION may require the newer SDK, an existing INVOCATION may not. 0 now stays out of kwargs and off the wire entirely. The FAVORITE_LIMIT counters are advisory and an envelope can carry the code without them; interpolating them unguarded rendered 'Favorite limit reached (None of None used).' at the user. The server's own message is the fallback. SDK_FLOOR is now tied to the pin by a test — it existed only so the guard messages could name the floor, and nothing failed if it drifted from pyproject.toml while every message named the wrong version. The guard tests assert against the constant rather than a literal. specs/03-formatters.md still declared the 4.2.0 floor, which is the drift specs/05 names the pin to prevent.
The help text sent users to 'rules view', which is the one ruleset command that deliberately does not carry new_results_count — the badge is a list-serializer field. It names 'rules list' now, in the docstring and specs/02. exc.request.result was read unguarded inside the handler whose whole job is to avoid a traceback; the only test built a Mock, which has every attribute, so it could never fail on this. getattr now, pinned by a request object that really lacks it. Also guards the counters-fallback test on the floor (it patched ruleset_favorite with autospec and no create=True, so it errored rather than skipped there), and signature-checks all four rules-list filters instead of two — autospec is what makes those assertions a check against the installed SDK, and a kwarg rename would otherwise ship as the floor guard refusing on an SDK that has the surface. The imports table had two rows with an identical left column after the earlier alias correction; folded into one.
require_sdk_kwargs refused any method declared **kwargs: signature() reports one VAR_KEYWORD parameter rather than the names it accepts, so the guard would have told a user to upgrade an SDK that already supports the option. It fails open there now — the reason for inspecting the signature at all is to avoid a confusing upgrade message on a working install. --livescan-id takes click.INT like every other id option; Python ints are arbitrary precision, so a 17-digit id survives exactly (the server renders it as a string for JS consumers, not for us) and a typo is refused before it reaches the server. specs/04 gains the floor-guard convention this change introduced. It is load-bearing and lived in no spec: guard on the narrowest dependency, because a class-level guard does NOT skip a test whose resource exists but does not parse the attribute — the render tests fail and an absence-asserting test passes vacuously. Also drops the 'exit 2 is the server-refusal code' claim from a comment and from specs/02: ExceptionHandlingGroup maps 2 to a broad bucket, so the supportable contract is '2, not 1'.
test_live_hunt_start_text / test_live_hunt_stop_text expect 'Rules in ruleset' and 'Historical hunts triggered' since the cassettes were re-recorded, but those lines only render when the SDK PARSES the attributes. On a floor install the formatter's getattr guard omits them and both tests FAIL — the exact case the specs/04 section this change adds legislates against. The guards are now needed in two modules, which is what earns them a shared home rather than a second copy of the resource-building boilerplate. Verified both directions: the tests run against the paired SDK and skip when the attribute is absent. specs/02 also records that the two hunt --since options differ in unit on purpose — live feed is minutes, historical list is seconds, because they are different endpoints and the server reads each accordingly. Without saying so the remaining 'seconds' reads as a missed rename.
A blockquote between rows terminates a GFM table, so the note split the catalogue: everything from 'historical' down rendered as a paragraph of pipe-delimited text, including the rules row this change rewrote. Moved below the last row.
The 'exit 2 is the server-refusal code' claim was dropped from a comment and specs/02 last commit but survived in the command DOCSTRING — which is what 'rules favorite --help' prints — and in two test docstrings. ExceptionHandlingGroup maps 2 to a broad bucket, so the supportable contract is '2, not 1'. cli_test.py hand-rolled a second copy of _needs_favorite_method against the same hasattr, which is the drift tests/_sdk_guards.py was added to stop; the last hardcoded '4.3.0' in a guard assertion now reads SDK_FLOOR, the constant SdkFloorConstantTest ties to the pin. Two dangling references: a comment cited '--include-counts', withdrawn inside this PR and present nowhere in the tree, and another still said list is zero-argument after this change gave it filters. specs/03 attributed the floor to the behaviours that set 4.2.0 — 4.3.0 came from the #264 bump — and now points at specs/05 rather than restating it. specs/04 and specs/05 said 'utils.' for helpers that live in client/utils.py, not the top-level utils.py specs/01 documents. Also records why exc.request is read directly: RequestException.__init__ assigns it unconditionally, so a guard there would be dead code. Raised twice in review; written down so it stays settled.
The 1440 default was written as 24*60 against a docstring that said minutes; the server reads seconds, so the real window was 24 minutes while the badge beside it counts 24 hours. Fixing the caller rather than the wire gets the same 24h with no break for existing integrations. historical list --since is seconds too, so the two now agree. Also trims the FAVORITE_LIMIT handler and the SDK guard comments.
The guard probes asked hasattr() for keys the probe payloads omitted. That works only because the SDK assigns every attribute unconditionally — verified, and the guarded tests do run — but it made the guards depend on that; the keys are in the payloads now, so a silent skip-everything cannot arise from an SDK style change. The FAVORITE_LIMIT fallback interpolated exc.request.result unchecked; result is the parsed body, so a dict would have reached the user as a repr. Two edits I left half-done: specs/03's floor paragraph lost its sentence, and specs/05 still named 4.2.0 above the heading that says 4.3.0. Drops the imports the shared-guard move orphaned.
`test_filters_are_forwarded_only_when_given` and `test_livescan_id_and_max_results_are_forwarded` pass a keyword the floor SDK does not accept, so `require_sdk_kwargs` refused and the command exited 2 — both asserted `exit_code == 0` and FAILED there rather than skipping. CI never caught it: the branch-name match installs the paired SDK, so the floor install the pin permits is the one nobody exercises. Guard them on the parameter's presence in the installed signature, the narrowest dependency a keyword-passing test has. The plain-invocation tests stay unguarded — the floor supports those, and skipping them would drop the coverage that matters most. specs/04's table prescribed `require_sdk_kwargs` for this row. That is product code and never skips a test, so the row described exactly the bug above; its reasoning about keeping the unfiltered call covered was right and is kept. Also drops two imports left over from moving the guards into `tests/_sdk_guards.py`.
`live feed --livescan-id` was documented as the drill-down for the ruleset new-results badge — "this is how you list them". It lists a subset: the badge counts the hunt across every community it runs in, public and private together, while the feed shows one at a time and this command always sends one. A user drilling down on a multi-community hunt sees fewer rows than the badge reported, or none at all. Documents the asymmetry instead of implying an equivalence that does not hold. No behaviour change.
`needs_live_feed_options` checked only `max_results` while gating a test that passes `--livescan-id` too, and `needs_ruleset_list_filters` checked only `name` while gating a test that passes all four filters. An SDK carrying the subset would satisfy the guard, then `require_sdk_kwargs` would refuse the invocation at exit 2 and the test would FAIL instead of skipping — the exact failure mode these guards were added to prevent, reintroduced by keying them too narrowly. `_accepts` now takes several names and requires all of them. Verified it discriminates: True against the paired signatures, False against a partial SDK carrying only `max_results`.
`test_favorite_calls_the_sdk_and_renders_the_budget` and `test_unfavorite_flag_flips_the_boolean` carried @_needs_favorite_resource only because the shared `_response()` built a real YaraRulesetFavorite. A rename of that class would therefore have skipped the only two tests that assert `rules favorite` calls the SDK at all — the failure the guard module's own docstring says it exists to prevent. TextOutput.ruleset_favorite reads `.id` plus getattrs, so a SimpleNamespace serves and the command tests now depend on the METHOD alone. The two fixture tests that genuinely instantiate the resource keep the guard. Also pins the exception hierarchy the exit-code mapping silently rests on: non-limit refusals exit 2 only because the SDK's RequestException subclasses PolyswarmException and the handler catches that base before the transport branch, which matches the bare name 'RequestException' against the MRO and exits 1 with "contact support". A reparent in the SDK would turn every fixable 4xx into that advice; the new test fails loudly instead. specs/05 records the two server-owned behaviours the CLI relies on and cannot enforce — `--since` being seconds, and `--since 0` meaning no filter — naming the server-side tests that pin each.
`test_ruleset_favorite_text` / `test_ruleset_unfavorite_text` assert rendered
lines ("Favorites used: N of M", "Favorited at:") that appear only when the
SDK parses those keys off the response. specs/04 row 3 says a render
assertion guards on the built resource attribute, not the method — otherwise
an absence-asserting test passes vacuously. Method and resource ship together
today, so this is the drift that row exists to stop rather than a live break.
Records the published 4.3.0 signatures in specs/05, read off the wheel rather
than inferred: `ruleset_list(self)` and `live_feed(self, since, rule_name,
family, polyscore_lower, polyscore_upper, community)`. Neither declares
**kwargs, so `require_sdk_kwargs`'s fail-open branch is unreachable against
the real floor and the floor tests' stand-ins match reality. That mattered:
had either taken **kwargs, the new options would have been forwarded to an
SDK that drops them and the caller would get an unfiltered list at exit 0.
specs/02 now also names `download stream --since` — a third --since that is
genuinely minutes with a 1440 default, the likeliest origin of the original
mistake. And drops a review-bookkeeping aside from rules.py.
--max-results 0 is deliberately dropped before the SDK while --since 0 must reach it, because 0 is how the server is told to apply no time filter. The two zeros mean the same thing to a user and take opposite paths in the code, and only one was pinned. Verified the new assertion catches the refactor it names: folding `since` into the conditional-kwargs block makes it arrive as None and fails that test alone. `live feed --help` was carrying the rationale for the option TYPES — why click.INT, why a negative is refused — which is reviewer context, not behaviour a user needs at the prompt. Moved to a comment; the help lines now read like every other option in the group. specs/05 also now documents `exc.request.result`, the second attribute `rules favorite` reaches for on the SDK's request object. The table exists to enumerate exactly that, and only `.errors['code']` was listed.
`require_sdk_kwargs` returns early when the installed method declares **kwargs, forwarding rather than false-refusing. Published 4.3.0 declares none, so the branch is unreachable today — but it is product code, and an SDK that grew one would take it and silently forward options the SDK drops. Verified the test discriminates: deleting the branch makes it fail alone. specs/01 §Support was the last spec still attributing `parse_hashes` to the top-level `utils.py`; it lives in `client/utils.py`, which is also where this change adds `require_sdk_kwargs` and `SDK_FLOOR`. specs/04 and 05 were corrected earlier in this PR, 01 was missed.
The CLI needed surfaces the published SDK did not have, and expressed that as runtime probes: `require_sdk_kwargs` inspecting signatures, a `getattr` on `ruleset_favorite`, and four per-test skip guards. The pin stayed behind at 4.3.0 so the probes had something to protect against. That put the same fact in two places — the pin, and each probe — with nothing keeping them in sync. Every probe was one edit from disagreeing with the code it guarded, in either direction: check less than the test uses and it FAILS where it should skip; check more and it SKIPS a test that would have passed, dropping coverage while CI stays green. Five defects came out of that in review, each a different way of getting the same mapping wrong. And a green run against the paired SDK never verified the floor install the guards existed for. The floor now names 4.4.0, the version that introduces those surfaces, and pip enforces it at install time before any code runs. Deleted: `_sdk_guards.py`, all 22 guard decorators, `require_sdk_kwargs`, `SDK_FLOOR`, the favorite getattr dance, and the six tests that only exercised the guards. Coverage goes UP: 146 tests, none skipped. Every test that could previously skip itself now always runs. specs/04 replaces the guard convention with the version contract and says why it is gone; specs/05 documents raising the floor as the procedure, the release ordering it forces, and the PEP 440 dev-suffix trap. specs/01 and specs/02 drop the helper and the degradation language.
The formatters guarded every hunt-page field with `getattr(result, x, None)` and said in three places it was so an SDK predating the fields could still render. The floor now forbids that SDK, and 4.4.0 assigns each attribute unconditionally — verified: a resource built from a payload carrying none of the keys still answers hasattr() for all six, with value None. So the getattr DEFAULT was unreachable and only `is not None` was ever doing work. Read the attributes directly in `hunt()` and `ruleset()`, and say what None now means: the SERVER had no answer. `ruleset_favorite()` keeps its getattrs — its budget counters are genuinely optional server-side — and `artifact_instance()` is untouched, pre-existing and outside this change. Removes the two tests that rendered a SimpleNamespace missing the attributes entirely: they pinned an install the pin forbids, which is the same fiction the deleted skip guards traded in. Also clears what the previous commit left behind: an empty `# SDK-surface guards` banner in client/utils.py, a test module docstring and comment block describing guards and `create=True` that no longer exist, a class docstring justifying the zero-arg call by the old floor's signature, and specs/03 both lagging at 4.3.0 and still describing the getattrs as old-SDK protection.
…hinery
`TextOutput.ruleset_favorite` was the one leg still guarding, and it failed in
the worst direction: a missing `favorite` made `starred` None and the else
branch printed "Favorite: no" — a wrong state after a successful star, not an
omission. `YaraRulesetFavorite` assigns all four attributes unconditionally
(verified: a resource built from `{'id': '5'}` alone answers hasattr for every
one), so the defaults were unreachable. Read them directly, and let the command
tests build the real resource again — the SimpleNamespace existed to decouple
them from a skip guard that no longer exists.
Prose that outlived the code: specs/02 still said the formatters getattr-guard,
and the test module docstring still advertised "the getattr guards convert an
attribute-name mismatch into silent omission" and an old-SDK SimpleNamespace
path whose tests are gone.
specs/04 no longer frames the rule as replacing something. The guards were
created and deleted inside this branch, so `develop` never had them and a
reader grepping history for the removal finds nothing; the forward-looking
"do not reintroduce" rule and its reasons are what carry over.
Also drops a dead `utils` import in live.py and moves the IntRange comment onto
the option it actually describes.
… rationale
specs/05's version-pin paragraph still ended "the pin has since moved to
4.3.0" — two lines above the `### Current floor — >=4.4.0` heading, in the very
file that declares §Current floor authoritative. Fresh drift from this branch's
own edit; the sentence now points at the heading instead of restating a value.
specs/02 still justified the conditional forwarding with the withdrawn design's
reasoning ("need the paired SDK ... unchanged on the pin's floor"). The pin
guarantees both parameters now.
The block itself is redundant and the comment now says so plainly: `livescan_id`
defaults to None and `as_result_bound` maps 0/negative/None to "no bound", and
I verified the request is byte-identical whether the kwargs are omitted, passed
as None, or passed as 0. It stays only so a pre-existing invocation's call shape
does not move — which the plain-feed test pins, alongside the `--since 0`
refactor hazard beside it. Simplifying it away would cost that guard for no
behavioural gain.
…line
`ruleset_favorite` gated the timestamp on `is not None` outside the starred
branch, so an unstar response still carrying `favorited_at` renders
"Favorite: no" with a "Favorited at" line beneath it — a contradiction, and
against what specs/03 documents ("the timestamp when starred"). The `ruleset`
leg directly above already nests it correctly.
No cassette catches this because the server sends null on unstar, and the
existing unfavorite test passes null too, so the case is unit-pinned. Verified
the new test discriminates: reverting the nesting fails it alone.
Also drops an unused pathlib import and separates the four new cassette tests,
which ran together as one block.
The FAVORITE_LIMIT fallback read `exc.request.result`. `PolyswarmRequest` has
no such attribute — the dataclass field is `_result`, and the response envelope
is `.json` — so `getattr(..., 'result', None)` was always None and the fallback
was dead code. At the cap without counters the user got the generic "Favorite
limit reached." instead of the server's own message.
Nothing caught it. The test that exercises the fallback set `.result` on a
mock.Mock, which fabricates whatever attribute it is asked for, so it passed
against a spelling no real request has. Its companion test deliberately used a
bare object, but asserted only that no traceback escaped — not that the message
came through. Between them they covered everything except the one thing that
was wrong.
Now reads `(getattr(exc.request, 'json', None) or {}).get('result')` — the path
the SDK's own comment documents — with getattr so a malformed request still
reaches the clean message. The test builds a real PolyswarmRequest; verified it
fails when the old spelling is restored.
specs/05 also overstated the pin: the recorded 400 carries counters, so that
cassette covers the `.errors` branch and never touches the fallback. Says so
now, and names the spelling trap.
The ordering section read as a warning: merge this side first and "develop CI breaks for every subsequent PR". That describes the mechanism correctly and the intent backwards. The two develop branches are tested against each other on purpose — that is how both ends carry the latest features and stay exercised together without waiting on a release. A change spanning the pair is pushed and merged to both develops together, and a failure out of lockstep is the pairing being broken, not a trap to design around. It also collapsed two separate events. Merging to develop publishes nothing; PyPI only sees a version at develop -> master. So the floor a feature PR sets is a working value that develop integration validates, and the cutoff is where the SDK version gets confirmed — bumped if the repo files do not already carry it — and this repo's dependency set to the version actually being released.
`New live results (last 24h)` and the `--since` help both named 24h as the badge window. The response carries only the count and its refreshed-at marker, and the SDK contract deliberately declines to name a window — so the CLI was the only thing claiming it, with nothing in either repo failing if the server's refresh interval changed. Dropped from both; the refreshed-at line beside the count is what actually tells a reader how current it is. Three more from the same round: - The FAVORITE_LIMIT message appended "Unfavorite another ruleset first" in both directions, including when the refused invocation *was* --unfavorite. Only the star direction can hit the cap, and only it has a remedy. - A comment named `as_result_bound`; the SDK made it `_as_result_bound` and marks it private, so the name is gone rather than re-spelled. - The formatter test built a StringIO and asserted on the stream. specs/04 Style 3 says write=False and assert on the returned lines, which is what the spec's own cited example does — drift introduced in the PR that edits that spec. specs/03 no longer restates the floor value. specs/05 §Current floor is authoritative, and repeating the number in 03 is exactly what let that line sit at 4.2.0 while the pin said 4.3.0.
The code stopped claiming a 24h window; two specs and the PR body did not. specs/03 still said the window is "the fixed 24 h product window, which the label names" — the label names nothing now, and the response carries no window field, so there was nothing left to name it. specs/02 justified the --since default as "matching the window the ruleset badge counts", which asserts a server constant neither repo pins. Also fixes the referent specs/02 shared with the help text before it: "the detail view deliberately does not carry it" reads as --livescan-id, and `rules view` does render Live Hunt Id. It means the badge. Adds the missing case for the list-shaped `errors` envelope. The guard is deliberate — only the mapping shape carries a machine-readable code, so a list-shaped refusal cannot be FAVORITE_LIMIT and must fall through to the generic path rather than raising on `.get`. Nothing drove that branch; now something does. And drops an `import io` left over from the write=False refactor.
Left over from dropping the parenthetical: the code comment still stated the window as a fact the response does not carry. Says what is actually true now — the badge is a stored counter, the window is the server's and unstated, and the staleness marker is what tells a reader how current the number is.
`api.ruleset_favorite(rule_id, not unfavorite)` binds the boolean positionally, and so does the test's `assert_called_once_with(mock.ANY, 5, True)`. autospec checks the signature, but positional binding means a parameter inserted between `ruleset_id` and `favorite` would silently misroute the toggle with both the command and its test still green. Passing `favorite=` is what makes the signature check load-bearing, which is the stated reason autospec is used throughout this file. Docs from the same round: - specs/04 carried "never mock and replay the same path", which this PR deliberately crosses for FAVORITE_LIMIT. The duplication is worth keeping — the mock pins the message and exit code, the cassette pins where in the envelope the code lives — so the carve-out is now written down with the test that each half has to name. - The re-record steps did not mention that `test_ruleset_favorite_limit_text` needs the favorite budget already saturated. Re-recording it against a fresh stack yields a 200 and a cassette that tests nothing. - specs/05 says why the no-counters fallback is unit-pinned and not recorded: the server sends counters on every refusal, so no cassette can produce the envelope the fallback exists for. - specs/03's floor sentence claimed authoritativeness twice and left a dangling clause — an artifact of my own previous edit. - A `live.py` comment described the SDK mapping a negative bound, which `IntRange(min=0)` refuses before the SDK ever sees it.
…cannot meet it specs/04 says the suite must pass against a live stack with VCR off, and "if a test only works against its recorded cassette, that's a bug in the test". The note I added last commit conceded the opposite for the favorite-limit cassette and put the concession in the re-recording section — which documents how to record, not how a VCR-off run is meant to pass. Both statements could not be true. Resolved on the invariant itself, with the mechanism named: the assertion needs a stack STATE the suite cannot create, because saturating the favorite budget on a shared stack consumes every slot the other favorite tests need. The paired SDK reached the same conclusion for the same refusal and used a stubbed transport rather than a recording. The carve-out also names what covers the ground when VCR is off — the SDK-boundary mock pins the message and exit code, the SDK's respx suite pins the envelope — so the exception is bounded rather than open. Also trims spec prose that narrated this PR instead of stating the contract: a line in specs/01 explaining that the section used to name the wrong module, and a specs/05 clause recounting which release moved the floor and how far the header lagged. A reader six months out wants the current contract.
…e rests on Three documentation gaps, each one a thing a future editor could get wrong without the code telling them. AGENTS.md asserted the VCR-off invariant unconditionally while specs/04 now carves out one named exception to it. AGENTS.md is the first file a contributor reads, so the contradiction resolves the wrong way by default. It now points at the argued exception and says that adding a second one means writing it down there too. specs/01's exit-code table describes the transport branch as matching legacy `requests`. It also matches the SDK's own RequestException, which shares that bare name and therefore satisfies the ancestry-name test. That exception exits 2 rather than 1-with-"contact support" purely because the PolyswarmException clause is matched BEFORE the transport branch — an ordering the table did not mention, so reordering the clauses looked free. It is not: it silently changes the exit code of every SDK request refusal. specs/03's formatter inventory listed the ruleset methods but not ruleset_favorite, which the same spec documents in detail further down.
… make the guards testable --max-results carried IntRange(min=0) with a stated rationale — 0 is meaningful, a negative is not, and bare INT forwards it to the server. --since two lines up had the identical property (0 means "no time filter" and is asserted as such) and stayed bare INT, so `live feed --since -1` was forwarded verbatim. Adding the guard surfaced a worse problem in the tests that were supposed to cover it. All three interface-refusal tests asserted only `exit_code != 0`. An unvalidated value does not stop at the interface — it reaches the network, the connection fails on its own, and the process exits non-zero for that reason instead. Verified by reverting each guard: every one of the three tests still passed with its guard removed. They were pinning nothing. They now assert the refusal happened at parse time: exit 2 with a usage error naming the option, which cannot be produced by a request that was never supposed to be made. Re-verified the same way — with the guards reverted all three fail, and with them restored the suite is green.
…amed for test_other_refusals_still_raise asserted exit 2 and that the raw FAVORITE_LIMIT code did not leak into the output. Neither fact distinguishes a refusal that fell through from one the handler swallowed: both paths exit 2, and the handler's own message says "Favorite limit reached", which does not contain the literal string the test looked for. Verified by forcing the branch to treat every refusal as the limit case — the test still passed. It now asserts the limit-specific message is absent, which does fail under that same injection, and keeps the raw-code and traceback assertions alongside it. Checked the neighbouring list-shaped test the same way before touching it: that one already discriminates (removing the isinstance guard makes a list .get() raise, and the traceback reaches the captured output), so it is left alone.
The test constructed RequestException with the request alone, which stringifies to empty — so the fall-through rendered a blank line and the only assertions available were negative ones about what was absent. Every raise site in the SDK passes a message alongside the request, so the test was not exercising a shape the CLI ever sees. With a message, the test can assert the positive fact it exists to establish: the server's own explanation reaches the user, unmodified, on the path that does not belong to the limit handler.
…st that cannot meet it" The carve-out was wrong three ways, and reverting is cheaper than repairing it. Wrong on the count. It named ONE test that cannot pass with VCR off. Three other cassettes added here pin server-generated values a live stack will not reproduce (`Favorited at: 2026-08-26 ...`, `Favorites used: 2 of 5`), because .click snapshots are exact string equality. So is a cassette already on develop, which pins `Created at: 2022-05-26 ...`. The next contributor would have counted wrong. Wrong on the mechanism. It said a VCR-off run "gets a 200 and the assertions fail". The recorded ruleset id does not exist on a fresh stack, so such a run 404s and exits 1. Same conclusion, wrong reason — and the re-recording steps were written around that wrong reason. Wrong on the venue. The condition it described is not something this change introduces: exact-match cassettes have never been runnable against a fresh stack. Weakening a project-wide invariant to accommodate a pre-existing condition is a maintainer decision, not a line item in a feature PR. AGENTS.md goes back to develop's wording verbatim. The tension between the invariant and snapshot-style cassettes is real and stays on the record as-is, for a decision made on its own terms. Also drops the claim that dropping the cassette "would leave the CLI asserting a shape nothing checks" — the paired SDK's respx suite does check it. What the cassette actually pins is the seam between the two, which is what it now says.
…igit count The envelope-spelling test assigned request.json itself, so it could not detect the rename it was written to detect — its own comment explains that a real request was chosen over a Mock because "a Mock fabricates whatever attribute it is asked for", and assigning the attribute does the same thing. It now pins the spelling on the untouched request first, before assigning: .json present, the mistaken .result spelling absent. Verified by renaming the attribute across the SDK (the class annotation as well as the assignments) and watching it fail. The empty-remedy arm of the FAVORITE_LIMIT handler had never executed: every test reaching that branch invoked without --unfavorite, so inverting the conditional would not have failed one. Verified by inverting it. Now covered. The known-good getattr guard carried the rationale this branch spent its commits removing — "a CLI running against an older installed SDK won't have them". Those attributes ship three releases below the floor, so the pin already guarantees them; the guard survives as belt-and-braces for an unsupported configuration, which is what the comment now says. Left as-is it invites both mistakes: deleting the guard, or adding siblings for versions the floor covers. --livescan-id's help promised "a 17-digit number"; this repo's own cassettes carry 16-digit ids. specs/02 records that a negative --since is now refused at parse time rather than forwarded, and that exit 2 does not identify a server refusal — click uses it for usage errors too. specs/05's cutoff gains the release-note obligation for behaviour changes to existing invocations, since there is no CHANGELOG to carry them.
…nd the wrong failure The section pointed at 'the VCR invariant above' for an exception that no longer exists there, and repeated the mechanism the revert corrected: a fresh-stack recording does not yield a 200. The recorded ruleset id is not on a fresh stack at all, so that run 404s; the 200 is what a stack WITH the ruleset and an unsaturated budget returns. Both states have to be set up, and saturating the budget takes slots the sibling favorite tests need.
…-since 0 rests on Both nested guards in the ruleset leg had only their True arm exercised. The tracking-fields test builds a count with no refresh marker — the inner guard's False arm — but never asserted the marker was absent, and no test rendered a favorite without a timestamp at all. Inverting either guard failed nothing. Verified by inverting both; both new assertions fail, and neither did before. --since 0's help text promises "no time filter at all", which is a claim about the server, not the CLI. Confirmed against the endpoint: the window is applied only when since is truthy, so 0 and an absent parameter are the same request. specs/02 now says so, and says what the CLI test can actually pin — that 0 is forwarded rather than dropped, which is the half that can regress in this repo. The re-record steps named one state-dependent cassette. All three ruleset-favorite cassettes are, because the rendered block is compared verbatim and the budget counter is inside it: one pins "Favorites used: 2 of 5", another "1 of 5". They have to be recorded together from a known starting state or the counters contradict each other.
…atisfiable The three ruleset-favorite cassettes describe one coherent sequence: star a ruleset (1 of 5), star another (2 of 5), unstar it (1 of 5). The cap refusal needs 5 of 5. With VCR off, saturating the budget to record or reproduce the refusal turns the other two into 400s, failing their expected exit 0; leaving it unsaturated turns the refusal into a 200. There is no stack state where all four pass, so the suite could not be run against a live stack at all. The previous commits tried to write that down instead of resolving it -- first as an exception to the VCR invariant (reverted), then as re-recording guidance. Documenting a test that cannot run is not the same as having one. Removing it costs little now, and less than it would have earlier in this branch. The message and exit code are pinned at the SDK boundary; the envelope spelling is pinned against a real PolyswarmRequest, which is the part a cassette was carrying until this branch added that pin; and the wire shape is pinned by the SDK's stubbed-transport suite -- which declined to record this same refusal, for this same reason, in the paired PR. Recording it on one side while stubbing it on the other was the inconsistency.
…exists The previous commit removed the favorite-cap cassette but left specs/04's "one sanctioned exception" paragraph asserting it — "the recorded 400 pins where in the envelope the machine-readable code actually lives", "the cassette is what pins the seam between them". Both describe a file this branch deletes, and the same paragraph licensed mock-plus-VCR coverage of one path, which is the rule it was nested under as an exception. Nothing needs the exception now: the refusal is covered by unit tests on this side and a stubbed transport on the SDK's. Also moves development history out of user-facing help and out of comments that had grown into review transcripts. `live feed --help` no longer explains that an old default was written as 24*60 in the belief that the unit was minutes — it says what the default is and how to get the old window back. "The badge" had no referent for someone reading --help and is now named for what it is. The kwargs and FAVORITE_LIMIT comments state the fact at hand and point at the spec for the reasoning, instead of naming test functions that will drift.
…y is
`errors` is isinstance-guarded so a non-mapping envelope falls through to the
generic path instead of raising on `.get`. The `.json` read two lines down had
no such guard: `(getattr(...) or {}).get('result')` only substitutes `{}` for a
falsy value, so a list envelope reaches `.get` and raises AttributeError —
inside the handler whose entire purpose is turning this refusal into a clean
message. Reproduced: full traceback plus "Unhandled exception happened. Please
contact support."
Needs three things at once (a dict `errors` carrying the code, no counters, and
a non-dict `.json`), which is why no existing test caught it. One does now;
verified by reverting the guard.
specs/04 also names the mechanism the favorite cassettes' sequence rests on:
unittest orders methods by sorted name, so renaming one breaks the recorded
star/star/unstar sequence silently — replay keeps passing and only the next
re-record surfaces it. And the VCR bullet now points at the re-recording section
rather than appearing to contradict it: the invariant is about a test's logic,
while a .click snapshot pinning server-generated values is a fixture concern.
The hygiene rule names commit messages, PR titles and PR descriptions. Branch names sit outside it, and reviewers have repeatedly and reasonably read that as an oversight — a branch ref is just as public and just as permanent. It isn't an oversight, but the reasoning lived nowhere in this repo. A change spanning this repo and the SDK has to use the identical branch name in both, because CI resolves the companion SDK by $CI_COMMIT_BRANCH; for a ticket-tracked change that name is usually the ticket. So the prefix is deliberate. What actually leaks is the default merge subject, which interpolates the head ref. Squash-merging with an explicit subject contains it, and that is now written down next to the rule it looks like an exception to, along with the preference for a descriptive shared name where one reads just as well.
… happened The recipe said to record the three favorite cassettes together, in sorted-name order, from a known starting state. Two things are wrong with it. It omits test_ruleset_list_json, which sorts between favorite_text and unfavorite_text and pins "favorite": false on every ruleset in its inventory. Anyone following the recipe records it while a ruleset is starred, so that snapshot changes as well. And the shipped cassettes were not made that way: favorite_text acts on 96652060989160147, which does not appear anywhere in list_json's inventory (77454540525125655, 44051669277897879, 78562964231669682). They were recorded against different stack states, so following the recipe would surface the contradiction rather than reproduce the fixtures. Now states the couplings instead of a sequence, keeps the silent-failure warning about renaming a method, and points at the escape hatch that exists already: _assert_text_result takes a replace= hook, so normalising the budget counter makes each cassette independent of what ran before it. Better to break the coupling than to document a wider one.
Render hunt-page ruleset tracking and hunt provenance fields
Release bump for 4.4.0 — the hunt-page ruleset tracking release (#266). The SDK floor polyswarm_api>=4.4.0 is already pinned; polyswarm-api Release 4.4.0 must be on PyPI before this repo's develop → master merge. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Reviewed against 1. Blocking — the version bump is not in this diffThis is the
It does not. At this PR's head:
The only That state is worse than "nothing happened": 2. Merge gate — confirm
|
Bump version: 4.3.0 → 4.4.0
admin-sbneto
approved these changes
Sep 8, 2026
|
Reviewed against What I checked and found clean:
Two things before merge, neither a code change: 1. Release gate — confirm 2. Minor spec gap — One note for next time, not actionable here: 🤖 Generated with Claude Code |
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
3 participants
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.
TL;DR — changelog for 4.4.0
polyswarm rules listrenders the hunt-page tracking columns (favorite, rule count, historical hunts, new-results badge + itscounted_atstaleness marker) and hunt output renders frozen-rule provenance.polyswarm rules favorite <rule_id> [--unfavorite]; the server's 5-slot budget refusal is rendered from the documentedFAVORITE_LIMITenvelope.rules listgains--name,--status active,--favorites-only,--has-new-results.live feedgains--livescan-idand--max-results(0 = unbounded);--sincekeeps seconds and its default moves from 1440 s (24 min — a units bug) to 86400 s. Behaviour change: a plainpolyswarm live feednow returns a day of results instead of 24 minutes.polyswarm_api>=4.4.0; the runtimegetattrprobing is gone.Requires
polyswarm-api Release 4.4.0 on PyPI first (the floor must resolve).
artifact-index#1963 on prod for the fields to be populated (additive before that).
The bump PR (
release-4.4.0→develop, "Bump version: 4.3.0 → 4.4.0") merged intodevelopfirst, so this diff carries the version change that triggers the PyPI upload.