Conversation
Reader app lands in this repo as a second plugin; annotations live in plugin state with markdown as a derived export; AI connection config is global while feature toggles are per-instance; Phase 1 waits on a real FreshRSS instance to test against. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
Puts every backend-specific decision in testable JS returning request descriptors, leaving QML with only the side effects -- so the Google Reader protocol can be unit-tested before a server exists to try it on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
Also records that DMS ships no reusable context-menu component -- all 44 DankCommon widgets and none is a menu -- so this is hand-rolled, and a layer-shell menu cannot overflow the widget's own bounds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
Standard and Miniflux backends as plain objects exposing capabilities
plus request descriptors -- { argv, parse } or null, no I/O. All curl
invocation stays in QML, so the protocol logic becomes unit-testable
without a server, which is the only automated verification this repo
can have for a backend.
A factory taking { FeedParser, ReaderState } resolves the cross-module
problem: require() breaks under QML and .import breaks under Node, so
neither module can import the other directly.
Behaviour preserved verbatim, including a pointless Content-Type header
on the bodyless bookmark PUT -- reproduced and documented, not fixed.
Tests: 277 -> 308 passing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
Review found the request-descriptor interface had nowhere to carry the Proc-side timeout. Miniflux calls need 30s explicitly -- deliberately longer than curl's own 25s --max-time -- and losing it in stage 0b would reintroduce spurious failure toasts on slow-but-fine requests. Now a timeoutMs field on the descriptor rather than a fact living only at the QML call site. Also adds tests/qml/run.sh. The design docs claimed QML could not be executed here; it can, headless, given QT_QPA_PLATFORM=offscreen and QML2_IMPORT_PATH. Both fail silently when missing, which is likely how that assumption formed. The first test retires this phase's biggest risk: that passing a QML JS namespace by value into the DI factory works, verified under a real engine rather than argued from docs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
Reading fetchAllFeeds showed the singular fetchRequest leaks: standard issues one request per feed and Miniflux one total, so QML would keep a sourceMode branch purely to decide whether to loop. Also specifies configState to absorb the empty-state branches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
fetchRequest returning one descriptor leaked at the one operation the abstraction most needs to cover: standard issues one request per feed, Miniflux one total, so QML would keep a sourceMode branch purely to decide whether to loop. Plural returns an array both backends fill. Descriptors carry meta so a response can be attributed to a feed without knowing the backend. configState collapses the empty-state branches into one question -- is this backend usable right now -- so the UI switches on the reason rather than on the backend's identity. Reuses the pre-existing ReaderState.activeFeeds for eligibility rather than reimplementing the enabled+url filter. Tests: 308 -> 318, plus the QML harness extended to cover both new functions through a real engine. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
The .pragma check named two files and so stopped covering Backends.js the day it was added. The qmllint job closes the larger gap: QML has had no automated checking at all. Staged under docs/ci/ because workflow writes are hook-blocked. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
The .pragma library check named FeedParser.js and ReaderState.js, so it silently stopped covering the shared modules the day Backends.js was added -- a check with a hardcoded file list has a built-in expiry date. Now globs ./*.js. The qmllint job closes the larger gap: ~3000 lines of QML had no automated checking at all, so a syntax error shipped. qmllint cannot resolve the qs.*/Quickshell imports in CI and warns about them, which is fine -- it exits 0 on warnings and nonzero only on a real parse error. Verified locally that both QML files exit 0 before adding it. Do not mark qmllint a required context until it has reported on a PR: protection lists contexts literally, and one that never reports hangs every PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
Targets the OpenAI-compatible chat API rather than ollama's native one, so vLLM, llama.cpp, LM Studio and LocalAI come free from one client. Verified against the local ollama rather than assumed. Includes measurements: ~4.8s for a two-sentence summary on this GPU, with ~60% of generation spent on reasoning tokens that neither documented suppression knob could disable through ollama's OpenAI shim. That makes model choice the lever, and rules out summarising on render -- summaries must be on-demand and cached. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
Replaces every sourceMode branch in the widget with lookups through backend.*: 17 exact branch sites down to zero, leaving one dispatch point where the backend is chosen. Net -91 lines. fetchAllFeeds now loops over backend.fetchRequests uniformly and attributes responses via each descriptor's meta. runRequest is the sole place a descriptor becomes a process, and honours req.timeoutMs. Read/star call sites call the backend unconditionally -- StandardBackend returns null and the call is a no-op -- so no site asks which backend it has. Empty states switch on configState().reason, using capabilities.serverState only to pick wording. fetchGeneration still increments above any backend work, preserving the shared-generation guarantee when the mode is toggled mid-flight. Per-feed status rows for disabled and url-less feeds are still built in QML, since those produce no descriptor. Deletes minifluxApiCall, fetchMinifluxEntries, minifluxMarkRead, minifluxMarkUnread and minifluxToggleStar, now living in Backends.js. Tests 318 green, QML harness ok, qmllint exit 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
Review of 0b caught a behaviour change the refactor introduced. The old fetchFeed built one curl invocation per feed and indexed positionally, so parseFeed always got that feed's own name. The rewrite matched descriptors to status rows by url, which collapses two enabled feeds sharing a url under different display names onto one descriptor -- so one of them rendered its articles under the other's name. Nothing enforces url uniqueness in settings, so the config is reachable. Descriptors now carry meta.index, the position in the original feeds array, and the caller matches on that. Regression test included. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
One client for ollama, vLLM, llama.cpp, LM Studio and LocalAI, since they all speak /v1/chat/completions -- a provider is a base URL, a model and an optional key, so adding a runtime is a preset row rather than a code path. Same discipline as Backends.js: request descriptors, no I/O, the API key always its own argv element. Reads choices[0].message.content and ignores message.reasoning, which ollama returns separately for reasoning models -- verified live, so no <think>-stripping regex. Emits neither /no_think nor chat_template_kwargs.enable_thinking: both were tested against the live endpoint and neither does anything through ollama's OpenAI shim, and a knob that silently does nothing is worse than no knob. The live test round-trips a real call against localhost:11434 and skips cleanly when nothing answers, so recorded fixtures cannot rot unnoticed. Tests: 318 -> 357. QML harness now covers both modules. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
Three copies of the same id-capitalising expression, per the 0b review. No behaviour change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
The real work is that feed content is untrusted input and this feature turns it into filesystem paths: a title of ../../../.bashrc must not escape the export directory. Specifies the path and YAML escaping rules with a hostile input for each, and the containment re-check that catches what sanitising alone misses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
- Preset provenance comment overclaimed: only ollama is measured; vLLM, llama.cpp and LM Studio are each that project's documented default, taken on faith. Say so plainly rather than flagging one of the three. - Probe used the 60s generation-sized timeout, so an unreachable host hung for a minute before admitting it. A /models GET now gets 8s. - Dropped --max-redirs from the authenticated argv: it is a no-op with no -L and only read as protection that was not there. Documented why -L is deliberately absent -- curl re-sends an explicit -H across a cross-host redirect, which would hand the API key to whatever host the redirect names. - Added the missing hardening-flag tests. The existing argv tests asserted structure only and would not have noticed a dropped --proto or --max-filesize; the review assumed otherwise. Mutation-checked: deleting --max-filesize now fails three tests, previously zero. Tests: 357 -> 362. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
Pure path+markdown builder for the notes providers. Obsidian is not an integration, it is a directory of markdown files, so the markdown-dir provider is the base case and Obsidian and Neovim are configurations of it. The substance is that feed content is attacker-controlled input which this turns into filesystem paths. Every rule from the design doc is implemented and has a hostile-input test: containment is re-checked on the assembled path rather than trusted to sanitising, separators and NUL are stripped rather than rejected so a hostile feed yields a safe note instead of a broken one, and names clamp to 255 BYTES on a codepoint boundary. Verified independently with Node's own path.resolve and Buffer.byteLength across 17 hostile titles: all contained. Four defects found and fixed after delivery, none of which the tests would have caught, because the suite covered the hostile path and not the ordinary one: - Filenames came out Title.md-8c38f0af.md -- the template's extension was sanitised as part of the title and the hash landed after it. The extension is now split off before sanitising. - An empty title rendered the template down to .md and produced the filename md instead of falling back to the item id. - Frontmatter emitted date: always: it read article.dateStr, but the widget's items carry timestamp. Now falls back to it, in UTC. - The source held a LITERAL NUL byte in the strip-NUL regex, which made the whole file read as binary to grep and risks mangling by editors and diff tooling. Escaped, with a test to keep it that way. Tests: 362 -> 411, QML harness now covers all three modules. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
Two real bugs, both pre-existing and both shipped in 2.3.3. Found by running the argv Backends.js generates against a real Miniflux server. 1. Miniflux types entry_ids as int64 and rejects the request outright if any element is a string. minifluxNumericId returns itemId.slice(2), a string, so every mark-read and mark-unread call returned HTTP 400 and read state stayed local-only. Starring was unaffected because it puts the id in the URL path, where the type is never checked -- which is why the bug survived: half the feature worked. 2. curl exits 0 on an HTTP 400, and the caller only reacts to a nonzero exit, so that 400 was discarded with no toast, no log, nothing. --fail-with-body makes a 4xx/5xx exit 22 while still returning the body, so the existing error path fires. The unit tests could not have caught either one: the argv was well-formed and the server rejected it. Adds tests/live-miniflux.js, which runs the module's own argv against a local instance and checks the full round trip -- fetch, mark read, mark unread, star, unstar. It is named .js not .test.js on purpose so CI never runs it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
Protocol probed against a live Miniflux rather than taken from docs, and two traps found that way: stream/contents returns a bare [] with HTTP 200 on Miniflux (success-shaped and empty, so a client built on it gets an empty reader and no clue why), and item ids come in two encodings in the same API. The bigger finding is that Phase 0's interface cannot express this backend: fetch is a chain (token -> ids -> contents), not a set of independent requests. Proposes an additive nextRequest on the parse result, and flags the pending-counter arithmetic as the dangerous part. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
Records that the plugin dir symlinks to the repo, so the checked-out branch is what DMS runs, and why the live-*.js tests are deliberately not *.test.js. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
GoogleReader.js implements the chained protocol behind Phase 0's
interface: ClientLogin -> post token -> items/ids -> items/contents,
expressed with the additive nextRequest on the parse result and capped
at 5 links so a server that always returns one cannot loop.
Four things the live server did that the design doc did not predict,
each handled with a comment citing the probe:
- No status line from plain curl -sS, and a bad-auth body is the bare
text "Unauthorized" with no distinguishing shape, so every argv now
appends -w HTTPSTATUS:%{http_code} and parse splits on the trailer.
- reading-list never drops read items, so the unread view needs an
explicit xt= exclude-tag; starred deliberately omits it.
- The categories array comes back as user/1/state/... with a literal
user id, not the user/-/... form edit-tag takes, so a straight string
match never fires. Matches the suffix instead.
- Enclosure types are application/octet-stream even for images, so the
Miniflux-style mime filter is useless here.
Signatures are normalised across all three backends. GoogleReader needs
a session and a currentlyStarred flag; adding them only there left
Miniflux's markReadRequest(config, ids) binding session to ids for a
positional caller -- no throw, mark-as-read simply stops working, which
is a bug this project has already shipped once. Standard and Miniflux
now declare the unused parameters rather than omitting them.
tests/backend-interface.test.js enforces the arity contract so it cannot
drift again; mutation-checked by reverting one signature, which fails it.
Tests: 411 -> 479. Both live suites pass, 4 QML smoke tests ok.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
Puts the chain arithmetic in a pure ChainRunner.js rather than in QML, because it is the one place a mistake finalises a fetch on partial results and QML cannot be executed. Session stays in memory: it is re-derivable with one ClientLogin and persisting it would write a credential to disk for nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
Moves the chain arithmetic out of the QML runner, where it would have been both the most dangerous code in the phase and the only code with no executable coverage. QML is left with no arithmetic: run a request, hand the parse result to step(), do what it says. step() throws if called after a terminal result rather than replaying one. Replaying would let a double-decrement of the caller's pending counter through unnoticed, which is the precise failure this module exists to prevent -- a fetch cycle finalising on partial results, intermittently, looking like a network fault. Cap mutation-checked: raising it effectively to infinity fails three tests. Tests: 479 -> 499, QML smoke tests 4 -> 5. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
Registers the greader backend, threads an in-memory session through every backend call, and makes fetchDescriptor follow a chain. The pending counter is the part that had to be right: every decrement site is followed by an early return, and the "next" branch returns without touching it, so a chain decrements exactly once regardless of length. Standard and Miniflux never return a nextRequest, so they terminate on the first step and take a path identical to before. chain.step() is wrapped in try/catch. It throws only if called after a terminal result, which should be structurally impossible here, but an uncaught exception in a running widget would leave isLoading stuck true forever -- so it is treated as a terminal error, decremented once, and logged. Loud in development, safe on a desktop. toggleBookmark reads bookmarkMap BEFORE applying the local toggle: edit-tag has no set endpoint, only add/remove, so reading after would report the new state and star where it meant to unstar. minifluxNumericId becomes backendItemId and strips r: as well as m:. 499 tests, 5 QML smoke tests, qmllint clean, both live suites pass, sourceMode branches still 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
Both found while testing a deliberately bad feed URL, which showed "Not fetched yet" forever. The fetch was fine -- the state file held the right row, error, "curl exit 6". The settings panel keeps its own SNAPSHOT of the status list and re-read it only when the panel opened, so a feed added while the panel was already open never got a status, and !st renders as "Not fetched yet". It now polls every 3s while visible; the state tier offers no change notification, so reacting is not an option. Separately, "curl exit 6" is accurate and useless to the person reading it. classifyFetch now maps the codes a user can act on -- could not resolve host, could not connect, timed out, certificate could not be verified, server returned an error -- keeping the numeric code in parentheses so a bug report stays diagnosable. 124 stays its own timeout state rather than folding into the curl mapping. Tests: 499 -> 502. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
The job passed locally and failed in CI. Not a code difference -- a qmllint version difference. The DMS shell is not installable in CI, so qs.Common/qs.Widgets/Quickshell and everything reached through them are unresolvable, and every semantic finding cascades from that one missing dependency: Theme unqualified, StyledText unknown, and by extension Rectangle appearing not to support color. On Qt 6.11 those are warnings and it exits 0; on Ubuntu's older Qt they are fatal. Disables the categories that depend on the missing modules, and probes --help first so only flags this binary supports are passed -- those have come and gone across releases and an unknown option is itself fatal, which would reproduce the failure by a new route. That narrows the check to syntax, which is a smaller claim than the first version made and still worth having: a QML syntax error ships silently and breaks the widget at load. Verified both ways -- clean on every .qml file, exit 255 on a deliberately malformed one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
Second qmllint failure. It is a type checker and the types live in the DMS shell, which cannot be installed on a runner -- Ubuntu's package ships the binary without the QML module tree, so it cannot load its own builtins, and the category flags from the last attempt were not honoured either. qmlformat parses without resolving anything, needs no module tree, and ships in the same package. It proves the file is syntactically valid QML, which is the only thing qmllint could ever have proven here. The job self-tests: it feeds qmlformat a malformed file first and fails if that is accepted. A syntax checker that cannot fail is worse than none, and both previous attempts would have gone unnoticed had they failed open rather than closed. Renamed qmllint -> qml-syntax, since it no longer claims to lint. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
The previous two commits staged this under docs/ci/ but never committed the workflow itself, so both pushes shipped the old qmllint job and CI kept failing for a reason already fixed locally. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
The real problem was accuracy, not length. The README listed two fixed bugs as current limitations -- the search second-click bug and the unverified Miniflux settings layout -- claimed 240 tests when there are 502, and described 2 shared modules when there are 7. README, 443 -> 327 lines: - Features cut from 27 bullets to 14 grouped ones; the removed ones were commit messages, not features. - Deleted the 30-row Test Coverage table. It was already wrong three ways (stale count, duplicated filterItems and search rows, six modules missing) and a hand-maintained inventory of tests only drifts further. Replaced with the three test tiers and why each exists, which stays true as tests are added. - Merged the overlapping Local development and Testing sections; dropped the Planned list that duplicated the Roadmap; moved Known limitations out from under Miniflux mode, where it was nested by accident; removed a duplicated screenshot section and bookmark paragraph. - Architecture now lists all seven modules and states two rules that were nowhere written down: modules never require() each other, since neither require nor .import works in both runtimes, and every backend shares one positional signature. docs/plans: a status line on each of the eleven docs saying whether it still describes reality, corrections where a claim had become false (fetchRequest singular, "QML cannot be executed", the overturned S10 pruning decision), and a README index so someone landing there can tell design from archaeology. docs/ci/README.md no longer describes an already-applied change as pending. 502 tests, 5 QML smoke tests, all internal links resolve, manifest check still finds its changelog heading. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
Adds the third source mode, and converts the settings panel's ~40 sourceMode string comparisons to capability lookups -- the same change Phase 0 made in the widget, still outstanding here because settings was scoped out. A third mode makes it actively wrong: every === "standard" that means "fetches feeds itself" silently excludes Google Reader. Connection sections stay keyed on the mode deliberately: a credential form is inherently backend-specific, and a capability flag per credential shape would be a worse abstraction than the string it replaced. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
1194 -> 1071 comment lines. A modest cut, and the wrong headline: the five module headers collapsed hard (GoogleReader 76 -> 24, ChainRunner 52 -> 24, Backends 29 -> 15) because they each re-explained the same four cross-file rules, which README's Architecture section now states once. The file bodies barely moved, because they are dense with findings rather than boilerplate, and cutting those was never the goal. Two improvements matter more than the count: Findings moved to their point of use. The stream/contents-returns-[] trap now sits above the function that avoids it, the T=-or-401 rule above the token request, the chain-cap arithmetic on MAX_CHAIN_LINKS, and the double-decrement invariant inside step(). A trap explained where it bites beats the same words in a header nobody scrolls to. Opaque citations replaced with the actual rule. Roughly twenty tags -- v2.4 §2.6, S10, D8, Risk #3, CONTRACT 6 -- pointed at design docs now marked historical. A tag is not documentation. tests/comment-findings.test.js asserts that 25 hard-won facts are still written down somewhere, deliberately not caring where or in what words. A tidy-up pass is exactly when context that cost hours gets deleted for looking like verbose prose. It earned its place immediately: the 255-BYTE filename clamp turned out never to have been documented at all -- the comment said 'maxBytes' and cited 'Rule 6'. Comment-only: the sole non-comment line in the diff is a trailing comment on a closing brace. 527 tests, 5 QML smoke tests, qmllint and qmlformat clean on both QML files, live Miniflux suite passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
Adds the third source mode with its Connection section (server URL, username, password, Test Connection), writing the greaderUrl/ greaderUsername/greaderPassword keys the widget already reads. The field descriptions say plainly that these are Miniflux's separate Google Reader integration credentials, not the web login -- entering the web password otherwise yields a bare 401 with nothing to explain it. Test Connection runs the real chain (ClientLogin, then user-info) rather than a reachability ping: ClientLogin succeeding only proves the server is up, not that the credentials work. Visibility for behavioural questions now asks the backend instead of comparing strings: 17 mode comparisons down to 13. The 13 survivors are all Connection-section fields and are the intended end state -- a credential form is inherently backend-specific. Also guards a fragility this stage introduced. The settings panel calls three GoogleReader.js functions that are top-level, so QML sees them, but are not in module.exports, so no Node test does. A rename during a refactor would break Test Connection at runtime with the suite still green. tests/comment-findings.test.js now asserts that surface by reading the source -- require() would miss it by construction. Mutation-checked by renaming one, which fails it. Two corrections to the design doc, recorded rather than quietly edited: the '~40 comparisons' figure was an estimate and wrong (17), and a table row referenced a sort visibility check that does not exist. Tests: 527 -> 535. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
The difficulty is focus retention, not the bindings. acceptsKeyboardFocus currently keys on hover, which was enough for search -- the pointer is still over the widget while typing -- but breaks the moment someone clicks a row and moves the mouse away to read: the surface stops being focus-eligible mid-navigation and the next j goes to the compositor, intermittently, depending on where the pointer rests. Latching on the focus scope's activeFocus instead holds it for as long as the user is driving. Key dispatch goes in a pure KeyMap.js -- the gg pending state, the layered Esc, and the -1 cursor rules are real logic and testable without a shell. Records that DankCommon's FocusRing is not usable here: it binds visible: parent.activeFocus, which is per-item Qt focus, the wrong model when focus lives on the surface and the cursor is an index. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
resolveKey(event, state) -> { action, index, pending }. Actions are
names the caller performs, so the gg pending state, the layered Esc and
the cursor rules are testable without a shell. Qt key codes are declared
as constants rather than requiring a Qt global, so the module works in
both runtimes.
Corrects the design doc, which said that at cursor -1 'j moves to 0 and
every other action is a no-op'. Read literally -- as it was implemented
first -- that also blocks /, r, A and G, so a freshly-clicked widget
ignores almost every key you press, and there is no keyboard route into
a list whose cursor has never been set. Only ROW actions are blocked
now; G and g g place a cursor rather than acting on one, and the
cursor-independent actions were never in question. The doc is corrected
rather than the code bent to match it.
Tests: 535 -> 604, QML smoke tests 5 -> 6.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
Every decision follows from one measurement: ~4.8s per summary on this GPU. That rules out anything automatic -- on demand only, cached against the item id, with a generation counter so a late result cannot render against the article the user has since moved to. The cache is bounded lower than the id lists on purpose: those store ids, this stores paragraphs, and the state file is rewritten on change. Errors show on the row that asked, never as toasts. A widget that toasts whenever a local model is unreachable is unusable on a laptop that only sometimes runs one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
Wires KeyMap.js into the widget. The load-bearing change is focus retention: acceptsKeyboardFocus now includes keyboardScope.activeFocus, because hover alone was enough for search -- the pointer stays over the widget while typing -- but breaks the moment someone clicks a row and moves the mouse away to read. Latching on the focus scope keeps focus for as long as the user is driving, and is not a new way to GRAB focus, only to keep what a click already granted. Three functions were extracted so the keyboard and mouse paths cannot drift: openItem, toggleReadSynced, closeSearch. Everything else calls the existing handlers. The cursor is drawn as a border rather than another fill, so it is distinguishable from the hover tint, and it clamps rather than resets when a filter shrinks the model. Also adds FreshRSS as a second live target, which is the point of the Google Reader work: the API is a protocol, not a product. tests/live-greader.js now takes a target argument. Two divergences found immediately, neither a bug in this client: - Miniflux returns a bare [] with HTTP 200 for every stream/contents variant; FreshRSS implements it properly. The two-step items/ids -> items/contents flow this client uses is the only one both support, so that choice is now vindicated rather than assumed. - The T= post token is REQUIRED by Miniflux (401 without it) and OPTIONAL on FreshRSS (200). A client developed only against FreshRSS would omit it, work perfectly, then fail on Miniflux with a bare 401. The assertion is now per server rather than pretending one behaviour is universal. 15 of 16 live checks passed against FreshRSS on the first run; the 16th was that assertion. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
Esc did not close search while the cursor was in the field. KeyMap handled the case correctly; the event never arrived, because DankTextField holds Qt focus and the FocusScope's handler never sees it. Handled on the field itself, returning focus to the list so j/k work without another click. The selection bar's Mark read was not state-aware, so a user who selected already-read items had no way to unread them. It now flips like the header's mark-all control, reusing ReaderState.removeAllRead and the existing batched server push. Its comment claimed 'additive only, never marks unread' -- a deliberate earlier decision that does not survive someone selecting read items. Added a ? overlay listing every binding. This was the most useful thing in the testing report: every key worked, and discoverability was zero. The user found j/k/Enter// by experiment and asked for a way to mark read or save -- m and s already did exactly that, and nothing said so. A keyboard interface with no way to ask what the keys are is usable only by whoever wrote it. DankKeycap resolves through the already-imported qs.Widgets, so the glyphs are the shell's own. Esc closes the overlay ahead of resolveKey's search/selection/cursor layering, since the overlay is the topmost thing on screen. Tests: 604 -> 610. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
Google Reader API support and keyboard navigation, plus six user-visible fixes -- three of which shipped broken in 2.3.3, most seriously that mark-as-read never reached Miniflux at all. Also updates the manifest description, which still described the widget as RSS plus Miniflux. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
Keys only worked after clicking an article. keyboardScope is a FocusScope around the ListView and only a row click focused it, so clicking the header, the chips or empty space left every key dead. A TapHandler on the root now grants focus on any click, skipped when searchField already has it -- a TextInput takes focus on press while onTapped fires on release, so that guard reads settled state rather than racing it. m and s now act on the whole selection when one exists, which is the model Space implies: gather, then act. Decided in KeyMap via hasSelection and returned as distinct actions, so the widget routes rather than re-deciding, and the read variant inherits the button's state-awareness -- an all-read selection flips to unread. Tab and the j/k cursor were two unsynced focus systems, and Tab could walk out of the FocusScope entirely, after which j/k went dead with no way back but a click. Bidirectional sync turned out to be unsafe for a concrete reason rather than a vague one: DankActionButton sets activeFocusOnTab and its own Keys.onPressed consumes Space, Return and Enter with event.accepted = true. Tabbing onto a row control would therefore hijack the very keys that select and open, which is the stranding as experienced. The three row controls now set activeFocusOnTab: false, leaving exactly one cursor concept. They stay reachable by mouse, and by m/s/Space on the cursor row. Tests: 610 -> 616. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
The last fix covered only the three row controls. Refresh, the search toggle, both close buttons and the search field were still Tab stops, and they sit outside keyboardScope -- so Tab still walked focus out of the key handler and j/k still went dead, which is exactly the symptom that was reported again. The widget now has no Tab stops at all. One cursor: j/k moves it, '/' reaches search, Esc leaves. Controls stay reachable by mouse and by m/s/Space on the cursor row. Adds a test that enumerates every tabbable type in the QML and fails if one lacks activeFocusOnTab: false. This invariant is easy to break -- adding a button is not obviously a keyboard-navigation change -- and breaks it in a way nothing else catches, surfacing only as 'the keyboard randomly stops working'. Mutation-checked by deleting one, which fails it with the line number. Tests: 616 -> 624. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRyF2uQvkQkcjbGHBsa7nS
BrendonJL
marked this pull request as ready for review
September 11, 2026 10:24
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
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.
Releases 2.4.0: Google Reader API support, keyboard navigation, and six user-visible fixes — three of which shipped broken in 2.3.3.
41 commits, 49 files, ~8.4k insertions. Large, but it splits cleanly into a refactor, two features, and the fixes that refactor made findable.
User-visible
Google Reader API support — adds FreshRSS, Tiny Tiny RSS (via its plugin), Inoreader, TheOldReader and BazQux as sources. One protocol rather than one integration each. Verified end to end against both Miniflux and FreshRSS.
Keyboard navigation —
j/kmove,o/Enteropen,mtoggles read,ssaves,Spaceselects,g g/Gjump,/searches,rrefreshes,Amarks all read,?lists the bindings.mandsact on the whole selection when one exists.Escunwinds one layer per press: help, then search, then selection, then cursor.Fixed — mark-as-read never reached Miniflux. Every read made in Miniflux mode stayed local. The API types
entry_idsasint64; the widget sent strings, so the server rejected the request with HTTP 400. Starring was unaffected because it puts the id in the URL path, where the type is never checked — which is why it survived: half the feature worked.Fixed — Miniflux API errors were silently discarded.
curlexits 0 on an HTTP 400 and the widget only reacted to a non-zero exit, so failures vanished without a toast or log line. This is what hid the bug above. Now uses--fail-with-body.Fixed — stale feed status in settings; search needing two clicks before it accepted typing; the search button disappearing during selection; selection being silently dropped by a search; two feeds sharing a URL rendering each other's names;
curl exit 6instead of "Could not resolve host".Internal
The widget no longer branches on the source mode anywhere — 17
sourceMode ===checks became one dispatch point. Backends are plain objects exposingcapabilitiesand returning request descriptors ({ argv, parse, timeoutMs, meta }) that perform no I/O; QML runs the process and owns nothing else. Adding Google Reader was a new file rather than a new branch in twenty places.Two constraints worth knowing before editing any module (both in README → Architecture):
.pragma library. It is not valid JavaScript and breaksrequire()in the tests. CI greps for it.require()each other. Neitherrequirenor.importworks in both QML and Node, so siblings are passed in:createBackends({ FeedParser, ReaderState, GoogleReader }).Testing
240 → 624 tests, plus a QML smoke harness and two live suites.
The interesting tier is
tests/live-*.js, which run the modules' own generatedargvagainst real Miniflux and FreshRSS servers. They are deliberately not*.test.js, so CI never runs them. Both Miniflux bugs above were invisible to the unit tests — the requests were well-formed and the server rejected them. That is the gap this tier exists to close.tests/qml/run.shruns QML headless. This repo previously assumed that was impossible; it needsQT_QPA_PLATFORM=offscreenandQML2_IMPORT_PATH, and without the second it fails silently with "Did not load any objects".CI gained a QML syntax check via
qmlformat. Not qmllint — it is a type checker and the DMS shell it would need cannot be installed on a runner (docs/ci/README.mdrecords both failed attempts).Reviewing this
It is large, so in rough order of value:
Backends.js+GoogleReader.js— the interface and its hardest implementation.ChainRunner.js— small, and the one place a mistake finalises a fetch on partial results.DankRssWidget.qml'sfetchAllFeeds/runChainLink— the only untested code that matters; it has no automated coverage at all.Design docs are in
docs/plans/, indexed indocs/plans/README.mdwith a status line saying which still describe reality.Known limitation: Google Reader has only been tested against Miniflux and FreshRSS. Those two already disagree — Miniflux returns a bare
[]with HTTP 200 forstream/contentswhile FreshRSS implements it, and theT=post token is required by Miniflux but optional on FreshRSS. Other servers may deviate again.