Instapaper: parse the article list the API actually sends - #31
Merged
Conversation
The first real sync of a real account was refused with "Instapaper answered
the article list in a shape this bridge does not know", twice, and the bridge
was right: it required an object with a "bookmarks" key, and the live API has
never sent one.
Recorded off the live API (shape only, no content ever logged or committed):
[ {"type":"meta"}, # "delete_ids" only when there are some
{"type":"user", user_id, username, subscription_is_active},
{"type":"bookmark", bookmark_id, hash, url, title, description, time,
progress, progress_timestamp, starred, private_source, tags}, ... ]
A JSON ARRAY of typed objects. No top-level "bookmarks", no "highlights", and
delete_ids rides INSIDE the meta element as a COMMA-SEPARATED STRING.
That last detail is why this is not a one-line fix. engine.py iterated
data["delete_ids"] and kept every part passing isdigit(). Iterating the string
"424242424,424242425" yields characters, all 19 of which are digits, so a fix
that merely stopped demanding a dict would have handed the engine bookmark ids
4, 2, 4, 2 ... and rmtree'd cached articles Instapaper never named. So
parse_delete_ids() owns that parsing, strictly (whole integers only, anything
else dropped and logged), and engine.py now takes only ints so the string path
cannot come back.
normalise_listing() is liberal in the other direction, because this API is old
and its docs are inaccurate: the array and the documented object envelope both
parse, an element with a bookmark_id but no "type" is still a bookmark, and
delete_ids is read wherever it appears. What it will not do is invent a
deletion -- an unrecognised body yields an empty listing, never a delete id.
THE SECOND BUG, fixed regardless: the refusal recorded nothing about what it
refused, so the log could not answer the question it raised. describe_shape()
now logs the top-level type, every key, every value's type, a list's length
and the field set of each distinct element shape. KEYS AND TYPES ONLY --
strings are reported as their length and only a field literally named "type"
is quoted, and only when it is a short lowercase token. This runs against a
real person's reading list; test_listing.py asserts the redaction rather than
trusting it. The non-JSON branch of _json() now logs status, content-type and
byte count for the same reason.
And the reason no suite caught this: tests/fake_instapaper.py answered with
the object the documentation implies, because the fake and the client were
written from the same paragraph. A fake derived from the code's own assumption
cannot falsify it. The fake now mirrors the wire, which turns test_api.py and
test_engine.py into real regression tests for this.
Verified by reverting normalise_listing to the pre-fix "must be a dict" (52
lines changed, confirmed by diff) and watching test_listing.py, test_engine.py
and test_api.py all go red with the owner's exact sentence, then restoring.
Docs: docs/apps/instapaper-plan.md records the observed envelope and rewrites
its "assumptions from documentation" section -- `have` suppression by id:hash
and have-scoped delete_ids are now observations; limit-scoping, the hash's
inputs and the progress-timestamp rule are still assumptions, and the plan
says so. server/read-bridge/README.md gains the full suite list and the rule
that the fake follows the wire, not the docs. check.sh runs test_listing.py.
Triage: not a queued card; opened from a live report.
Dream check: no drift -- this is the bridge doing less guessing, not more.
Same bug class as the parent commit, in the twin path of the same file. A non-200 from bookmarks/get_text carrying no error element told the user "Instapaper could not send this article" and told the log nothing at all -- not the status code, not the body's shape, not even that a body arrived. It now logs the status, the byte count and describe_shape() of whatever came back, under the same keys-and-types redaction as the parent commit. Reachable only by standing in for the transport, which is why no suite had ever been through that branch; test_listing.py now stubs _post and goes through it. Verified by deleting the log call (6 lines changed, confirmed by diff) and watching the two new checks go red, then restoring. Docs: none affected -- the behaviour the docs describe is unchanged, only what the service records about it.
describe_shape named a nested list's length and stopped: an unknown body of
{"results": [...], "cursor": "abc"} logged "results: list[2]" and nothing about
what results held. That is the shape where the answer is one level down -- a
wrapper is the most likely way this API surprises us next, and the log would
have named the wrapper and hidden the payload.
_fields now descends one level into a nested list or object, and the list
element summariser is shared with describe_shape rather than duplicated inside
it. Distinct element shapes with counts, not just the first element: the one
that differs is usually the one that explains the body, and Instapaper's meta
object is element zero exactly once.
Depth stays at one. The redaction is unchanged and still asserted -- the suite
runs the privacy checks over a dict wrapping the full bookmark fixture, so the
descent is exercised against a title and a URL it must not print.
Written red first: the two new checks failed against the previous code
(54 checks, 2 failed) before the descent existed.
Docs: none affected.
# Conflicts: # scripts_local/check.sh
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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
1 participant
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.
The owner's first real sync was refused twice: the bridge required an object with a
bookmarkskey and the live API has never sent one. It sends a JSON ARRAY of typed objects, withdelete_idsinside the meta element as a comma-separated STRING.That detail is why this is not a one-line fix.
engine.pyiterateddata["delete_ids"]keeping every part that passedisdigit(), and iterating "424242424,424242425" yields characters -- so merely dropping the dict requirement would have rmtree'd cached articles Instapaper never named. Parsing now lives inparse_delete_ids(whole integers only).No suite could have caught it: the fake and the client were written from the same paragraph of documentation. The fake now mirrors the wire.
Proven on the deployed container against a live account: 26 bookmarks, 21 articles / 73,454 words fetched, and a second call returning 0 and 0 -- the steady state.
🤖 Generated with Claude Code