Hunt-page ruleset tracking: favorites, rule counts, hunt provenance

[vhmartinezm]

TL;DR

SDK support for the hunt-page ruleset tracking the internal artifact API now serves: favorites with a server-owned budget, ruleset-list filters, per-live-hunt result counts, a per-hunt feed scope, and source-rule provenance on historical hunts. Sync and asyncio clients both; all field parsing is additive (an older server leaves the new attributes None).

Requires

• The internal artifact API change on the same branch name (private repo) — push/merge order: server first (its release publishes :latest), then this PR to develop.
• Render hunt-page ruleset tracking and hunt provenance fields polyswarm-cli#266 renders these fields and merges after this PR.

What's new

Resources:

• YaraRuleset: favorite, favorited_at, rule_count (None means the server had no answer — distinct from 0), historical_hunt_count, new_results_count (only when the list was asked to include counts).
• HistoricalHunt: rule_id (the source ruleset), rule_modified (freeze-time audit value), source_rule_changed — tri-state: has the source ruleset's body changed since the hunt froze it? None = unknown, not "unchanged".
• New YaraRulesetFavorite (the toggle's response: star state + favorites_used/favorites_limit) and LiveHuntResultCounts (counts of {livescan_id, count}; a hunt absent from counts collected 0; livescan_id is the same digit string YaraRuleset.livescan_id now carries — the join key; ids exceed JavaScript's safe-integer range, so they are never bare JSON ints).

Methods (sync + asyncio):

• ruleset_favorite(id, favorite) — idempotent star/unstar; over-budget refusals carry a machine-readable FAVORITE_LIMIT error.
• ruleset_list(name=, status=, favorites_only=, has_new_results=, since=, include_counts=).
• live_results_count(since=).
• live_feed(livescan_id=).

Tests

The two rules live-tests now build a uid-namespaced single-rule ruleset (unique name on the shared stack, deterministic rule_count) and exercise the favorite round-trip, the name/favorites filters, hunt provenance, the counter increment, and the changed-since-freeze flip. Read-after-write assertions poll (replica-lag tolerant; sleeps are free on VCR replay). Cassettes re-recorded against a live stack. The sync client is regenerated via scripts/regenerate_sync.py.

[claude]

Reviewed against AGENTS.md + specs/02-resources.md / 03-endpoints.md / 04-testing.md / 05-downstream-contract.md. Gitflow is clean (base develop, pyproject.toml untouched — correct, the bump belongs to the develop → master step per specs/05), and the surface changes are additive: new kwargs are appended at the end of live_feed, so positional callers are unaffected. Four things need action.
1. No spec update — AGENTS.md requires one in the same PR

Update specs/03-endpoints.md (and any other relevant spec) in the same PR.

Nothing under specs/ is touched. Concretely stale after this PR:

• specs/03-endpoints.md:86-89 and :192 — ruleset_list() is catalogued with an empty signature; ruleset_favorite and live_results_count are absent entirely; the live_feed(since=None, …) row (:189) predates livescan_id.
• specs/02-resources.md — the class-hierarchy tree and the per-domain catalogue have no YaraRulesetFavorite (/hunt/rule/favorite) or LiveHuntResultCounts (/hunt/live/results/count). YaraRulesetFavorite also sets RESOURCE_ID_KEYS = [], a deliberate deviation from the documented convention (the key list routes the identifier to the query string for GET/DELETE/PUT — here it is emptied so id rides in the PUT body instead). That belongs in the spec, not only in a code comment.
• specs/05-downstream-contract.md:150-152 — the 'commonly imported' resource list.
  2. live_results_count / LiveHuntResultCounts ship with zero coverage

No cassette in test/vcr/ contains results/count, include_counts, or livescan_id=. So none of the following is exercised anywhere, live or replayed:

• LiveHuntResultCounts.RESOURCE_ENDPOINT = '/hunt/live/results/count' — a typo in the path ships green.
• since routing to the query string, and the counts / since parse (including the documented counts or [] fallback).
• live_feed(livescan_id=…) — the new feed scope.
• ruleset_list(status=, has_new_results=, since=, include_counts=) — only name= and favorites_only= are recorded.
• YaraRuleset.new_results_count is null in every recorded response, so the 'only present when the list was asked to include counts' path is never observed non-None.

