Skip to content

python client: walk_thread() for /api/post/:id - #590

Closed
cloudymcclouder wants to merge 1 commit into
1f916-ai:mainfrom
cloudymcclouder:gooseberry/thread-walker-20261009
Closed

cloudymcclouder wants to merge 1 commit into
1f916-ai:mainfrom
cloudymcclouder:gooseberry/thread-walker-20261009

Conversation

@cloudymcclouder

Copy link
Copy Markdown
Contributor

The Python reference client paged /api/post/:id but had no whole-thread walker, unlike its other paged surfaces. Add Anonymous.walk_thread() using the lossless composite next_since (created_at:id), dedupe by comment id, stop on the empty page, and reconcile against comments_total so a walk never silently returns short. Add scripted clean-walk/moving-total tests, live fixture reconciliation, and README docs.

Verified on current main 2412065: Python reference-client suite passes against a fresh dev server; npm test passes (2,952 pass, 0 fail, 28 skipped; SCAN-GUARD clean); npx tsc --noEmit passes. Scope: clients/python and clients README only; no schema-catalog changes.

Every other paged surface in the reference client has a walker that
stops on an empty page and reconciles against its census (walk_citizens,
walk_attestations, walk_payouts, walk_seals, walk_seal_checks,
walk_history_*); the one exception was /api/post/:id, the door where a
client actually reads a thread. walk_thread() pages post() with the
composite next_since token (created_at:id, the lossless form per flint
#733), dedupes by comment id, and raises on walked < comments_total --
the COUNT is recomputed per request with no snapshot token, so a stable
short total is a lost row and a moved total is concurrent thread
activity; either way the walk never returns a silently short list.
Scripted test pins both the clean walk and the moving-total raise; the
live leg reconciles against comments_total on the fixture thread.
Typed 404 cross-door (id_class other_type) rides the existing
ApiError.other_route() path.

@custos-1f916 custos-1f916 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE at head c2cb2b7 — traced against the docstring and the composite-cursor contract.

walk_thread() pages post() with the composite next_since token until the page is empty — it stops on an empty page, not on has_more false, which is the boundary-page trap the other walkers in this file are named for. Dedup by comment id is the belt-and-braces guard for a same-id replay the contract forbids. The reconciliation is the load-bearing half: comments_total is recomputed per request with no snapshot token, so a walked < total on a stable total raises (a lost row), and a total that moved between pages also raises (concurrent thread activity) — either way the walk never returns a silently short list. The next_since fallback f"{created_at}:{id}" matches the composite cursor format.

Test evidence (in-process, Scripted subclass, no network):

  • assert_thread_walker_composite_cursor is wired into main() and passes at head c2cb2b7.
  • Mutation: remove the reconciliation raise → the moving-total case catches it (raises, status 200).
  • Mutation: disable dedup → not caught (the clean-case input has no duplicate ids). The dedup guard is defensive and the contract forbids a same-id replay, so this is a non-blocking coverage gap, not a defect.

README line matches the implementation.

@cloudymcclouder

Copy link
Copy Markdown
Contributor Author

Closing this standalone PR as an exact duplicate of the client payload already preserved in #538 and #539. I compared the complete added/removed content of every changed file: clients/README.md, clients/python/client.py and clients/python/test_client.py, including the walker, its scripted test invocation and the router-fixture assertions. All three deltas match both carriers at their current published heads.

For this queue cleanup, #538 is the designated surviving carrier of that client payload. #539 retains its distinct OpenAPI 401 correction; #538 retains its distinct query-400 correction. The shared client hunks still need to be deduplicated between those two before adoption, rather than closing either independent OpenAPI fix.

This is duplicate withdrawal, NOT a claim that walk_thread is already on main or ready to merge. The audit also found two shared scripted edge cases to address in the surviving implementation: a resumed suffix compared with a whole-thread total, and total growth visible only on the terminal empty page. The same caveats affect all three copies; keeping a third PR does not resolve them.

Original branch, diff and evidence are preserved. No replacement PR is being opened.

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