The app's first streaming client: sse.js reads the /api/chat POST body
with fetch + a stream reader (apiCall's 60s JSON path cannot stream),
parsing SSE events with abort support. AiChatContext holds the
conversation (browser-only, per design) so it survives visiting the
Manage tab; switching modes starts a fresh conversation. The Chat tab
has the three-button mode picker, per-mode suggestion chips, streamed
answers with clickable relative WROLPi links, tool-activity indicators,
a slow-hardware banner, and a "not ready" state pointing at Manage.
ChatMessage states are in the theme-sample gallery.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
New AI nav item and /ai route with Chat and Manage tabs (Chat is a
placeholder until the SSE chat lands). The Manage tab mirrors the
Map/Zim download UX: the model catalog with the RAM-based tier
recommendation and a Pi 4 performance hint, downloads through the
download manager (progress on the Downloads page), active-model
select, enable toggle, idle-unload minutes, and llama-server
start/stop through the Controller (compose service name in docker,
systemd unit natively). AiModelRow is in the theme-sample gallery.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Tool definitions are generated in-process from the /api/ai blueprint's
own OpenAPI operations (operationId = tool name, description = tool
description, dataclass bodies + declared parameters = one flat JSON
schema) — the spec IS the catalog, nothing to drift. Manage endpoints
never become tools. A mode is a server-enforced system prompt + tool
allowlist (Help / Research / System per ai.md); the executor can only
reach allowlisted /api/ai endpoints, path parameters are encoded and
traversal-rejected so a crafted argument cannot escape an endpoint.
POST /api/chat streams SSE (status/token/tool_call/tool_result/done/
error), starts llama-server on demand, validates tool args server-side
with instructive errors the model can retry on, caps tool results with
a paging hint, and bounds the loop by ai.yaml max_tool_calls plus a
hard timeout. Caddy passes the stream unbuffered (flush_interval -1)
in both deployments.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
install_llama_cpp.sh builds llama-server from the SHA-pinned v0.3.0
source tarball (single source of truth: on-device upgrade, Pi image,
Live ISO, and the new docker/llm container all call it; version-marker
makes re-runs no-ops). start_llama_server.sh refuses to start unless
ai.yaml enables AI and the active model file exists — which also covers
unmounted media. wrolpi-llm.service is installed but deliberately
never enabled at boot: the Controller starts it on demand (registered
in managed_services; the llm compose service follows the samba
pattern), and the API's perpetual idle-unload worker asks the
Controller to stop it after the configured idle minutes. llama-server
is localhost-only natively, internal-network-only in docker, and never
proxied by Caddy.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ai.yaml is the source of truth (opt-in: enabled=False by default) with
active model, idle-unload minutes, context override, and tool-call
bound. The model catalog follows the map pattern: a bundled constant
keeps the Manage tab working offline, the CDN manifest (ai/models.json
+ detached GPG signature) wins when reachable. GGUF downloads clone
the map-extract flow — the per-file meta4 is GPG-verified once and the
same bytes handed to aria2c, so the verified hash is the enforced one;
unverified downloads are refused. Models land in <media>/ai/models/
(gated by the existing media_mounted download rules). Manage endpoints
(catalog with RAM-tier recommendation, settings) live under
/api/ai/manage/ which the future agent loop excludes from its tool
catalog.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Search the wrolpi-help markdown in-process: files are parsed on demand
and cached by mtime; title matches outrank headings outrank body. No
database table — the corpus is dozens of small files. Pages are read
as paged markdown. The docker api container gets a read-only bind
mount of the help docs at the native /opt/wrolpi-help location; a
missing help directory degrades to empty results with a message.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Proxy the Controller under /api/ai so the model sees one API: an
aggregate /status to start every diagnosis, lean service list, recent
service logs (lines clamped), and disks + SMART. One controller_client
helper resolves the Controller base URL (docker service name vs
localhost). Every proxy degrades to an `errors` entry when the
Controller is unreachable — never a 500.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Lean, flat, LLM-shaped read-only endpoints over the library: global,
video, archive, and doc search; paged captions, archive text, and zim
entries; collections, inventories, and a media-directory text reader.
Links are server-computed; big content is paged, never truncated.
This ports the formatting layer from wrolpi_mcp (untouched for now)
into tested code. First phase of ai.md.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Completed once-downloads are never written to the downloads config, and
importing the config (which runs on every startup) deleted any DB Download
the config did not list. That erased the last_download_attempt rows the
per-domain and global daily limits count from, so a restart reset the count.
Keep completed once-downloads on import; pending/failed and recurring
downloads still follow the config as the source of truth.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* Fix API stalls: non-blocking switch worker, threaded searches, indexed Videos page
Three response-time fixes found by benchmarking the read-only API endpoints
hot and cold on real libraries:
* `switch_worker` runs on the Sanic worker's event loop every 0.1s and called
the blocking `multiprocessing.Event.wait(timeout=1)`. With no switch pending
(the normal state) this froze the loop for a second at a time; about a third
of all requests, including trivial ones, took ~1,000 ms. It now polls
`is_set()` and returns immediately.
* The search handlers (videos, files, archives, docs, zims) and `/api/statistics`
ran synchronous SQLite queries on the event loop, so one slow search stalled
every other request on that worker. They now run via `asyncio.to_thread`.
* The unfiltered Videos page (`POST /api/videos/search` with only an order)
used `mimetype LIKE 'video/%' OR ...` plus `COUNT(*) OVER()`, which visits
every video's index entry before LIMIT applies: seconds cold on a large
library on a spinning disk, and ~10 s hot for a deep page. It now seeks each
exact mimetype on the (mimetype, effective_datetime) index for `limit + offset`
rows and merges them, with the total from a separate count cached for 60 s.
Results are identical to the generic query.
Also adds `scripts/api_perf.py` (times every read-only endpoint against a host,
optionally restarting the API over SSH for cold numbers) and
`scripts/api_perf_compare.py` (merges per-host results). Host profiles live in a
gitignored JSON file; an example is included.
Measured on an 8-core docker host with a 10 GB library: median hot request
1,007 ms -> 22 ms; Videos page 1 hot 287 -> 108 ms, page 50 1,345 -> 120 ms.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Serve the video description from its own endpoint instead of Video.__json__
`Video.__json__` computed `description` as `c_text or get_video_description()`,
where the fallback opens and parses the video's info json from disk. The list
endpoints serialize every result with `__json__`, so any page containing videos
without an indexed description (common: many videos have none) read their info
json files on every request, cold, for a field the list never displays. The
description is not part of search either: search runs against the FTS copy of
`c_text`, so the fallback never fed the index.
Drop `description` from `__json__` and serve it from
`GET /api/videos/<id>/description`, like comments and captions. The endpoint
returns the indexed `c_text`, falling back to the info json only there, for a
row indexed before descriptions were stored. The player page fetches it through
`useVideoExtras` alongside the comments.
Tests assert the list and detail responses never touch the info json, and cover
the endpoint's indexed, fallback, and not-found cases.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Address review: thread-local zim sessions, fast-path equivalence test, comments
* Zim search, single-zim search, and search estimates run libzim off the event
loop, each opening its own session inside the worker thread rather than
passing the request's SQLAlchemy Session across threads.
* The Videos fast-path test now runs the same requests through the generic
query and asserts identical rows and paging for both date orders. The one
intentional difference is documented: past the end, the fast path still
reports the true total.
* switch_worker returns before its try block so the empty path does not clear
the event; its comment states the invariant only. The copied "synchronous
SQLite" comments are reduced to one line in one place.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* videos: a collaboration video's channel directory conflict killed the download
YouTube collaboration videos appear in every collaborator's feed but their
metadata names only the primary uploader's channel. When that channel's
computed directory already belonged to an existing Channel that no longer
matched by source_id/url/name, channel creation raised
ChannelDirectoryConflict and the download failed unrecoverably. Now the
channel that owns the directory is used instead of creating a conflicting one.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* review: trim comments that restated the code
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
NAME_PARSER's suffix whitelist had no audio suffixes (and no mkv/m4v),
so a correctly-named audio file got no published date and sorted to the
very end of its Channel -- which reads as missing. The whitelist now
carries the media suffixes a Video FileGroup can lead with.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* videos: an uploaded audio file never appeared in its Channel
The upload flow asks Video.can_model, which only accepted video/*, so an
uploaded .flac was indexed with no Video row -- and the batch modeler's
`indexed != True` gate then excluded it on every later refresh, leaving
the file permanently invisible in its Channel.
can_model now accepts audio/* like the batch modeler always has, and the
modeler gates on the Video row being missing (mirroring the doc_modeler
fix for the same class of bug) so already-stuck files heal on the next
refresh. Audio playlists (.m3u detects as audio/x-mpegurl) are excluded
from both paths and unmodeled by video_cleanup.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* review: widen the playlist denylist, prove modeler termination
The denylist gains audio/x-scpls and audio/scpls -- .pls detection
varies by libmagic version, and any audio/* playlist type must be
refused wherever it comes back. Modeling one file is extracted as
_model_video so a test can force persistent failure and prove the loop
terminates; failed_ids is recorded before the PYTEST re-raise so no
failure path can re-select an id forever. Playlist mimetypes are bound
through json_each like the rest of video_cleanup, and the comments
state the invariants instead of the history.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test: the playlist test must not depend on libmagic's m3u opinion
CI's libmagic detects .m3u as text/plain where a Mac's says
audio/x-mpegurl. The invariant is that a playlist mimetype is never
modeled by either path, so the test forces the mimetype like the other
denylist tests instead of asserting what detection returned.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* common: config writes must be atomic and version-checked under the save lock
A config file must never be observable in a truncated state: writes go to a
temporary file in the same directory and are renamed into place. The version
check in ConfigFile.save() reads the config file, so it must hold
config_save_lock like the write does.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* collections: item writes must begin as SQLite writers
The add/remove/reorder item endpoints read before their first write; a
deferred transaction upgrading at that write fails instantly with "database
is locked" when any other connection wrote in between, bypassing
busy_timeout. Open their sessions with get_db_session(commit=True) so the
transaction takes the write lock at BEGIN, matching post_download.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* review: error responses must not commit the write session; trim comments
Map exceptions to 4xx outside the get_db_session(commit=True) block so an
error response never commits, and serialize inside the session. Close the
mkstemp descriptor when fdopen never took ownership. Trim comments and test
docstrings to what the code and assertions actually cover.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* app: the navbar corner wrapped onto a second row on small phones
Every status-icon slot carried a 1.5em separator margin -- ~80px across
three slots -- which wrapped the mobile bar's whole corner onto a second
row on any screen narrower than about 380px. The margin is now 0.25em on
both bars: the corner's gap and the slots' own padding still space the
icons, and on desktop the freed room just goes back to the links that
useOverflowNav folds into "More".
The mobile bar is extracted as MobileNav (mirroring DesktopNav) so a
Cypress suite can mount it and hold it to one row at phone widths.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* review: prove 320px with a faithful fixture, trim the CSS narration
The suite claimed 360px covered narrower devices because the fixture was
wider than production, but the fixture also lacked the real theme
toggle's 0.8em right margin, so the safety margin was overstated. The
fixture now matches a phone -- 32px logo home link, the toggle's margin,
and a coarse pointer so the search link drops its keyboard hint, with a
test asserting the hint is really absent -- and 320px is measured
directly.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
The channel downloader refreshes a Channel's files before downloading,
already passing send_events=False, but the parameter was never plumbed
into the file worker, so every pre-download refresh notified the user.
FileTask now carries send_events and handle_refresh skips the
Refreshing/Refreshed events when it is False. User-requested refreshes
and global refreshes still send Events.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* e2e: exercise the FileBrowser footer buttons against real files
Sixteen Cypress tests covering Rename, Move, Delete (including the
tagged-file force-delete flow), Ignore/Unignore, Upload, and scoped
Refresh, with no API mocking: fb:* tasks seed a cypress-fb/ sandbox
inside the media directory, the UI mutates it through the real API, and
assertions check the disk as well as the table. Skipped in CI, which
runs only the React dev server.
Background jobs (move, refresh) only queue work and worker_status reads
idle while jobs wait, so cy.refreshFiles / cy.waitForEvent poll the
events feed for completion events newer than the server's clock, and
fail loudly when one never arrives. Every suite cleans up
unconditionally: sandbox files, DB rows via a final refresh, test tags
and their tags/ directories, and any leaked ignored-directory config.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* e2e: act on review — drain stragglers, teardown that cannot be skipped
Events carry no job id, so waitForEvent cannot prove WHICH job finished.
The rename tests now drain the parent-directory refresh their rename
queues before ending, so the next test's refresh wait cannot be
satisfied by a straggler while the DB still reflects the old tree; move
waits match the destination in the completion message.
Teardown must always reach the disk wipe: a failed command aborts the
rest of its hook, so the tag cleanup and the DB-purge refresh are now
best-effort (failOnStatusCode: false, no assertions), and the unignore
runs after every test instead of once per spec. History and
forward-looking product notes trimmed from comments.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
perpetual_shutdown is set by the real before_server_stop listener, so any
earlier async_client teardown in the same pytest-xdist worker leaves it
True on the module-global app and the watchdog returns before reclaiming.
Failed under -nauto, passed alone.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Starting the file-worker pump in every Sanic worker made 5 processes
race the shared status dict, the busy flag, and the same directories.
Non-owners now run only a reclaim watchdog: they refresh nothing unless
the owner PID is gone or its heartbeat is stale.
Production wait_for_job polls every 0.1s instead of 100 Hz. A failed
rename includes the recorded job error in the 409 body.
A directory rename waits inline on a move job. When that move failed
(stale FileGroup rows colliding on primary_path), handle_move reverted
the files and sent a failure event but left the job pending. wait_for_job
only understands 'complete', so POST /api/files/rename spun until its
300s timeout and the Rename modal never closed.
The same sequence could kill file processing for the life of the process.
perpetual_signal treated any CancelledError as terminal and logged
nothing. Separately, perpetual_tasks_started is a shared Event that
survives the worker that set it: docker auto-reloads and worker crashes
left every later after_server_start as a no-op. Queued refreshes sat
untouched while worker_status reported idle. Restarting the API was the
only recovery.
Mark failed jobs failed and raise immediately from wait_for_job. Catch
unexpected cancellation and reschedule unless the app is actually
stopping. Track the perpetual-loop owner PID and retake the claim when
that process is gone; every Sanic worker still runs the file-worker
pump. Harden transfer_queue against the cross-process Empty race, and
stop request processes stealing from the shared public queue.
* upgrade: restart services when the upgrade fails before code changes
The upgrade script stops wrolpi-api and wrolpi-app before fetching, so a
failed fetch, signature verification, or checkout left the WROLPi down
with unchanged code on disk. The EXIT trap now restarts both services
for failures that occur before scripts/upgrade.sh begins modifying the
system. The post-checkout verification failure rolls the branch back to
the previously-running commit first, so services never restart on an
unverified commit.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* upgrade: only restart services when rollback lands on the known-good commit
Review fixes: a failed rollback no longer leaves the restart flag set, so
services cannot start on an unverified or mixed tree; the non-atomic
checkout failure path now rolls back too; trimmed narrative comments.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* downloads: a batch of failed downloads that retried in lockstep
`calculate_next_download` gives every deferred download the same 3^attempts
backoff, measured from the moment it failed. Downloads that fail together
therefore retry together, and a synchronized burst against one host is what
gets a whole batch rejected -- which defers them all again, on the same
schedule. The backoff grows, but the batch never breaks up.
Jitter each retry forward by up to half its backoff. The backoff stays a
floor, so nothing is retried early; a recurring download is still capped at
its own frequency, so nothing is retried later than it was before.
Deferred scheduling is no longer an exact value, so the tests that pinned it
assert a floor and a ceiling instead.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* review: comments that narrated the design instead of the code
Trim the jitter comment to a single WHY and the test docstring to its summary
line. The reasoning that led here belongs in the PR, not beside the
arithmetic.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* downloads: a deferred once-download that nothing ever retried
Deferring a download only schedules it: `defer()` sets the status and
`calculate_next_download` sets `next_download`. Acting on that schedule was
`renew_recurring_downloads`, which filters on `frequency != None` -- so only
recurring downloads were ever marked "new" again.
A once-download has no frequency. One transient error (an HTTP 403 from the
host, a dropped connection) deferred it with a `next_download` that nothing
read, and it stayed deferred until someone retried it by hand.
`delete_old_once_downloads` did not clear it either -- that requires a
`last_successful_download`, which a download that never succeeded does not
have. So every transient failure added one more permanently-stuck row to the
downloads list.
Add `renew_deferred_once_downloads`, called from `do_downloads` alongside the
recurring renewal. `attempts` is preserved, so `calculate_next_download`'s
3^attempts backoff keeps growing across retries: a URL that never succeeds
backs off toward never instead of retrying at a fixed interval.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* review: comments that narrated the bug instead of the contract
The new docstrings restated why the bug existed and how the fix was reasoned
about. That history belongs in the PR, not in code that has to stay true.
Keep the invariant that a reader needs: attempts are not reset, so the backoff
grows. In the test, the assertions already document the contract.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* ui: a download form that emptied itself after every submit
A successful submit reset the whole form, so adding a second download meant
re-choosing the tags, the destination and the video settings that the first
one had already been given.
It now empties only the URLs. Everything else stays for the next download,
and a Clear button beside Download puts the form back to its defaults when
the next one is unrelated.
`useForm` gains `clearPathsOnSuccess`; `clearOnSuccess` is untouched for the
forms that want the whole reset.
* review: the one download form that kept its URLs
Scrape took a `clearOnSuccess` prop that no caller passed, so mapping it to
the URL paths left create-mode scrape clearing nothing. It follows the same
rule as Videos and Archive now: the edit form supplies `onSuccess`, and only
that keeps the form as it was.
`clearPaths` also wrote every path it was given, so a `urls` form gained a
`url: ''` it never declared -- which the comment beside it and the test named
for it both said it would not do. It skips a path neither the defaults nor
the form has, and the test asserts the key is absent.
* ui: two upload dialogs that had drifted apart
The dashboard and the file browser each had their own upload component: two
dropzones, two overwrite toggles, two copies of the progress-bar rendering,
against the same `useUploadFile` state. The file browser's dialog was made
fullscreen with a drop target that grows; the dashboard's stayed a `large`
dialog with a strip to drop onto, because it was a different file.
They differ in one thing that is real: the file browser knows the destination
because the user selected it, and the dashboard has to ask. That is now the
`destination` prop -- `undefined` asks, a string uploads there, and '' is the
media directory rather than a missing destination.
Everything else is one component, so the next change to either entry point
lands on both.
* review: an upload dialog that looped on the media directory
Pressing Upload in the file browser with nothing selected passes `''` -- the
media directory. Three checks along that path read it as "no destination":
* The clearing effect ran, and `doClear` is a new function every render whose
own setState scheduled the next one. React ended it with "Maximum update
depth exceeded", 16 warnings deep, on the default Upload path. The effect
now clears only while asking, and `doClear` is stable.
* The title read `Upload to: /media/wrolpi/`.
* `doUpload` bailed, so a drop on the media root uploaded nothing.
All three predate this branch; moving the dialog is what put them behind one
component. Tests cover the title, what the hook is told, and that a fixed
destination is never the thing that empties the queue.
The upload dialog was 682x367 on a 1440x900 desktop -- 47% of the width, 41% of the
height -- and the drop target inside it was a strip about 90px tall. A small thing to
drag a file onto, with the file browser it came from covering most of the screen behind.
Upload and Move ask for `fullScreen`, which is edge to edge on a phone and 90% above the
tablet breakpoint: filling a desktop reads as the app having been replaced rather than as
something opened over it. Move holds a DirectorySearch above a row per path being moved,
which grows with the selection; 620px showed a handful and scrolled the rest.
Two things had to follow. Mantine sizes a fullscreen dialog's content to the viewport but
leaves the body at its content height, so the footer landed two thirds up the screen with
a quarter of the viewport empty beneath it -- the body takes the leftover now and the
actions ride to the bottom of it. And the drop target itself grows into the room, with a
floor so it stays large once progress bars claim their own space.
Rename and the confirmations keep their sizes: a fullscreen dialog for one text field or
one sentence reads as broken. New Folder keeps `large` too -- it is a text field and a
path preview, the same shape as rename, not the directory picker its name suggests.
`size='fullscreen'` is a DIFFERENT thing -- full width, height from the content, which is
what a log or a preview wants. Both are documented where the sizes are declared, and the
gallery grows a fullscreen example so the difference is visible in all four themes.
339 component tests pass, 69 suites / 1214 jest tests pass, npm run build is clean.
* review: the 375px phone this fix had not been measured on
Answers the five findings on #660.
* Recurring's four fixed columns wanted 416px between them, so at 375px the URL
column got exactly 0 and the table overflowed again -- the same bug as before,
moved down from 540px. Widths are phone-first now, in col-* classes, widening
at the tablet breakpoint.
* The in-progress button in Completed At carried "5 MBps / 3m44s" -- 169-188px
measured, against 124px of column. It shows the percent; the modal it opens
already lists speed, ETA and bytes, and the full text is on the button's title.
* The select column was 2.5em: 16.5px of content for a 22px checkbox, which hung
into the URL column. 3em holds the checkbox and its padding.
* The Cypress fixtures were a raw <table>, so tableLayout would read 'fixed' even
with the selector that loses to Mantine -- the test could not fail for the reason
it claimed. They mount SortableTable, Table.HeaderCell onSort and a real
Checkbox now, and the header check measures the text rather than skipping cells
with no element child.
* Comments trimmed to what cannot be recovered from the code.
Fixing the first surfaced one more: once URL became the small column, its own header
overran -- "URL" and its arrow need 72px against 58 -- and a single word cannot wrap.
A header is clipped by its column as a last resort.
Measured at 375px: once 46/58/123/146, recurring 44/100/104/57/69, nothing
overflowing. 69 suites / 1214 jest tests pass.
The Cypress tests here are NOT verified: the component runner does not start in this
repo (see the separate cypress investigation), so every layout claim above rests on
direct measurement in Chrome, not on that suite.
* review: the 375px phone this fix had not been measured on
Answers the five findings on #660, and now runs against a component suite that can
actually execute (#661).
* Recurring's four fixed columns wanted 416px between them, so at 375px the URL got
exactly 0 and the table overflowed -- the same bug as before, moved down from 540px.
Widths are phone-first now, in col-* classes, widening at the tablet breakpoint.
* The in-progress button in Completed At carried "5 MBps / 3m44s", 169-188px against
124px of column. It shows the percent; the modal it opens already lists speed, ETA
and bytes, and the full text is on the button's title.
* The select column was 2.5em: 16.5px of content for a 22px checkbox, which hung into
the URL column. 3em holds the checkbox and its padding.
* The fixtures were a raw <table>, so tableLayout read 'fixed' even with the selector
that loses to Mantine -- the test could not fail for the reason it claimed. They
mount SortableTable, Table.HeaderCell onSort and a real Checkbox now.
* Comments trimmed to what cannot be recovered from the code.
Two defects the restored suite caught, neither of them findable before it ran:
* The elastic URL column ends up narrower than its own header -- "URL" and its arrow
need 72px against 58 -- and one word cannot wrap, so the column clips. A sized column
must fit its label; only the elastic one may clip. That is what the header test
asserts, per kind of column, rather than measuring text and calling it the label: the
first version measured the text alone, missed the sort arrow, and passed while the
button overran by 14px.
* Recurring's Control column was 4.5em, 70px of content for a 78px "Control". Sized
for the word rather than the button it holds: 5.2em.
At 375px: once 46/58/123/146, recurring 33/100/104/57/80, nothing overflowing and no
sized header clipped. The URL column there is 33px -- the accepted trade for keeping
every column on a phone.
336 component tests pass, 69 suites / 1214 jest tests pass, npm run build is clean.
* review: a header assertion that stopped at the padding box
Three findings on #662.
* The sized-header check compared scrollWidth to clientWidth, which is the padding box,
so a label could overflow its content box into the padding and pass -- the same class
of miss the 4.5em Control failure was meant to close. It measures the label's longest
word (plus the sort arrow, where there is one) against the content box now.
That found two more columns tuned to their exact limit and clipping by a pixel:
Download Frequency wanted 79.4px of a 78.1px box, Completed At 82.3 of 81.9. Every
sized column now has margin over its own label.
* SortableTable stopped applying `width`, leaving dead keys at the Tags and Zim call
sites. Removed, so a column is sized one way. CollectionTable's `width` is NOT dead
-- it converts sixteenths to percentages -- so Channels, Docs, Playlists and Archive
keep theirs.
* Comments trimmed again: the percent button, the header clip, the ButtonGroup rule and
the Cypress helper.
336 component tests pass, 69 suites / 1214 jest tests pass.
* ui: a phone header short enough to leave room for the URL
Five full-length headers do not fit a phone. Sized so each held its own label, the
recurring URL column came out 20px -- narrower than one character and its ellipsis, a
column present but carrying nothing.
A long header carries both forms and `.label-*` shows one: Freq and Done below the tablet
breakpoint, Download Frequency and Completed At above it, where there is room. The mobile
widths are sized for the short word.
At 375px the URL column goes from 20px to 105px on the recurring table and 54px to 97px on
the once table. Nothing overflows, nothing clips, and every column stays.
336 component tests pass, 69 suites / 1214 jest tests pass, npm run build is clean.
* test: the component suite has not run since cypress 15, on any machine
Six component spec files -- 334 tests, including the 4,300-line layout suite -- could not
start. Cypress exited before printing anything, and under its default Electron browser it
did not even do that: it hung at 0% CPU until killed. Chrome prints the error in seconds,
which is the only reason this was findable.
Three things broke, all of them from the 13.17.0 -> 15.20.0 bump.
`baseUrl: "src"`. Cypress 15 loads its config with tsx, and tsx honours tsconfig path
resolution. Cypress requires Node's core `events` module; with that baseUrl the bare
specifier resolves against `src`, and on a case-insensitive filesystem it matches
`src/Events.js`. That file imports the app, so the whole component tree was required in
Node: JSX in `.js` files, which esbuild will not parse.
This is why CI stayed green while every local run died. Linux is case-sensitive, `events`
never matched `Events.js`, and it resolved to the built-in. The baseUrl buys nothing --
there is not one absolute import in the codebase -- so it goes.
`framework: "create-react-app"`. Cypress 14 removed that dev server; the handler is in the
13.17.0 binary and absent from 15. The webpack config is handed over explicitly now, taken
from react-scripts so components compile exactly as `npm start` compiles them. Two things
follow from that: `webpack-dev-server` 5 is a devDependency because Cypress requires 5 and
react-scripts pins 4 (npm nests react-scripts' copy, and the production build is unchanged),
and babel-loader is widened to `cypress/`, which react-scripts does not compile -- the
support file is JSX and was reaching babel with no preset.
The suite ran nowhere. `cypress run` means e2e -- its specPattern is `cypress/e2e/**` --
and these specs sit beside the components in `src`, so no job has ever executed them. A
`cypress-component-tests` job runs them now; without it this breaks again in silence.
One test fails the moment the suite can run, and it is mine, from #660: it asserted the URL
column is the widest in the row. It is not, and never was -- at that width the fixed columns
take ~307px of 400, leaving the URL ~93 against Control's 146. It shipped green because
nothing could run it. It asserts a usable remainder instead.
334 component tests pass, 69 suites / 1214 jest tests pass, `npm run build` is clean.
* review: an assertion with a floor, and comments that told the story instead of the rule
The leftover-width test had a lower bound and nothing else, so it passed on any column
wide enough to have padding -- including one that had grown, which is what losing
table-layout: fixed does. It measures the leftover now: the sized columns are held to
the widths they declare, and the truncating column to exactly what they did not take.
Comments in the config, the CI job and the spec cut back to the rule a reader needs. The
version history and the measurements are in the PR.
The two downloads tables came out 676px and 727px in a 540px window, so Completed At and
Control sat off the right edge and their buttons could not be reached without scrolling
sideways.
`.table-ellipsis` was trying to truncate under `table-layout: auto`, where a table is as
wide as its columns want to be and the container simply overflows. The rule leaned on
`max-width: 0` to force an ellipsis and `min-width: 25em` to keep the text column readable
-- but 25em is 385px, and a floor wider than the screen makes the overflow certain.
Fixed layout inverts it: the columns divide the table's width and the column marked
`column-ellipsis` gives up what it cannot show. Neither the max-width trick nor the floor
is needed then. Columns that must fit something exact -- a checkbox, a group of buttons --
state a width; the rest is the truncating column's.
`table.table-ellipsis`, not the bare class: Mantine sets `table-layout` on its own
`.mantine-Table-table`, the same specificity and later in the cascade, so the first version
of this lost silently. Every other property in the rule applied, which made the table look
fixed while it stayed auto -- the recurring table got *wider* than before, not narrower.
Three things the rework then had to answer for, each of them measured in the browser:
* A header overran its column: "Completed At" printed on top of "Control", because
`.wrolpi-th-sort` is nowrap so a label never splits from its arrow. It wraps here, and
needs `min-width: 0` as well -- the label is an anonymous flex item whose automatic
minimum is its longest word, so without it the label still refuses to go below
"Completed" and still overruns.
* A grouped button was squeezed: `ButtonGroup` is a nowrap flex row whose children shrink,
and the Stop button on a pending download came out 23px against its neighbour's 40 --
clipped, and too small to hit. The columns are sized for the widest group a row can
have, so the buttons no longer give.
* Status's Top Processes table had the same bug. It already asked for `.table-ellipsis`
and already marked its Command cell, so it overflowed a phone too (548px into 503px).
The same rule fixes it, with widths for the three short readings.
A wrapped header is the fallback, not the layout, so every column is sized to hold its own
label on one line -- including on a phone. "Download Frequency" is the exception at 11.7em,
which a phone cannot spare beside four other columns, so it takes a class that widens at the
tablet breakpoint and wraps below it.
Measured, nothing overflowing, no header overrun, no clipped cell, no shrunken button:
540px once 40 / 208 / 146 / 146 recurring 123 / 108 / 131 / 62 / 116
1400px once 40 / 1033 / 146 / 146 recurring 871 / 185 / 131 / 62 / 116
Desktop gains from it as well: the checkbox column drops from 90px to 40 and Completed At
from 291 to 146, so the URL column picks up about 255px.
Covered by five tests in the Cypress layout suite. None of this is checkable in jsdom --
it is all resolved widths, and the central question is whether our `table-layout` beats the
one Mantine sets, which is exactly the part that failed silently.
69 suites / 1214 jest tests pass.
* ui: a preview map that waited for a container it could never see
Both map preview modals -- the per-region eye button and "View All Regions"
-- opened to an empty box. Each renders a div into a Modal and hands it to
MapLibre from an effect gated on `open`, reading the node out of a `useRef`:
if (!open || !mapContainer.current) return;
Mantine's Modal mounts its body one commit after `opened` flips true, so on
the render that opens the modal the ref is still null. The effect returns,
nothing it depends on changes again, and it never runs a second time. No
console error, no rejected promise, no canvas -- just an empty div.
Semantic UI's Portal mounted the body in the same commit that opened it, so
the ref was attached by the time effects ran. The effect bodies are
byte-identical across that migration; the modal library is the only variable,
confirmed against a device still running the older build where both previews
render.
A callback ref instead, so the effect re-runs when the node arrives and this
stops depending on any modal's mounting schedule. The async source lookup
also gets a cancelled flag: it could resolve after the cleanup ran, building
a map nothing would ever remove and leaking a WebGL context per open.
The tests drive the real page and assert a map was built against a node that
is in the document -- the outcome, rather than the mechanism that quietly
changed underneath it. Seven of the eight fail on the code before this.
* build: docker had no world underneath its map regions
Caddy serves /blobs/map-overview.pmtiles out of /opt/wrolpi-blobs, and that
blob is the world at zoom 0-6 -- what a regional extract sits on top of.
Without it a user who subscribes to a few regions gets those regions floating
on an empty background everywhere their data does not reach. Only a full
planet file, at 134GB, makes it unnecessary.
Raspberry Pi and Debian installs get the blob from pi-gen, the live-build
config, and upgrade.sh. Docker got nothing: the compose file bind-mounted a
git-ignored host directory over /opt/wrolpi-blobs, and a bind mount replaces
the image's directory rather than merging with it, so the path existed and was
empty on every Docker WROLPi. Verified against a running deployment, where
that URL returned 404 while the same URL on hardware returned 206.
So the web image fetches the blob at build time, as the other three builders
do, and the mount that would hide it is gone. `curl -f` so an HTTP error
fails the layer -- an image that builds without the blob is this same bug,
except silent at build time too.
The tests read the deployment files rather than a built container: building
to check costs a CDN download and several minutes, and both things that broke
-- nothing fetches it, a mount hides it -- are visible in the source.
* ui: cancel the load handler too, and make the cancel test falsifiable
Review follow-ups on the preview modals.
The `cancelled` flag was checked before constructing the map but not inside
the `load` handler it installs, so cleanup landing between the two left that
body drawing into a map `remove()` had already torn down. Guarded, mirroring
the `destroyed` pattern in MapViewer.js. The source promise also gets a
`.catch`: the helper cannot reject today, but nothing says it never will, and
an unhandled rejection here would be as quiet as the bug this all started as.
Two test corrections, both cases of an assertion claiming more than it read:
The region-drawing test spied on `addSource` AFTER construction and then
re-fired `load` by hand, so its count came entirely from that manual call and
said nothing about the production path. FakeMap now records the sources it is
given, and the assertion reads what the component's own `load` drew -- each
region once, in order, and none for the entry with no bbox.
The new cancel test passed with the guard deleted, which is worse than not
having it: Mantine had not mounted the modal body when it closed, so the
effect had never run and there was nothing to cancel. It now waits for the
effect to reach the source lookup before closing, and ships beside a control
that releases the same held promise WITHOUT closing and requires a map. The
control is what makes the zero meaningful; verified that removing the guard
fails the cancel test while the control still passes.
`IconButton` resolved `variant={props.variant ?? fromRole?.variant ?? 'default'}`, so any
call site that named a color without also naming a role or a variant landed on Mantine's
`default` variant. That variant draws its background, hover, text and border from
`--mantine-color-default*` alone and never reads `color`, so the button rendered
byte-identical to one with no props at all. The color was accepted, stored, and dropped.
`Button` was never affected: it leaves `variant` undefined when there is no role and
Mantine fills in `filled`, which is why the same `color='violet'` works there. Match it --
`filled` when a color is named, `default` when one is not, which is most of them.
Eleven call sites were passing a color that could not reach a pixel:
DonatePage:19, Files:421 the color the USER picked, thrown away
Downloads:241,404,427,460 danger and blue on the download controls
ControllerPage:326,408,1591 the service Open buttons
FilePreview:71,541
The fallback is last, behind both `variant` and `role`, so `role='danger'` keeps its
dashed night outline even when a color is also named; there is a test for that.
Verified in the browser across all four themes: each icon button now matches the labelled
button of the same color exactly, and an icon button with no color keeps the neutral
default. In night and amber the hues collapse to brightnesses, as those themes intend.
The gallery grows a color pair per hue, so a regression here is visible rather than silent.
68 suites / 1204 tests pass.
On the Control tab, each service's actions -- start/stop, restart, logs -- sit in a
`.wrolpi-button-row`, a wrapping flex row. Open sat outside it, after the logs modal,
so it was a block-level sibling of the row rather than a member of it: it landed on its
own line under the others, on both the mobile and the desktop row.
Move it inside the row, after logs. Nothing else changes -- same icon, same link, same
order left to right -- and the row already wraps, so a narrow Actions column breaks it
where it needs to instead of hard-coding a break at Open.
The two row components are now exported so the test can mount them: `Media` renders
neither branch under jsdom, where no media query matches, so `ServicesSection` yields no
rows there at all and the buttons were untestable through the page. The test asserts
that Open shares a `.wrolpi-button-row` with stop, restart and logs, and it fails on the
old layout.
68 suites / 1202 tests pass; `npm run build` is clean.
* ui: a PDF asks before it lights up a night-adapted eye
Night and amber pass media through an SVG color matrix so nothing a user opens costs
them their dark adaptation. It reaches everything drawn in our own document -- images,
video, canvases, and same-origin iframes like the EPUB viewer.
It does not reach a PDF, and the miss is not subtle. Chrome draws PDFs in its own
out-of-process viewer, which the compositor paints outside the parent's filter: the
computed style on the iframe really does say `filter: url(#wrolpi-night-red)` and the
page still arrives in full color. Measured in night mode on a device, a PDF covered
55% of the doc page in non-red pixels -- a full-brightness photograph of a blue sky --
while the chrome around it was correctly red. The preview modal measured 39.8%.
Tinting from outside was tried and does not work. A red `mix-blend-mode: multiply`
overlay above the iframe cannot see the plugin's pixels, so it stops blending and paints
flat opaque red: zero non-red pixels by measurement, document entirely obliterated.
So the PDF is withheld and the reader chooses. Nothing renders until they do; hiding it
with CSS would still load and paint the viewer, which is the flash being avoided.
Gated on whether the theme is CURRENTLY filtering, not on its name. A reader who turns
night's filter off has said they do not want this, and amber with its filter on has the
same unreachable PDF and does. Only PDFs: text and the EPUB viewer are documents of our
own and tint correctly, so gating them would hide something for no reason.
The gallery paragraph claiming the matrix remaps PDFs is corrected -- it was the one
place we told users something this measurement disproves.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* review: the reveal that outlived its document, and a test that proved nothing
Five review comments, all correct.
The bug: MediaGate was not keyed in FilePreview. The effect that rebuilds a preview sets
the modal to null and then to the new content in one pass, so React 18 batches both and
reconciles instead of unmounting -- the fiber survives, and a reader who revealed PDF A
would get PDF B in full color with no prompt. Keyed by url, which is what Docs already
did with activePath.
The test named "re-gates when filtering is turned back on" never revealed anything first,
so it went from gated=false to gated=true with `revealed` already false. That path never
reaches the reset effect: the test passed with the effect deleted. It now clicks Show
first, and fails without the effect.
The hide control moved above the media. A preview fills `calc(100dvh - 18.75rem)`, a
height calibrated with no room for a trailing button row, so a control after it lands
below the fold of a full-screen modal -- leaving the reader who just lit up their eyes
scrolling to find the way back.
Docs' embed -- the surface measured at 55% non-red -- had no contract test where
FilePreview had one. It has one now, asserting the gate wraps the iframe, keys per
document, and reads mediaFilterEnabled rather than the theme's name.
Gallery copy said "night and amber withhold it"; amber's filter is off unless asked for.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test: do not leave videos_destination as an invalid template
test_format_videos_destination deliberately sets %(channel)s to prove
unknown placeholders fail, then never restored the global WROLPi config.
On xdist workers that later ran channel_factory without async_client
re-init, the polluted template caused KeyError/FileNotFoundError in
test_format_video_filename_uses_channel_name_over_info_json.
* test: stop search_suggestions from flaking empty domains on CI
A different failure than videos_destination pollution: under xdist load,
search_suggestions sometimes returned no domains for a valid query while
channels still matched. Harden the path by:
- running channel/domain/author/subject searches sequentially on the shared
Session (asyncio.gather is not safe with one SQLAlchemy session)
- giving archive_factory a URL when only a domain is set so validate() does
not clear collection_id via apply_domain()
- rewriting test_search_suggestions without hardcoded Collection ids and with
unique names that will not collide with other tests
Every Python job started failing in "Install Requirements" with
Error: [Errno 2] No such file or directory: '/home/circleci/project/venv/bin/python3'
on branches whose only changes were under app/. The same config passed two days
earlier, so this is the environment moving, not the code.
The cache key names the minor version (`deps-3.13-...`) but not the patch, and the
cached paths include the whole `venv`. A venv's bin/python3 points at the exact
interpreter that built it, so once cimg/python:3.13 takes a patch bump the restored
venv refers to an interpreter the image no longer has, and creating a venv over that
directory fails instead of repairing it.
Two changes, both needed:
- Rebuild the venv when its interpreter cannot run. This heals the restored cache
whatever left it unusable, and costs a reinstall only on the first run after a bump.
- Move the cache key (-5 -> -6). CircleCI keys are immutable and `save_cache` will
not overwrite one, so without this the poisoned entry is restored and discarded on
every run forever and is never replaced.
Not reproduced locally: macOS python recreates the symlink and exits 0, so the exact
crash is CI's Linux python. The guard does not depend on the mechanism -- it removes
any venv whose bin/python3 will not run, which is the state the error names. Verified
that the guard heals a venv with a dangling interpreter and keeps a healthy one, so the
cache still pays off.
Unrelated and still open: api-tests fails on master itself, in
test_format_video_filename_uses_channel_name_over_info_json, which flakes under -nauto
and passes alone.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replaces Semantic UI React, abandoned upstream, with a component library built on
Mantine and driven entirely by CSS custom properties. A theme is a table of token
values rather than code, so no component branches on which theme is active.
Four themes ship: light, dark, night, and amber. Night exists for night vision --
rods stay insensitive above ~620nm, so the interface is red-only, status is carried
by brightness, and danger by a dashed border rather than a second hue. Media is
filtered through an SVG colour matrix, and that filter is per-theme and under the
user's control: night applies it by default, amber offers it opt-in.
Verified by upgrade QA on a Raspberry Pi 4, twice -- before and after this branch was
rebased onto master. Both runs: upgrade.sh clean from a signed tip, every route
rendering with no console errors, /map rendering from the production bundle, the
night filter audited at zero non-red pixels across the viewport, and a clean reboot.
The pins that fixed the blank React /map were removed as collateral when the
Mantine commit rewrote `overrides` for TypeScript 5. Nothing failed, so nothing
objected: the map kept working only because package-lock.json still happened to
resolve @babel/core to 7.20.12, and devices run `npm install`, not `npm ci`.
Verified on this tree rather than assumed. Raising @babel/core to 7.29.7 and
reinstalling floats the whole pipeline (traverse/generator/types to 7.29.8) and
`npm run build` then fails outright: "'opera_mobile' is not a valid target",
from a helper-compilation-targets that floated away from the browserslist data
it was pinned against. With the pins restored the build compiles, all 141
resolved copies match, and /map renders from the built bundle.
The test walks every path in the lockfile, not just node_modules/<name>: npm
nests a second copy whenever versions conflict, and a nested copy that escaped
the override would put the new pipeline back into the build while the top-level
entry still read correctly.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rebasing onto master brought @testing-library/dom 8.20.0 -> 10.4.1, while
user-event stayed at 13.5.0. v13 targets dom v8 and reaches into internals v10
moved, so its keyboard simulation silently stopped dispatching: two SearchBox
tests failed with `Number of calls: 0` -- the arrow keys and Enter did nothing.
Test-harness only. @testing-library/dom never reaches a build, and no product
code changed in the rebase; the combination was simply never installed together
until master's bump met this branch.
v14's direct API is call-compatible with what the ten suites already do, so no
test needed rewriting -- the calls were already awaited.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
InfoPopup and HelpPopup default to `icon = 'info circle'` and `icon = 'help circle'`.
Neither name was in iconNameAliases, so both fell through to the fallback glyph and
logged an error -- and being defaults, they fired on every use of either helper.
Opening Settings on a device logged 43 of them.
The test that should have caught this was called "maps every icon name the app uses"
and checked a list of twenty names written by hand; these two were not on it. It now
reads the icon defaults off the source -- 85 of them -- and names the file it found an
unmapped one in.
Scanning `name='...'` as well was tried and reverted: `name` is overwhelmingly a
form-field attribute, so it reported 27 field names as unmapped icons and failed on
correct code.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The clear button sat detached from the filter field and 11px below it, and it
dragged the collapse-all button out of line with both.
One stale rule did all of that. `.search-clear { margin: 0.625rem }` is left
over from the Semantic-era filter, where the clear button was a sibling of the
input and needed the gap. It is now ActionInput's action, which joins the two
by dropping the input's right border -- so the margin reopened the seam the
component exists to close, pushed the button down by its own top margin, and,
because a flex line is as tall as its tallest item plus that item's margins,
stretched the wrapper to 62px around a 40px field. The collapse-all button
beside it was centred in that 62px, which is why it looked wrong too.
Removing the rule fixes all three: the button abuts the field and shares its
outline, like the filters on Videos and Archives, and the row is one height.
The class had exactly one user left, so the rule goes with it.
The test asserts the button is inside ActionInput's wrapper, and that no rule
reintroduces a margin on it -- jsdom applies no stylesheet, so the geometry
itself cannot be measured there. Confirmed it fails when the rule is put back.
* ui: the navbar shows which section you are in
Clicking Videos opened the videos page and left the bar looking untouched.
react-router was doing its part the whole time -- the tab carried `active` and
`aria-current="page"` on every one of those pages. Nothing in the stylesheet
drew that class, so all eleven tabs rendered identically and the mark was
visible only to a screen reader.
The current tab now takes a deepened version of the hover wash plus an
underline. The underline is a pseudo-element in `currentColor` rather than a
border, for two reasons: `currentColor` is the foreground the bar already
computed for whichever of the fourteen navbar colors the user picked, so one
rule covers every color in every theme without naming one; and a pseudo-element
adds no height, where a border would jog the whole row as the mark moved
between tabs. The overflow "More" menu and the mobile hamburger mark their
copy of the tab too -- on a phone that menu is where most tabs live.
Calculators and Statistics also lose `end: true`. A tab is meant to stay
current for the pages beneath it, and `end` restricts the match to the exact
path, so either one would have dropped its mark the moment it grew a sub-page.
The other nine links never had it, which is why /videos/1234 already kept
Videos marked.
The test renders the real bar, which needed two pieces of browser jsdom does
not have: fresnel renders no navbar at all unless a breakpoint query matches,
and every width is 0, which convinces the overflow logic that nothing fits and
sweeps all eleven tabs into the closed "More" menu -- where every assertion
about them would have passed vacuously. Both are stubbed, and the styling
itself is asserted against the stylesheet, since jsdom applies none.
* review: the mark follows the tab into the overflow menu
Six review points, no bugs among them. The substantive one: when the window
narrows enough to fold the current tab into "More", the bar went blank again --
the trigger is a plain button, not a NavLink, so nothing was marked until the
user opened the menu to look, which is exactly when they least need telling.
"More" now takes the mark when it holds the current section.
That needs the matching rule outside a NavLink, where the hooks that answer it
cannot be called in a loop, so `isLinkActive` states it once: a link owns the
pages beneath it unless it asks for an exact match. The trigger gets the class
only -- the real NavLink inside the menu already carries `aria-current`, and a
second one on the button that opens the menu announces the same page twice.
The other five, all fair:
* The mobile hamburger and the overflow menu had no test at all, so
`.mantine-Menu-item.active` -- the whole of the mark on a phone -- was
never exercised. Both now are, which meant splitting the viewport and the
bar width out of the fixture so a test can ask for a narrow bar or a phone.
The mobile case also needs the real getBoundingClientRect back: floating-ui
positions the menu's portal and crashes the worker on a stub DOMRect.
* The CSS guard matched the selector as a substring, which still passed with
the rule body emptied or the underline deleted -- and the underline is the
half that survives on a dark bar. It now reads the declarations.
* The table cases used `toContain`, which a regression leaving Home
permanently marked would satisfy on every row. `toEqual([text])`.
* A comment claimed the tab "only matched its own exact path", which is the
root cause the PR description explicitly rules out -- Videos never carried
`end`. Corrected to what those cases actually guard.
* The navbar offers twelve colors, not fourteen. I had carried over
Semantic's palette count, which includes black and white.
Writing the tightened height assertion I reached for `(?!0)`, which calls
0.1875rem zero. It reads the number instead.
* ui: sample data names Blender, not somebody's channel
The gallery, the card samples, and the search fixtures used a real YouTube
channel and one of its video titles as demo content. Sample data ships to
users and appears in a page linked from Settings; naming a real third party in
it puts words in their mouth -- the delete-confirmation sample in particular
read "The channel Wranglerstar and its download rules will be removed."
Blender fits better anyway: the repo's own test media is Big Buck Bunny, so the
sample library now refers to something it actually contains, and to a project
that publishes its films for exactly this kind of use.
* ui: the second sample channel is WROLPi's own
Same reason as the last commit: the remaining sample channel and its video
title belonged to a real person. The pair now reads as Blender and WROLPi,
which are the two things this project can name freely.
Acting on review of #644.
The largest finding came out of a suggestion rather than being in it. The
reviewer asked for coverage of the `color-scheme: dark` rule, on the grounds
that dropping it would leave night's track wrong with no failing test. Written
and passing -- then mutated, by pointing the selector at a type that does not
exist, and the tests stayed green.
Mantine's own stylesheet puts `color-scheme: var(--mantine-color-scheme)` on
`:root`, and ThemeProvider drives it with `forceColorScheme`. The property is
inherited, so a range slider on a dark theme has had a dark track all along and
the rule added here changed no pixel. It is deleted, and the claim about "the
UA's pale slab" is out of the comments and the gallery copy with it.
The tests stay, re-aimed at what actually supplies it: breaking
`forceColorScheme` fails all three, which is worth guarding because it lives in
another module and every accent assertion passes without it.
The other suggestions:
- The gallery slider misrepresented the control it stands for -- it invented an
"Error" step, dropped Trace and ran 0..4, where Settings maps 1..5 onto
Critical / Warning / Info / Debug / Trace. A gallery whose purpose is showing
the real control is worse than useless when it shows a different one. Keyed
to match `levelNameMap` exactly, and it starts at Info so both halves of the
track are visible.
- The "never leaves it at the UA default" test restated the four above it:
`toRgb` throws on anything unresolved, and the computed value of
`accent-color: auto` is the keyword, so those already fail on an unstyled
slider. Replaced with a constraint they do not cover -- `accent-color` is
inherited, so a text field beside the slider must still read `auto`, which is
what says the accent comes from this rule and not from a tinted document.
- The comment implied native checkboxes and radios are on screen the way
sliders are. They are not: those are Mantine's and Progress is a div, so
three of the four selectors are a guard rather than a fix. Said so.
Nits: four product sliders, not five -- the fifth is the sample itself. And
the progressive-enhancement limit is now stated where the rule is, including
what a complete fix would take.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A native `<input type="range">` is drawn by the UA, and with nothing said about
it the UA uses its own accent -- a fixed blue no theme can reach. Five are on
screen: the log level on Settings, and the duration sliders in the food, water
and ration calculators. None had ever been themed; `accent-color` appeared
nowhere in the app.
Night is where that is not merely inconsistent. The theme exists so a user
reading in the dark keeps their adaptation and every token is a red, so the
slider was the brightest non-red thing on the page -- on the one control a user
drags back and forth while watching it.
`accent-color: var(--blue)` for the same reason links take that token: it
remaps per theme, so night gets the dim red (#8a2828) and amber the amber
(#a86618) with no second rule. Checkboxes, radios and `<progress>` are
included because they are painted from the same accent and would have been the
next report.
`accent-color` reaches the thumb and the FILLED half of the track and nothing
else, so on a dark ground the rest stayed the UA's pale slab. `color-scheme:
dark` fixes that, scoped to these controls rather than declared on `html` --
on `html` it also governs scrollbars, spinners and every other piece of browser
chrome, which is a larger change than this one and worth making deliberately.
Tests assert the accent equals `--blue` in each of the four themes, plus a
negative: in the monochrome themes it is a resolved color and never `auto`.
Without that second one the first four would also pass on an unstyled slider
whose accent happened to stringify the same way.
Added to the gallery, which is how the monochrome themes get looked at. Its
slider is stateful and starts mid-track: the accent only shows on the filled
half, so one parked at either end would show almost none of the color the
section exists to check.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The whole panel is wrapped in a `<Link to='/admin/status'>`, so the block is one
anchor -- and `a` takes `color: var(--blue)` with an underline on hover from
tokens.css. `.wrolpi-statistic-value` sets no color of its own, so the three
load figures inherited the link color: blue numbers on a panel where nothing
else is blue, underlining as a group whenever the pointer crossed any part of
it. The Downloading figure below the divider had it too.
`no-link-underscore card-link` is the pair already used wherever a link IS the
element's own text rather than a link through it -- every card title and meta
line, via `cardLinkClass` in Common.js. Reused rather than adding a third
class for the same idea.
The pointer cursor is untouched, which is the point: it comes from the UA
stylesheet for `a[href]`, and the affordance is the only reason to wrap a panel
in a link at all. A test asserts it so a later change cannot take it away
while tidying the rest.
DashboardStatus is exported so its call site can be rendered directly; the
colors are asserted in cypress, since jsdom drops `color: var(--text)` as an
invalid declaration and cannot see this at all.
Each new assertion has a paired negative: the same panel WITHOUT the classes
comes out blue, and has no rule protecting it from an underline. The first
draft of the underline test checked only that no matching hover rule was
`!important`, which was true before the fix as well -- it would have passed on
a panel that still underlined.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The cypress component suite has been failing 12 of 311 for long enough to read
as background noise. Every one of them was the test's own fault, and four
distinct causes account for all twelve.
Dead selectors. `div.ui.label.large`, `div.divider.text`, `.item` and
`have.attr 'error'` all named markup the old library rendered. None of them
started failing when the library was replaced -- they went on matching nothing,
or looking for an attribute no control carries, which is the failure mode that
makes a test look broken rather than the code. A tag chip is `.wrolpi-tag`, a
Select is a real input whose value can be read, an option carries the ARIA role,
and an invalid field says `aria-invalid`. Both directions of that last one are
asserted by value: a valid field says `aria-invalid="false"` rather than
dropping the attribute, so `not.have.attr` would have passed on absence.
Missing context. `useFileWorkerStatus` throws rather than defaulting when its
provider is absent, so the channel edit page died on a modal buried three levels
down while every other page mounted fine. App.js wraps the whole app in that
provider; the mount helper now does too, with the poll slowed to a minute.
Unstubbed fetches. The inventory page loads the food catalog from its own
endpoint, and the RSS edit form fetched downloaders before its intercept was
declared. Both rejected with "Failed to fetch" out of an effect, which Cypress
counts as an uncaught application error and fails the running test on -- so
three inventory tests and one download test died for a reason unrelated to
anything they assert.
Assertions reading the wrong thing. `cy.contains('Food Storage')` found the
hidden `.inventory-print` copy, since contains takes the deepest match and the
print sheet is mounted at all times. And the PUT body arrives as a JSON string,
so `req.body.items` was undefined both in the reply the interceptor built and in
the assertion waiting on it. The RSS submit test deep-equalled one exact JSON
string and broke the moment the video downloader gained eight settings the form
fills in by default; the envelope is still matched exactly and the settings are
checked for the values the fixture set. It also never waited on its own alias,
so an interceptor that never ran would have passed in silence.
Separately, the navbar's shrink test read its link count in a bare `.then()`
right after the corner finished growing -- before the recalculation that growth
triggers had run, so `shrunken` was sometimes already the full count and the
assertion came out "expected 10 to be above 10" under load. It now waits for
the bar to settle to one row, which is the same signal the test above it
asserts, and requires the "More" trigger to exist so it cannot pass having
measured a bar that never overflowed.
New: two tests for the file browser filter's width, desktop and phone, which is
what caught the dead selector behind it.
311 passing, 0 failing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The library has been uninstalled for some time, but the codebase still read as
though it were there: identifiers named after it, rules written against its
class names, and about 180 comments explaining a rule in terms of a thing that
is no longer installed.
Renamed, keeping the vocabulary the app actually uses: semanticUIColorMap ->
navColorHexMap, semanticColorNames -> paletteColorNames, semanticNames ->
iconNameAliases, semanticSizes -> sizeAliases, and SemanticUIColors ->
NavColors in wrolpi/schema.py, which is what validates nav_color on the way in.
ColorToSemanticHexColor had no callers at all and is gone.
The baseline ratchet has done its job -- the list reached zero, so the JSON is
deleted and the guard keeps only the checks with teeth: no package, no import,
no `className="ui "`, no toast library. Its Theme.tsx check went with the
wrappers it was watching for. It also asserts its own walk found files, so a
clean result cannot mean it scanned nothing.
Deleted from App.css: every rule requiring `.ui`, `.inverted`, `.segment` or
`.modal`, none of which any component has set since the stylesheet stopped
loading, plus the form skeletons, ui-alerts and progress-text rules that
nothing renders. Two of them were doing harm rather than nothing: the drag
tints had a dark-theme variant under `.inverted` that stopped applying, leaving
15% of a fixed blue over a dark row, and now mix a token instead.
Two selectors naming markup that no longer exists were live bugs. The file
browser's filter had `.ui.input.file-browser-filter`, so it rendered at the
browser default width on a bar laid out to give it 34em -- now qualified by its
container, because `.wrolpi-action-input` sets `width: 100%` at the same
specificity and is loaded later. And SearchBox takes named props rather than a
rest spread, so the `id` DestinationForm hands it was dropped in silence and
the label it renders pointed at nothing; the download forms' Destination field
has had no associated label since the migration.
"Semantic roles" survives in four places, where it means what it says.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Five findings, no bugs, all valid.
`.wrolpi-or-split > *` still laid out the label, the same structural pitfall the
border rule was already fixed for -- the word escaped the half's 1.5rem padding
on source order alone, same specificity and declared later. Excluded now, and
the test asserts the word's padding is SMALLER than a half's rather than merely
greater than zero, which is what fails when the two blocks are reordered.
The night "never the brightest thing" assertion read `html`'s computed
background. Tokens paint `--bg` on BODY, so that came back `rgba(0, 0, 0, 0)`
-- luminance zero -- making the check "no brighter than black, plus slack". It
did fail the white chip, so it caught the reported bug, but by accident of the
comparison rather than by measuring the page. Reads the `--bg` token now.
The video inset was only ever asserted above the breakpoint; the phone branch
was commented and unmeasured. Worse than it looks, since cypress defaults to a
500px viewport and that is the branch everything else in the suite runs in. It
now asserts zero padding and that the panel really reaches the edge.
The duration chip's border went unasserted -- and the border is what separates a
`--panel`-colored chip from a poster near that color, so losing it left every
contrast assertion green. Asserted on all four themes, against `--border`.
And `Group` was left imported by the dashboard after the panel stopped using it.
All three new assertions verified against the mutation each describes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