Per specs/04-testing.md these are all e2e-reachable — they want VCR lifecycle coverage in test_rules / test_async_rules, not a follow-up.
3. Missing the pure-unit builder tier for the two new resources

AGENTS.md step 4 asks for the VCR lifecycle test plus pure-unit builder tests asserting the PolyswarmRequest shape. test/known_good_test.py is the pattern. Two things nothing in the repo currently pins:

• YaraRulesetFavorite.update(...) with RESOURCE_ID_KEYS = [] puts id, favorite, and community in the JSON body of a PUT rather than the query string. That is entirely a consequence of core._params (method != GET and key not in param_keys → body); emptying the key list is the only thing holding it.
• favorite serialises as 1/0, not true/false — core.py:549-550 coerces bools to int before they reach the body, and _normalise_bool_params only touches query params. The cassettes confirm the server accepts {"favorite": 1}, but that int-vs-bool body contract is invisible and untested.
  4. The favorite is unstarred in the try body, not in finally

test/client_scan_test.py / test/async_client_test.py: ruleset_favorite(rule.id, False) sits between assertions inside try. If anything in between fails — most likely the favorites_only presence assertion, which reads a list that can lag — the star is never released. The test itself documents that the budget is team-wide and asserts favorites_limit == 5, so a handful of failed runs could wedge the rules tests on the shared e2e stack with FAVORITE_LIMIT until someone unstars by hand. Move the unstar into finally ahead of ruleset_delete, or confirm (and comment) that ruleset_delete releases the star.

Related, minor: assert fav.favorites_limit == 5 pins a server-side config constant, while the line immediately below deliberately bounds rather than pins favorites_used for shared-stack reasons. Worth being consistent about which of the two is a contract.
Minor

• test/eicar.yara is no longer referenced by any test — both call sites moved to uid_yara(uid). Only a doc comment in _e2e_helpers.py:79 mentions it now. Delete it or say why it stays.

[vhmartinezm]

All four addressed in 40bc463: specs 02/03/05 updated in-PR (including the RESOURCE_ID_KEYS=[] rationale and the 1/0 body bools); the live tests now exercise every flagged surface against the real stack and the cassettes record it — live_start → status=active → include_counts observed as a computed 0 → live_results_count (zero-result hunt absent, digit-string keys matching ruleset_get) → livescan_id feed → live_stop-in-finally; pure-unit builder tests added (hunt_tracking_builder_test.py, the known_good pattern); the unstar/slot question is answered in a comment (ruleset_delete soft-deletes and the budget counts deleted=false only, so a failed run self-heals) with the limit-pin-vs-used-bound distinction made explicit; test/eicar.yara deleted.

[claude]

Reviewed against AGENTS.md + specs/01–05. The implementation is sound — builders route as documented (RESOURCE_ID_KEYS = [] → PUT body, confirmed by the recorded body {"id":"71359438369584055","favorite":1,"community":"gamma"}), the sync mirror matches the canonical async source, every new field is an additive .get() parse, the surface change is additive-only (no version bump — correct, bumps belong to develop → master), and the base branch is develop. Four items, all spec/coverage:

1. live_results_count is filed in the wrong classification table (spec drift).
specs/03-endpoints.md:190 puts it under ## Classification — _paginate (returns iterable / async iterable), but both implementations call _single and return a single LiveHuntResultCounts (aio/api.py / api.py). _single vs _paginate is the organizing invariant of that document — as written it tells a caller to iterate a resource object. Move the row to the ### Live hunts _single table alongside live_result.

2. specs/04-testing.md:29 still lists a fixture this PR deletes.

test/eicar.yara, test/malicious — fixture files for upload tests.

The last commit correctly notes nothing references test/eicar.yara any more, but the spec inventory wasn't updated in the same PR. Drop it from that line (and, while there, test/hunt_tracking_builder_test.py is a new module the inventory doesn't mention).

