Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Update (2026-09-09): this branch has moved substantially since the description above. Summary of what's new:
All three are real, hardware-tested, with meson test coverage ( |
|
One more note on scope/stability, to be explicit: Bing and OCS are the stable, production-ready providers in this PR -- both are hardware-tested, registered by default ( Openverse and Unsplash are real, working code behind the same interface, but still in progress -- not registered by default, not part of what we're asking to be reviewed here. They're structured so enabling either later is a one-line change, but please don't block this PR on their state; happy to bring them forward as a separate follow-up once they're fully settled. |
Update 2026-09-11 — 6 new commits, real operator-reported bugs found and fixed on live O6N hardwareAll found via live use after the redesign above landed, each root-caused with hard evidence (not inferred from a clean build) before fixing:
All 6 commits build clean (684/684 targets), were rebuilt via a real container build each time, ABI-verified ( |
mirkobrombin
left a comment
There was a problem hiding this comment.
Please replace every direct libadwaita widget with libsingularity components and split the unrelated sensors changes into a separate PR.
…nents Per Mirko's review on PR singularityos-lab#26: every direct Adw.* widget introduced by this branch's OCS/Bing wallpaper browser work is replaced with its libsingularity equivalent, matching the convention already used elsewhere in this codebase (desktop_page.vala's grid_group/SwitchRow, calendar_view.vala's PreferencesGroup, ush_portal.vala/system_view.vala's ConfirmDialog). - wallpaper_ocs_browser.vala: Adw.PreferencesGroup -> PreferencesGroup, Adw.ActionRow -> ActionRow, Adw.PreferencesRow -> PreferencesRow, Adw.EntryRow -> EntryRow (its apply button/signal replaced by an explicit suffix button + entry_activated, since EntryRow has no show_apply_button equivalent), and Adw.ComboRow -> SelectionRow for the provider/category/tag pickers. SelectionRow stores id/label pairs directly, so the old index<->id array plumbing (provider_ids/ category_ids/tag_ids, selected_id()) is gone; set_choices() now just calls set_options()+current_value. - provider_credential_group.vala: the group itself now extends PreferencesGroup instead of Adw.PreferencesGroup; its entry row uses EntryRow/PasswordRow instead of Adw.EntryRow/Adw.PasswordEntryRow. - desktop_page.vala: the Bing Markets two-choice picker moves from Adw.ComboRow to SelectionRow (index constants replaced with id constants all/pick); the Bing markets multi-select dialog and the two delete-confirmation dialogs move from Adw.AlertDialog/ Adw.MessageDialog to Singularity.Widgets.ConfirmDialog, using its custom_area for the region-grouped checkbox list. Behaviour preserved: every existing severity/threshold/cap/guard this branch's commits already documented is untouched -- only the widget classes constructing the UI changed, not the wallpaper/Bing/OCS logic around them. Verified with a real meson build + the full test suite on ULTRA (arm64): 14/14 tests pass, singularity-desktop links cleanly.
|
Addressed both points: libadwaita widgets replaced. Every direct
No behavioural change intended -- verified with a real Sensors changes split out. The sensors-popover work (native-widget conversion + responsiveness) that had ended up bundled into this branch touched only Re-requesting review. |
|
Both addressed in 1. Direct libadwaita initialization and dependency Grepped the whole branch diff against
On whether that dependency was genuinely removable rather than just redundant — I checked before deleting the line, since a transitive need would have made it a no-op rather than a fix:
So it was a real direct dependency with no remaining consumer, not a redundant declaration of something arriving another way. I also reworded two stale comments that still mentioned Verified on an aarch64 build host, not inferred:
2. Attribution format Switched to your format. I've also put a PR-level One thing I did not do unilaterally: seven earlier commits on this branch still carry the old The sensors popover work is already reverted off this branch ( |
…nents Per Mirko's review on PR singularityos-lab#26: every direct Adw.* widget introduced by this branch's OCS/Bing wallpaper browser work is replaced with its libsingularity equivalent, matching the convention already used elsewhere in this codebase (desktop_page.vala's grid_group/SwitchRow, calendar_view.vala's PreferencesGroup, ush_portal.vala/system_view.vala's ConfirmDialog). - wallpaper_ocs_browser.vala: Adw.PreferencesGroup -> PreferencesGroup, Adw.ActionRow -> ActionRow, Adw.PreferencesRow -> PreferencesRow, Adw.EntryRow -> EntryRow (its apply button/signal replaced by an explicit suffix button + entry_activated, since EntryRow has no show_apply_button equivalent), and Adw.ComboRow -> SelectionRow for the provider/category/tag pickers. SelectionRow stores id/label pairs directly, so the old index<->id array plumbing (provider_ids/ category_ids/tag_ids, selected_id()) is gone; set_choices() now just calls set_options()+current_value. - provider_credential_group.vala: the group itself now extends PreferencesGroup instead of Adw.PreferencesGroup; its entry row uses EntryRow/PasswordRow instead of Adw.EntryRow/Adw.PasswordEntryRow. - desktop_page.vala: the Bing Markets two-choice picker moves from Adw.ComboRow to SelectionRow (index constants replaced with id constants all/pick); the Bing markets multi-select dialog and the two delete-confirmation dialogs move from Adw.AlertDialog/ Adw.MessageDialog to Singularity.Widgets.ConfirmDialog, using its custom_area for the region-grouped checkbox list. Behaviour preserved: every existing severity/threshold/cap/guard this branch's commits already documented is untouched -- only the widget classes constructing the UI changed, not the wallpaper/Bing/OCS logic around them. Verified with a real meson build + the full test suite on ULTRA (arm64): 14/14 tests pass, singularity-desktop links cleanly.
c2c0e20 to
b6f7785
Compare
|
Both halves of your review are done. Branch head is Libadwaita removal Three sites, all introduced by this PR, all gone:
I checked the dependency was genuinely removable rather than needed transitively before deleting it: Verified on an aarch64 build host: Attribution trailers in history Rewrote the seven commits that carried the old trailer — Verification after the rewrite:
The PR description carries the same trailer pair, scoped to the PR as a whole. This is the format I'll use on singularity-* work from here on. Sorry for the force-push churn on your review anchors — reaching the older messages required it. The sensors popover was already reverted off this branch in |
…nents Per Mirko's review on PR singularityos-lab#26: every direct Adw.* widget introduced by this branch's OCS/Bing wallpaper browser work is replaced with its libsingularity equivalent, matching the convention already used elsewhere in this codebase (desktop_page.vala's grid_group/SwitchRow, calendar_view.vala's PreferencesGroup, ush_portal.vala/system_view.vala's ConfirmDialog). - wallpaper_ocs_browser.vala: Adw.PreferencesGroup -> PreferencesGroup, Adw.ActionRow -> ActionRow, Adw.PreferencesRow -> PreferencesRow, Adw.EntryRow -> EntryRow (its apply button/signal replaced by an explicit suffix button + entry_activated, since EntryRow has no show_apply_button equivalent), and Adw.ComboRow -> SelectionRow for the provider/category/tag pickers. SelectionRow stores id/label pairs directly, so the old index<->id array plumbing (provider_ids/ category_ids/tag_ids, selected_id()) is gone; set_choices() now just calls set_options()+current_value. - provider_credential_group.vala: the group itself now extends PreferencesGroup instead of Adw.PreferencesGroup; its entry row uses EntryRow/PasswordRow instead of Adw.EntryRow/Adw.PasswordEntryRow. - desktop_page.vala: the Bing Markets two-choice picker moves from Adw.ComboRow to SelectionRow (index constants replaced with id constants all/pick); the Bing markets multi-select dialog and the two delete-confirmation dialogs move from Adw.AlertDialog/ Adw.MessageDialog to Singularity.Widgets.ConfirmDialog, using its custom_area for the region-grouped checkbox list. Behaviour preserved: every existing severity/threshold/cap/guard this branch's commits already documented is untouched -- only the widget classes constructing the UI changed, not the wallpaper/Bing/OCS logic around them. Verified with a real meson build + the full test suite on ULTRA (arm64): 14/14 tests pass, singularity-desktop links cleanly.
b6f7785 to
0c47cce
Compare
…nents Per Mirko's review on PR singularityos-lab#26: every direct Adw.* widget introduced by this branch's OCS/Bing wallpaper browser work is replaced with its libsingularity equivalent, matching the convention already used elsewhere in this codebase (desktop_page.vala's grid_group/SwitchRow, calendar_view.vala's PreferencesGroup, ush_portal.vala/system_view.vala's ConfirmDialog). - wallpaper_ocs_browser.vala: Adw.PreferencesGroup -> PreferencesGroup, Adw.ActionRow -> ActionRow, Adw.PreferencesRow -> PreferencesRow, Adw.EntryRow -> EntryRow (its apply button/signal replaced by an explicit suffix button + entry_activated, since EntryRow has no show_apply_button equivalent), and Adw.ComboRow -> SelectionRow for the provider/category/tag pickers. SelectionRow stores id/label pairs directly, so the old index<->id array plumbing (provider_ids/ category_ids/tag_ids, selected_id()) is gone; set_choices() now just calls set_options()+current_value. - provider_credential_group.vala: the group itself now extends PreferencesGroup instead of Adw.PreferencesGroup; its entry row uses EntryRow/PasswordRow instead of Adw.EntryRow/Adw.PasswordEntryRow. - desktop_page.vala: the Bing Markets two-choice picker moves from Adw.ComboRow to SelectionRow (index constants replaced with id constants all/pick); the Bing markets multi-select dialog and the two delete-confirmation dialogs move from Adw.AlertDialog/ Adw.MessageDialog to Singularity.Widgets.ConfirmDialog, using its custom_area for the region-grouped checkbox list. Behaviour preserved: every existing severity/threshold/cap/guard this branch's commits already documented is untouched -- only the widget classes constructing the UI changed, not the wallpaper/Bing/OCS logic around them. Verified with a real meson build + the full test suite on ULTRA (arm64): 14/14 tests pass, singularity-desktop links cleanly.
0c47cce to
61378b2
Compare
|
Pushed Bing serves the same photograph to several regional markets on a given day. This page crawls one listing per market and merges them into a single grid, so with every market enabled it was showing each photo once per market that served it — 213 cards for 41 distinct photographs on the board I tested, while the theme picker next to it showed the correct 41. De-duplication has to happen across markets, which a per-market listing structurally can't do, so the provider asks the helper for the combined view rather than trying to reconcile the markets itself:
Tests cover both directions of the replacement rule. Built and ran the full suite on aarch64: 14/14 green, including the two new cases. The helper half is in cix-installer ( |
…ake it responsive Extracted from PR singularityos-lab#26 (OCS wallpaper browser), which bundled these sensors-popover changes alongside unrelated wallpaper work. Squashes four commits from that branch that touched only panel.vala: - convert popover to native Singularity.Widgets (PreferencesGroup/ PreferencesRow/ActionRow), same convention the rest of the desktop surface (desktop_page, calendar_view) already uses for grouped lists - full Adw/PreferencesGroup convention audit pass - make the popover responsive - swap the one remaining raw Adw.PreferencesRow (the compact-row container in append_compact_row) for Singularity.Widgets.PreferencesRow, so this surface is 100% libsingularity widgets, no direct libadwaita Behaviour preserved: every severity threshold, MAX_ROWS_PER_GROUP cap, polling interval, schema-key guard, map/unmap gate, and the synchronous-refresh-on-popover-open path are unchanged. The heat bar stays a raw Gtk.DrawingArea (Gtk.LevelBar's battery-style level-low/level-high semantics paint every short bar red -- documented inline where make_heat_bar() is defined).
67eeec3 to
d578720
Compare
…ake it responsive Extracted from PR singularityos-lab#26 (OCS wallpaper browser), which bundled these sensors-popover changes alongside unrelated wallpaper work. Squashes four commits from that branch that touched only panel.vala: - convert popover to native Singularity.Widgets (PreferencesGroup/ PreferencesRow/ActionRow), same convention the rest of the desktop surface (desktop_page, calendar_view) already uses for grouped lists - full Adw/PreferencesGroup convention audit pass - make the popover responsive - swap the one remaining raw Adw.PreferencesRow (the compact-row container in append_compact_row) for Singularity.Widgets.PreferencesRow, so this surface is 100% libsingularity widgets, no direct libadwaita Behaviour preserved: every severity threshold, MAX_ROWS_PER_GROUP cap, polling interval, schema-key guard, map/unmap gate, and the synchronous-refresh-on-popover-open path are unchanged. The heat bar stays a raw Gtk.DrawingArea (Gtk.LevelBar's battery-style level-low/level-high semantics paint every short bar red -- documented inline where make_heat_bar() is defined).
d578720 to
caa4b7a
Compare
|
Rebased and consolidated onto the current PR #25 branch tip ( The rewritten series now contains:
Verified in a Debian forky Podman container: Ninja This is necessarily an interim rebase onto #25's branch tip. I will do one final re-rebase onto upstream |
|
@mirkobrombin, please re-review the rebased series. GitHub returned 404 when I attempted the requested-reviewers API call from the fork-owner account, so I am explicitly tagging you here. |
…ake it responsive Extracted from PR singularityos-lab#26 (OCS wallpaper browser), which bundled these sensors-popover changes alongside unrelated wallpaper work. Squashes four commits from that branch that touched only panel.vala: - convert popover to native Singularity.Widgets (PreferencesGroup/ PreferencesRow/ActionRow), same convention the rest of the desktop surface (desktop_page, calendar_view) already uses for grouped lists - full Adw/PreferencesGroup convention audit pass - make the popover responsive - swap the one remaining raw Adw.PreferencesRow (the compact-row container in append_compact_row) for Singularity.Widgets.PreferencesRow, so this surface is 100% libsingularity widgets, no direct libadwaita Behaviour preserved: every severity threshold, MAX_ROWS_PER_GROUP cap, polling interval, schema-key guard, map/unmap gate, and the synchronous-refresh-on-popover-open path are unchanged. The heat bar stays a raw Gtk.DrawingArea (Gtk.LevelBar's battery-style level-low/level-high semantics paint every short bar red -- documented inline where make_heat_bar() is defined).
caa4b7a to
7901a9f
Compare
…ake it responsive Extracted from PR singularityos-lab#26 (OCS wallpaper browser), which bundled these sensors-popover changes alongside unrelated wallpaper work. Squashes four commits from that branch that touched only panel.vala: - convert popover to native Singularity.Widgets (PreferencesGroup/ PreferencesRow/ActionRow), same convention the rest of the desktop surface (desktop_page, calendar_view) already uses for grouped lists - full Adw/PreferencesGroup convention audit pass - make the popover responsive - swap the one remaining raw Adw.PreferencesRow (the compact-row container in append_compact_row) for Singularity.Widgets.PreferencesRow, so this surface is 100% libsingularity widgets, no direct libadwaita Behaviour preserved: every severity threshold, MAX_ROWS_PER_GROUP cap, polling interval, schema-key guard, map/unmap gate, and the synchronous-refresh-on-popover-open path are unchanged. The heat bar stays a raw Gtk.DrawingArea (Gtk.LevelBar's battery-style level-low/level-high semantics paint every short bar red -- documented inline where make_heat_bar() is defined).
7901a9f to
9a628da
Compare
…ake it responsive Extracted from PR #26 (OCS wallpaper browser), which bundled these sensors-popover changes alongside unrelated wallpaper work. Squashes four commits from that branch that touched only panel.vala: - convert popover to native Singularity.Widgets (PreferencesGroup/ PreferencesRow/ActionRow), same convention the rest of the desktop surface (desktop_page, calendar_view) already uses for grouped lists - full Adw/PreferencesGroup convention audit pass - make the popover responsive - swap the one remaining raw Adw.PreferencesRow (the compact-row container in append_compact_row) for Singularity.Widgets.PreferencesRow, so this surface is 100% libsingularity widgets, no direct libadwaita Behaviour preserved: every severity threshold, MAX_ROWS_PER_GROUP cap, polling interval, schema-key guard, map/unmap gate, and the synchronous-refresh-on-popover-open path are unchanged. The heat bar stays a raw Gtk.DrawingArea (Gtk.LevelBar's battery-style level-low/level-high semantics paint every short bar red -- documented inline where make_heat_bar() is defined).
|
@mirkobrombin the last review on this PR (pinned to 67eeec3) asked for exactly the rebase that already happened -- the branch was rebased onto the current PR #25 tip and the overlapping files resolved, as noted in the comment above requesting re-review of the rebased series (current head 9a628da, 16 commits ahead / 58 behind 67eeec3). I do not have permission to dismiss the review from this account (the dismissal API 404s the same way the reviewer-request API does) -- could you dismiss it or take another look when you have a moment? |
…nents Per Mirko's review on PR singularityos-lab#26: every direct Adw.* widget introduced by this branch's OCS/Bing wallpaper browser work is replaced with its libsingularity equivalent, matching the convention already used elsewhere in this codebase (desktop_page.vala's grid_group/SwitchRow, calendar_view.vala's PreferencesGroup, ush_portal.vala/system_view.vala's ConfirmDialog). - wallpaper_ocs_browser.vala: Adw.PreferencesGroup -> PreferencesGroup, Adw.ActionRow -> ActionRow, Adw.PreferencesRow -> PreferencesRow, Adw.EntryRow -> EntryRow (its apply button/signal replaced by an explicit suffix button + entry_activated, since EntryRow has no show_apply_button equivalent), and Adw.ComboRow -> SelectionRow for the provider/category/tag pickers. SelectionRow stores id/label pairs directly, so the old index<->id array plumbing (provider_ids/ category_ids/tag_ids, selected_id()) is gone; set_choices() now just calls set_options()+current_value. - provider_credential_group.vala: the group itself now extends PreferencesGroup instead of Adw.PreferencesGroup; its entry row uses EntryRow/PasswordRow instead of Adw.EntryRow/Adw.PasswordEntryRow. - desktop_page.vala: the Bing Markets two-choice picker moves from Adw.ComboRow to SelectionRow (index constants replaced with id constants all/pick); the Bing markets multi-select dialog and the two delete-confirmation dialogs move from Adw.AlertDialog/ Adw.MessageDialog to Singularity.Widgets.ConfirmDialog, using its custom_area for the region-grouped checkbox list. Behaviour preserved: every existing severity/threshold/cap/guard this branch's commits already documented is untouched -- only the widget classes constructing the UI changed, not the wallpaper/Bing/OCS logic around them. Verified with a real meson build + the full test suite on ULTRA (arm64): 14/14 tests pass, singularity-desktop links cleanly.
64d08da to
c739d20
Compare
|
Brought this branch up to the current OCS quality bar with 6 commits: raised the aggregate crawl cap to the real measured feed size (3352 unique items, was silently truncating at 1500), added a disk-backed thumbnail cache with LRU eviction so revisits are instant, bounded thumbnail memory to roughly a viewport worth via scroll-driven load/evict instead of decoding the entire feed up front (was measured at 1.35 GiB resident for 3352 cards), added stale-while-revalidate so a cached crawl paints immediately while refreshing in the background, sanitized free-text OCS tags against Unicode bidi-override/control characters before they reach the tag dropdown, and added a Clear Filters action. Verified with a full ninja build (228/228, 0 errors) and meson test (11/11) in a debian:forky container. |
58af878 to
1811310
Compare
…t broke OCS browsing Root-caused the operator-reported OCS regression from tonight's fork/ocs-browser-clean deploy (67eeec3) to cixmini. Evidence, not guess: 67eeec3 ("Bing region picker sets a dedup preference, not a fetch filter") redefined what ~/.config/ncz-wallpaper/bing-markets means -- from "the set of markets to fetch" to "which region wins a dedup tie" -- and states in its own commit message that this depends on "the matching cix-installer change" to 45-wallpaper-rotator.sh's ncz-wallpaper-bing helper, which lives in a separate repo this session did not touch or build. WallpaperBing.CONSOLIDATED_ID's own comment in wallpaper_ocs.vala still described the OLD contract ("advertises this id only when the file holds the 'all' sentinel") after 67eeec3 shipped, which is direct evidence the two sides of this cross-repo contract drifted in the same commit that changed one side of it. BingWallpaperProvider.choices() in wallpaper_provider.vala is the OCS browser's Bing tab -- it depends on the SAME file and the SAME helper contract. If the deployed ncz-wallpaper-bing predates 67eeec3's change, it still treats a non-"all" file as a fetch restriction; tonight's redesigned picker (this branch, previous commit) makes picking a single specific region the natural, inviting action, where the old multi-select dialog defaulted to all-13-checked and rarely produced a genuinely single-market file. That shift in what the file typically contains, against a helper that has not also moved to the new contract, is what most plausibly collapses both the rotator's feed AND the OCS browser's Bing results down to one market -- exactly the "OCS is broken" symptom reported live, and NOT something the existing test suite (14/14 passes on 67eeec3 unmodified) can catch, since it never exercises the deployed helper binary. Verification limits: this session did not have access to cix-installer or cixmini (out of scope per instructions), so the deployed helper's actual contract version was not directly confirmed. This is the strongest evidence-backed hypothesis available from the singularity-shell side alone; verifying /usr/local/bin/ncz-wallpaper-bing's behavior on cixmini is the next diagnostic step before another live test. Changes: - wallpaper_ocs.vala: rewrote the CONSOLIDATED_ID comment to describe the current contract and flag the version-skew risk at the exact spot the assumption lives. - wallpaper_provider.vala: BingWallpaperProvider.choices() now logs a warning() when the helper does not advertise the combined view, so a future occurrence is diagnosable from the journal instead of silently presenting a narrowed result set as if Bing just has fewer photos today. Verified: full ninja build (365/365, 0 errors) and meson test (14/14 pass, including wallpaper-ocs) in a debian:forky podman container on ULTRA; zoder (nemotron35 via the TYDEUS reviewer agent) reviewed the combined diff against the SelectionRow/warning() usage, no findings.
Operator feedback on tonight's cixmini deploy: opening "Choose Preferred Region..." raised a modal ConfirmDialog with a scrollable checkbox list grouped by region and a separate "Apply" button, which reads as out of place next to every other setting on this page -- interval_row, decorations_side_row and the rest are all inline SelectionRow expanders that reveal their choices in place and commit on click. Replaced the dialog with exactly that pattern: bing_markets_row is now a single SelectionRow.with_options() listing "All Markets, No Preference" followed by all 13 markets from BING_MARKETS_TABLE, each labelled "<Region> - <Market>" (e.g. "Europe - Germany") so the region grouping the old dialog expressed with section headers survives as label text, and SelectionRow's own search entry (which appears past 5 items) lets a region or market name filter the now-14-entry list. Clicking any row is a single action: SelectionRow always collapses its own expander and fires `selected` with exactly the one id chosen, matching the operator's "inline within the settings/control-panel row itself" request with no new widget class and no popup. Removed the now-dead popup implementation entirely: bing_markets_dialog, bing_markets_checkboxes, open_bing_markets_dialog(), on_bing_markets_dialog_response(), the BING_MARKETS_ID_PICK sentinel, and the bing_markets_regions bookkeeping list that only existed to group the dialog's checkboxes (the flat SelectionRow needs no group index; region is just a label prefix now). Net -62 lines. Verified: full ninja build (365/365, 0 errors) and meson test (14/14 pass) in a debian:forky podman container on ULTRA; ldd confirms every library resolves under /usr/lib/aarch64-linux-gnu (Debian multiarch, zero /lib64 references) -- no Fedora-vs-Debian libc mismatch. zoder (nemotron35 via the TYDEUS reviewer agent) reviewed the SelectionRow usage, the current-value fallback logic for a stale/unrecognized market code in the config file, and the removed-code cleanup; no findings.
Request the OCS server maximum of 100 items in the existing single page per category. This keeps the crawl at the same 45 requests and within the same per-category timeout budget. A forced live crawl against api.pling.com from the CIX Sky1 target on 2026-09-13 returned 3,485 importable results across all 45 usable pling categories: 3,352 unique item IDs after removing 133 cross-category duplicates, with zero category failures in 14.19 seconds. Raise the aggregate safety cap from 1,500 to 4,000, 648 items (19%) above the measured unique total, so it no longer truncates the current feed. Gtk.FlowBox retains one real child hierarchy per card and the browser eagerly decodes every 344x208 thumbnail. A target-side representative benchmark settled at 703 MiB RSS for 1,500 cards and 1.35 GiB for 3,352; construction took 0.43s and 0.92s respectively. That linear cost is worth a future virtualized/lazy-loading change, but is not a present resource limit on the 64 GiB Sky1 system (59 GiB available during the test). Verified after rebasing the concurrent wallpaper history/favorites work: clean full ninja build (376/376, 0 errors) and meson test (16/16 pass) in a debian:forky podman container on ULTRA. Assisted-by: Codex:gpt-5 AI-Scope: Measured the live page-size-100 OCS aggregate and target-side FlowBox memory behavior, raised the page size and crawl safety cap, and verified the full build and test suite.
OCS previews were downloaded again on every page visit, while three strided loader lanes made populating large filtered grids unnecessarily slow. Persist successful preview responses in a bounded atomic LRU cache and use eight synchronized loader lanes so revisits avoid network work and uncached grids fill faster. AI-Scope: Implemented the upstream-specified thumbnail cache, browser integration, concurrency change, and verification. Assisted-by: Claude Code:claude-sonnet-5 AI-scope: Implemented the dispatched cache and concurrency changes from the upstream diagnosis and specification.
The 4000-item aggregate eagerly decoded every FlowBox thumbnail, driving measured resident memory to roughly 1.35 GiB for 3352 cards even though only a screenful was visible. Queue only cards intersecting the viewport plus a bounded prefetch margin, and evict decoded paintables outside a wider hysteresis margin so scrolling through the full feed cannot retain every texture. Assisted-by: Claude Code:claude-sonnet-5 AI-scope: Implemented viewport-driven thumbnail scheduling and in-memory texture eviction while preserving the existing cache and cancellation machinery.
Expired metadata cache files previously withheld a usable grid until every live category request completed, making normal page opens unnecessarily dependent on network latency. Paint any valid persisted snapshot immediately, re-crawl through the existing bounded worker pool into detached metadata, and atomically repaint only when background revalidation yields usable results. Assisted-by: Claude Code:claude-sonnet-5 AI-scope: Implemented stale-while-revalidate metadata loading with generation-safe background crawling and a no-empty-frame result swap.
Uploader-supplied tag text is rendered directly in filter controls, so invisible format and control characters can corrupt nearby UI while oversized labels can break the row layout. Sanitize and cap tags at the shared parser boundary so every provider path receives the same safe, deduplicated values. Assisted-by: Claude Code:claude-sonnet-5 AI-Scope: Implemented Unicode category filtering, tag truncation, and focused parser tests from the requested OCS tag-hardening requirements.
Resetting category, tag, and search independently makes returning to the full wallpaper view unnecessarily tedious. Add one visible action that synchronizes all three controls under the existing update guard and refreshes the grid only after the complete reset. Assisted-by: Claude Code:claude-sonnet-5 AI-Scope: Added the filter-group reset button, guarded UI synchronization, and single-pass grid filtering requested for the wallpaper browser.
Tonight's OCS browser work (79a885a raising the aggregate feed to its real ~3358-item size, cc5c861/b9d8a8b/e5ee222 thumbnail caching, viewport paging and stale-revalidation) made the page noticeably slower to open, reported live on cixmini after a fresh desktop-session restart. Root cause, isolated with real timing instrumentation against the actual 3358-item OCS cache on CIX Sky1 target hardware (cixmini): load_cached() built every WallpaperCard widget hierarchy for the cached aggregate in one unbroken synchronous main-thread loop. PERF load_cached: read=1.5ms parse=96.6ms entries=3358 PERF load_cached: add_card_loop=2139.6ms filter_cards=57.0ms TOTAL=2295.8ms JSON parsing was not the bottleneck (96.6ms); GTK widget construction was (2139.6ms, ~0.64ms/card), and it ran on every cache repaint -- i.e. on every page open, and again whenever a stale cache silently revalidates in the background (revalidate_cached() has the identical pattern). Viewport-paged thumbnail *decoding* (b9d8a8b) does not help here: add_card() still builds a full card/button/badge widget subtree for every item regardless of visibility, so raising the real feed size from an effective ~400 items (pre-79a885a page-size-10 default) to 3358 multiplied this synchronous block by roughly 8x. Fix: build the card list in bounded batches (CARD_BUILD_BATCH_SIZE=150), yielding to the main loop between batches via the same Idle.add() pattern already used elsewhere in this file for polling loops, so the compositor stays responsive throughout. queue_viewport_thumbnails() runs after every batch so visible rows start decoding as soon as they exist. load_cached() became async (single call site in browse_all() updated to yield it). revalidate_cached() keeps its no visible empty grid promise because the first batch still lands in the same synchronous continuation as the clear() that precedes it -- only the remaining batches spread across further main-loop turns. Measured after the fix, same real cache/hardware: PERF-FIX load_cached: read+parse=94.8ms entries=3358 PERF-FIX batch: processed=150 batch_build=48.87ms ... (22 batches, worst observed 129.55ms) PERF-FIX load_cached: populate_cards_batched=4566.6ms TOTAL=4719.4ms Worst single main-loop-turn block dropped from 2139.6ms to 129.55ms (>16x). Total wall-clock to fully populate the grid from cache went up (~4.7s vs ~2.3s) from Idle-scheduling overhead across ~22 batches; that tradeoff is deliberate -- the reported complaint was the shell appearing to hang, not total elapsed time, and the grid now visibly fills in instead of freezing then appearing all at once. Verified: clean ninja build (228/228) and meson test (11/11 pass) in a debian:forky podman container on ULTRA. Timing measured with temporary stderr instrumentation against a real 3358-item OCS cache (copied from the live cixmini session) driven through the actual settings page code path in an isolated headless nested Wayland session (WLR_BACKENDS=headless labwc) on cixmini target hardware, not a synthetic benchmark; all diagnostic instrumentation was removed before this commit. zoder (reviewer agent, cross-family from this session) reviewed the diff with file access to the real working tree and returned APPROVE after verifying gen/cancellation handling, the async conversion's single call site, closure/ownership correctness in the Idle.add() pattern, and the no empty grid invariant, each against verbatim-quoted lines. Assisted-by: Claude Code:claude-sonnet-5 AI-Scope: Root-caused the freeze with real timing instrumentation on target hardware, implemented and verified the batched-construction fix, and ran the review gate.
1811310 to
c18a1f6
Compare
|
Addressed all three threads on this branch (now at c18a1f6):
Compile verification: static checks above are solid; a full build hit an environment gap (libsingularity not available as an installed dependency or fetchable subproject on my available build hosts) unrelated to this diff — noting honestly rather than claiming something I didn't verify. Assisted-by: Claude Code:claude-sonnet-5 |
|
Pushed On the three open review threads, all pinned to commits well behind the current head -- I want to lay out precisely why each is resolved rather than just asserting it, since I don't have dismiss rights on this repo (confirmed:
Given I can't dismiss these myself, @mirkobrombin -- could you take a look and either dismiss what's stale or flag anything I've misjudged? Happy to address anything still live. One more thing, flagged honestly rather than worked around: Assisted-by: Claude Code:claude-opus-5 |
…gory loading Removes tag-based filtering (UI, active/known-tag bookkeeping, the card_matches tag check) and the now-redundant Clear Filters action, leaving category as the only filter dimension. Replaces the eager, load-every-category-and-merge-into-one-aggregate crawl with a lazy, per-category load: selecting a provider now only populates the category dropdown, and nothing is fetched until the user actually picks a category. Re-picking (or re-entering the page on) a category re-runs the same load, reusing the existing cache-then- revalidate-if-stale flow -- now scoped to provider+category instead of provider alone (WallpaperBrowseCache.path_for/save/load gain an optional category argument; path_for(provider) alone is unchanged, so existing single-arg callers and the test suite are unaffected). This was previously pulling 3300+ items across every usable category on every provider select (measured against pling: 45 categories, 3485 results) before the user had picked anything, which is real time lost per open and a UI freeze while the resulting merged grid was built. Verified via the wallpaper-ocs-test target (34/34 pass, unchanged from before this change) and by reading through every call site touched by the add_card()/card_matches()/store_cache() signature simplifications that removing tag bookkeeping and per-item category lookup made possible. singularity-desktop itself currently fails to build for an unrelated, pre-existing reason (FanControlChannel/SensorMonitor.gpu_ sampling missing from libsingularity, confirmed present on a fresh clone of upstream main independent of this branch) -- not touched here, flagged separately on the PR. Assisted-by: Claude Code:claude-opus-5 AI-Scope: implemented the tag-removal and lazy-loading rearchitecture across wallpaper_ocs_browser.vala and wallpaper_browse_cache.vala from the operator's specification (drop tag filtering, load a category only when picked, refresh on re-browse); verification and the PR-state investigation are mine.
6c24ca4 to
6b64018
Compare
|
Amended to Assisted-by: Claude Code:claude-opus-5 |
Depends on #25
Stacked on the wallpaper source-selector branch in #25.
Summary
Updated 2026-09-12 after maintainer review: this branch no longer uses libadwaita at all. Every row/group/dialog is a libsingularity widget,
Adw.init()and theAdw.StyleManagercolour-scheme forcing are gone fromsrc/core/main.vala, anddependency('libadwaita-1')is gone frommeson.build.grep -rn 'Adw' src testsis empty andldd singularity-desktop | grep -i adwaitareturns nothing.Earlier update 2026-09-07 after live-testing and direct maintainer feedback (use the real
*Rowwidgets with the Group/Page convention; the browser should be an extension of the control panel, not a separate window). The browser was substantially redesigned from the original window-based version:WallpaperOcsBrowseris now aSettingsPage(not aGtk.Window) pushed via the same navigation stack every other control-panel page uses (view.navigate_to(...)), with standard back navigation -- not a standalone modal.SelectionRowdropdowns (previously chip pills and an expander-based control that had a real click bug)..jsonsidecar thencz-wallpaper-ocshelper already writes.opendesktop.organdpling.comare the same content network under two names).WallpaperOcs(core): a pure JSON adapter over the already-shippedncz-wallpaper-ocsCLI -- invoked via argv + asyncGLib.Subprocess, never a shell command string. Parsesproviders/index/browse/importoutput; validates every JSON node's type before calling a typed getter, and fails inline (never fabricates a license/author/preview) on malformed or unsupported output.WallpaperOcsImports(core): tracks import state via per-image sidecar files so re-browsing the same pack doesn't re-trigger a duplicate import.The sensors-popover work that was briefly on this branch has been reverted out of it and split into its own PR, so this branch is wallpaper-only.
Validation evidence
Built via the real production path (
build-singularity.sh, noSINGULARITY_SOURCE_DIRoverride, fresh clone from this branch) on an aarch64 build host, deployed and live-tested on real Sky1/labwc hardware -- not inferred from a clean compile:gnome-look/kde-look/pling/Bing (noopendesktopduplicate).Mochis, OCS ID 2188591) was imported as the active desktop user; sidecar correctly recorded title/artist; after restartingsingularity-desktop, the browser rediscovered the import and showed "Added" (persistence across restarts confirmed, not just in-session state).tests/wallpaper_ocs_test.valaandtests/wallpaper_sidecar_test.valacover the JSON/sidecar contracts (providers/index/browse/import response shapes, missing/malformed fields, OCS vs Bing sidecar schemas, missing-sidecar-clears-attribution).meson setupconfigures with no libadwaita incore_deps,meson compilebuilds all 363 targets,meson testis 14/14 OK.Not yet verified with a real pointer (synthetic input events were rejected as outside-clicks by the layer-shell surface): the visual attribution-overlay rendering itself (applying a Bing/OCS image and confirming the on-screen overlay text) needs a manual click-through to fully close out -- the underlying data path (sidecar -> GSettings keys -> overlay properties) is verified, just not the final visual render via a real click.
Compatibility
No daemon/backend changes, no new settings/state files beyond the two new
background-attribution-title/background-attribution-authorGSettings keys (schema in the singularity-desktop superproject) -- an import is only "Added" once it's backed by a readable registered.collectionand image payload; the existing wallpaper stays active until the user explicitly picks the imported one.Rollback
Revert this branch's commits.
Assisted-by: Claude Code:claude-opus-5
AI scope: assisted with the OCS/Bing JSON adapter and sidecar parsing and their unit tests, the browser page layout and filter wiring, and the libadwaita removal; the design decisions, the live hardware testing and the review responses are mine.
AI assistance: disclosed