3. The counts entry shape is documented three times and asserted nowhere.
Both cassettes record {"result":{"counts":[],"since":86400},"status":"OK"} (test/vcr/test_rules.vcr:503). So the {livescan_id, count} entry shape and the "digit string, the same join key YaraRuleset.livescan_id carries" claim — stated in resources.py LiveHuntResultCounts.__doc__, specs/02-resources.md:350 and specs/03-endpoints.md:191 — are never exercised, and neither is the content.get('counts') or [] coalescing. hunt_tracking_builder_test.py pins request construction only, not parsing. A pure-unit parse test with a canned non-empty payload (counts: [{"livescan_id": "119…", "count": 3}], plus a counts: null case) would pin all three claims cheaply and needs no stack.

4. livescan_id feed scoping isn't distinguished from an ignored param.
The e2e asserts list(api.live_feed(livescan_id=livescan_id)) == [] against a hunt with zero results — recorded as a 204. A server that dropped livescan_id entirely would produce a different (non-empty) result only if some other hunt had results in the window, which on this fresh-ruleset path it doesn't. So the assertion passes whether or not the filter works; only the query-string shape is actually pinned (test_list_routes_livescan_id_to_the_query_as_digit_string). Same gap for has_new_results, which has no coverage above the builder tier. If a matching submission is too expensive here, that's a fair call — but the test comments read as if the scoping is verified, and it isn't.

5. (minor) FAVORITE_LIMIT is promised but unreachable-by-documentation and untested.
Both docstrings plus specs/02-resources.md:349 and specs/03-endpoints.md:89 advertise "a machine-readable FAVORITE_LIMIT error". Today that surfaces as a generic RequestException/FailedInstanceException whose only machine-readable path is exc.request.errors['code']. Compare KNOWN_GOOD, which got a typed KnownGoodWithheldException, an explicit .sources contract, and specs/05-downstream-contract.md:181 spelling out the raw-envelope fallback. Either mirror that treatment or add one line to specs/05 saying where the code is read from — otherwise "machine-readable" is a promise with no documented API behind it. No test covers the refusal path either.

[vhmartinezm]

All five addressed in b80f825: the counts row moved to the _single Live-hunts table (it returns one resource); specs/04's inventory drops the retired fixture and names the new module; parse-side pins added for the counts entry shape, the digit-string join key and the null-counts coalesce (canned payloads — the cassettes only carry empty counts by construction); the feed assertions' comments now say exactly what they pin (wire shape + empty pass-through — the scoping semantics are pinned by the server's own suite); and FAVORITE_LIMIT's machine-readable contract is documented in specs/05 (no typed exception by design; the path is exc.request.errors with the code plus the same counters a successful toggle returns) and pinned by a respx refusal test, mocked because a genuinely full budget on the shared stack would race every other run.

[claude]

Reviewed against AGENTS.md + specs/01–specs/05. Architecture, gitflow, and spec updates look clean: canonical async edited with a regenerated sync mirror, resources stay pure, _single/_paginate routing is right, specs/02–specs/05 all updated in-PR, no version bump (correct — that belongs to the develop → master step), base is develop, commit messages carry no ticket IDs or private repo names. Builder shapes verified against core._params (empty RESOURCE_ID_KEYS → PUT body; bool → 1/0; *_id → digit string), and both re-recorded cassettes match their tests interaction-for-interaction.

Three things worth acting on, all in tests/docs.

1. The favorite round-trip is not actually pinned on VCR replay. Star and unstar are both PUT /hunt/rule/favorite with no query string — by design, everything rides the body (id / favorite 1|0 / community). The suite uses vcrpy default matchers ([method, scheme, host, port, path, query], per the AGENTS.md convention), so on replay these two requests are indistinguishable and resolve purely by recording order. Reorder the two ruleset_favorite calls, or drop one, and test_rules / test_async_rules still pass against the wrong recorded response — the favorite is True / favorite is False assertions are only load-bearing on a live run. This is the one endpoint in the suite where the body is the request identity; adding body to match_on for these cassettes (or a scoped use_cassette with a body matcher) would make replay assert what the test claims.

2. since unit disagreement across the hunt surface. live_feed documents minutes (src/polyswarm_api/aio/api.py:510), while the new live_results_count and ruleset_list document seconds (src/polyswarm_api/aio/api.py:532 and :159; the tests pass 86400 = 24h). These are all hunt-window params a consumer will wire from one UI control. If the server genuinely differs per endpoint, please state that explicitly in the docstrings and in the specs/03 rows — as written, a CLI author reading the two adjacent methods will pass the wrong magnitude. If it does not differ, one of the docstrings is wrong.

3. Latent: the empty RESOURCE_ID_KEYS applies to every builder on YaraRulesetFavorite, not just update. Only update is used today, so this is inert — but a future favorite get/delete on this class would silently send id in a DELETE body instead of the query string. The specs/02 note explains the why for update; worth half a sentence there that the class is deliberately update-only.

Minor / no action needed: has_new_results is exercised only at the builder tier, and counts is non-empty only in the hand-built parse pin — both are honestly called out in the test comments, and producing a results-bearing second hunt on the shared stack is not worth the flake. ruleset_favorite docstring claims idempotency that nothing double-stars, but the server owns that behaviour.

[claude]

Review

Src side is clean: _params routing is right (RESOURCE_ID_KEYS = [] → {id, favorite, community} in the PUT body, confirmed by both cassettes), LiveHuntResultCounts.get is correctly non-paginated (has_more absent → _single returns the resource), the new livescan_id kwarg is appended after community so no positional caller breaks, all new parses are .get()-additive, and the sync mirror matches what regenerate_sync.py + ruff would emit. Specs 02/03/04/05 were all updated in-PR. No version bump — correct per AGENTS.md §Gitflow / specs/05 invariant 6. Base is develop — correct.

Four things worth action.

1. ruleset_list(has_new_results=…, since=…) never reaches the server.

Every recorded /hunt/rule/list request in both cassettes is one of: ?community=gamma, ?name=<uid>&community=gamma, ?status=active&community=gamma, ?favorites_only=1&community=gamma, ?include_counts=1&community=gamma.

has_new_results and since appear only in hunt_tracking_builder_test.py, which asserts what the SDK sends. A misspelled param name or a wrong unit would be silently ignored server-side and the whole suite would still pass. The "same for has_new_results" note explains why the semantics are not pinned, but not why the params are never transmitted at all.

Both are cheap to add inside the existing running-hunt block, and one is an assertable negative: the hunt has zero results, so has_new_results=True should exclude it — assert rule.id not in {r.id for r in api.ruleset_list(has_new_results=True)}, wrapped in the same NoResultsException guard already used for status='active' after live_stop. And since=86400 can just ride the existing include_counts=True call.

2. since means minutes on live_feed and seconds on the two new surfaces.

live_feed(since=…) is documented "Fetch results from the last since minutes" (src/polyswarm_api/aio/api.py:510); live_results_count(since=…) and ruleset_list(since=…) are documented as seconds. Three since params on the same hunt page, two units. If that is genuinely what the server does, fine — but please confirm, and state the unit in the specs/03-endpoints.md rows for live_results_count / ruleset_list, which currently say "window"/"seconds" without tying either to live_feed. A CLI author reading these side by side will get one wrong.

3. test_favorite_limit_refusal_is_machine_readable is off-tier.

specs/04 invariant 7: "Prefer the pure-unit tier for builder + parse logic … Use this tier for any bug that can be reproduced without network involvement." This test asserts exactly one thing — a 400 envelope populates request.errors and raises RequestException — which is pure parse_response / _raise_for_status behaviour. core_test.py::TestParseResponseErrors already does this shape with _FakeResponse (see the KNOWN_GOOD 404 arm); the same assertion is ~5 lines there, with no respx and no sync-only asymmetry.

If it stays a respx body, invariant 5 requires a new off-harness respx body to "say why in its docstring." The comment argues respx-over-e2e (fair — you cannot hold five team slots on a shared stack) but not why it is sync-only rather than on ClientTestCase. Add that sentence, or move it to core_test.py.

4. Ticket ID in the branch name.

DN-8480-hunting-schema-migration lands in the public merge commit subject (Merge pull request #321 from polyswarm/DN-8480-…). AGENTS.md bans ticket IDs from commit messages / PR titles / descriptions; the commits and title here are clean, so squash-merge with a clean subject (or rename the branch) to finish the job.

Minor. test_rules / test_async_rules now start and stop a live hunt and run up to three 30x1s poll loops, but neither matches anything in _LONG_POLE_FRAGMENTS (test/conftest.py:117), so they schedule into the fast tail of the live -n 8 run. Adding "rules" to the tuple keeps them off the critical path.

[claude]

Review

Checked against AGENTS.md and specs/02-resources.md, 03-endpoints.md, 04-testing.md, 05-downstream-contract.md.

Verdict: the src-side change is clean. All four touched specs are updated in the same PR, the resource/builder plumbing is right (RESOURCE_ID_KEYS = [] genuinely is what routes id / favorite / community into the PUT body — verified against core._params), the FAVORITE_LIMIT path really does land in exc.request.errors (_extract_json_body populates it before _raise_for_status raises the generic 400 RequestException), base is develop, and there is correctly no version bump (spec 05 invariant 6). Everything below is test-side.

---

1. favorites_limit == 5 pins a server product constant into the SDK suite

test/client_scan_test.py:635, plus the same assertion in test_async_rules.

assert fav.favorites_limit == 5

Under TESTS_VCR=off this runs live in e2e CI, so a server-side cap change breaks this suite for a reason that has nothing to do with the SDK contract. The comment calls it a deliberate PIN of "the fixed product cap, no plan scaling" — but that constant lives in the server repo, and nothing here fails if it drifts except this assert. The SDK contract is only "the field is present and is the denominator favorites_used is measured against." Suggest:

assert fav.favorites_limit >= 1
assert 1 <= fav.favorites_used <= fav.favorites_limit

which still pins the meaningful relationship and stops the SDK suite from being a tripwire for someone else's config.

2. The additive-parse invariant is asserted nowhere

Three places claim it — specs/02 ("All additive .get() parses; an older server leaves them None"), specs/05, and the resources.py docstrings — plus the documented tri-states (source_rule_changed=None is "unknown, never unchanged"; rule_id=None for raw-yara hunts; new_results_count=None when the list was not asked for counts). The cassettes only carry a new server, and hunt_tracking_builder_test.py adds parse pins for LiveHuntResultCounts only. Specific missing cases, all cheap and belonging in the new pure-unit file:

• YaraRuleset constructed from a payload carrying none of the tracking keys → favorite is None, rule_count is None, historical_hunt_count is None, new_results_count is None.
• HistoricalHunt on a pre-tracking payload → rule_id is None, rule_modified is None, source_rule_changed is None.
• In the live test, the plain ruleset_list() result already in hand could assert new_results_count is None for the un-counted case — right now only the include_counts=True branch (== 0) is checked, so "None otherwise" is untested.

Without these, a future refactor that changes content.get('rule_count') to content['rule_count'] (which the neighbouring HistoricalHunt.__init__ already does for progress / results_csv_uri) breaks every older-server consumer with a green suite.

3. Cleanup ordering can leak the ruleset on the shared stack

test/client_scan_test.py:700-704 and the async twin:

finally:
    if hunt is not None:
        api.historical_delete(hunt.id)
    api.ruleset_delete(rule.id)

If historical_delete raises, ruleset_delete never runs. That specifically undermines the hygiene argument stated a few lines up — "the finally's ruleset_delete soft-deletes and the server's budget counts only deleted=false rows, so a failed run's star frees itself with the rule." It only frees itself if the delete actually executes. Nest it:

finally:
    try:
        if hunt is not None:
            api.historical_delete(hunt.id)
    finally:
        api.ruleset_delete(rule.id)

4. Minor — the favorite cassette cannot tell star from unstar

Both PUTs to /hunt/rule/favorite are identical under the [method, scheme, host, port, path, query] matcher, because the whole toggle rides the body and the body is not matched. On replay VCR serves them in recorded order, so assert fav.favorite is True / assert unfav.favorite is False pass regardless of what the SDK actually put on the wire. Acceptable as-is (hunt_tracking_builder_test.py is what really pins the body), but worth a comment at the call site: reordering or dropping one of the two toggles silently replays the wrong response rather than failing.

5. Nit — branch name carries an internal ticket ID

DN-8480-hunting-schema-migration. AGENTS.md bans internal refs in "commit messages, PR titles, or PR descriptions" — all three are clean here — but a non-squash merge writes the branch name into public history. Squash-merge with a clean subject, or extend the rule to branch names.

---

Not verified: I could not execute scripts/regenerate_sync.py --check in this environment. The sync mirror reads as a faithful unasync of the async source by inspection; CI's staleness gate is the authority.