feat(lobby): find a thread without knowing which room holds it - #508
Open
WilliamKarolDiCioccio wants to merge 16 commits into
Open
feat(lobby): find a thread without knowing which room holds it#508WilliamKarolDiCioccio wants to merge 16 commits into
WilliamKarolDiCioccio wants to merge 16 commits into
Conversation
The only thread listing we expose is room-scoped, so an aggregated view would have had to call it once per room and merge -- downloading every thread on the server before it could render the first one. Add 'getAllThreads', which pages the new cross-room endpoint, and a 'ThreadPage' holding the page alongside its bounds. 'hasMore' compares the offset against the total rather than treating a short page as the end, so an exactly-full final page does not provoke a wasted request. The mapper preserves the order it is given: the backend already returns each room's threads contiguously, and re-sorting here would break the divider-on-room-change the listing is built around. A malformed thread fails the page instead of being skipped, since dropping a row silently would desync the rendered list from the total the caller pages against. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Groundwork for the lobby's threads tab: a signals controller that pages one server's threads, plus the persisted tab selection. The listing follows the selected server rather than spanning all of them. Each server is a separate backend, so a truly global list would mean N independent paginated fetches with no coherent order across the results -- and, once labels land, no shared label namespace either. 'loadMore' refuses to run while a page is in flight. Without that guard a scroll listener firing twice near the end would request the same offset twice and append the page twice. Every completion also re-checks that nothing moved underneath it -- cancelled, disposed, or switched server -- since writing late would show one server's threads under another's heading. A 404 becomes its own state rather than a failure: it means the server predates the endpoint, and offering a retry that will always 404 reads as a bug to the user. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The lobby listed rooms and nothing else, so reaching a thread meant first remembering where it lived, opening that room, and scanning its sidebar. Threads in rooms you had not thought to check were, in practice, lost. Split the lobby pane into Rooms and Threads tabs. The threads tab lists every thread you have on the selected server in one lazily-paged list, headed by room. Room order follows recent activity and threads sort alphabetically inside a room, both decided by the backend -- the client only starts a new heading when the room changes, so the grouping stays correct while pages are still arriving. The tabs sit under the server heading because that is what they are: sections of the selected server. Filter and sort belong to rooms alone, so they hide on the threads tab rather than sitting there inert. The two tabs live in an IndexedStack so each keeps its scroll position -- the threads list may be many pages deep by the time you flip back. Threads deliberately carry no label chips yet; that lands with labels, and the point of this list is that it stays scannable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The hand-written fixtures all used the convenient shape. A live server returns something slightly different -- the id arrives as 'thread_id' rather than 'id', the name is nested under metadata, 'runs' is null, and 'created' carries no timezone while 'last_activity' does. Every one of those is a path the mapper already handles, but nothing held them together, so a future tidy-up could drop one and still pass. Capture a response verbatim from GET /api/v1/agui/threads as a regression test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The rule beside each room name shared a Row with it: the name took a Flexible slot and the rule an Expanded one, so the two split the pane evenly no matter how short the name was. Every room rule came out at exactly half width, against the full-width rule under the server heading directly above -- so the section breaks read as stunted rather than deliberate. Put the rule on its own line under the name, where it spans the full content width. The test measures laid-out width rather than asserting on the widget tree, since the bug was entirely in how the Row distributed space. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
6 tasks
'ThreadLabel' plus the five calls the UI needs: read the catalogue, create, rename/recolour, delete, and replace the set a thread carries. 'getAllThreads' gains the matching filters. Threads carry whole labels rather than IDs, matching what the backend sends, so a chip can be painted straight from a listing instead of being joined client-side against a separately-fetched catalogue -- which would leave a window where a just-renamed label still renders under its old name. Two shapes here are deliberate and easy to "tidy" into bugs: - 'usageCount' stays null when absent rather than defaulting to zero. The server withholds it from non-administrators, and reading "you may not know" as "nothing uses this" would present a destructive delete as a harmless one. - An empty 'labelIds' omits the parameter entirely rather than sending a blank one. Clearing the last chip has to widen the listing, not empty it. 'ThreadInfo.labels' also carries a warning worth repeating here: 'ThreadInfo' equality is id-only, so relabelling a thread does *not* make two instances compare unequal. Anything that must notice a relabel has to compare the labels itself rather than lean on '=='. 'setThreadLabels' returns the whole updated thread rather than nothing. That is not a convenience: the backend commits its transaction after sending the response, so a listing requested immediately after a write can still report the old labels. Callers fold the response into local state instead of re-fetching. Verified against a live server -- a tight write-then-list loop missed its own write 7 times out of 8, and the pre-existing thread-rename route missed 8 out of 8. 'UrlBuilder.build' widens 'queryParameters' to 'Map<String, Object>' so a value may be an 'Iterable<String>', repeating the key once per element. The backend reads a repeated 'label_ids' as a list; a comma-joined value would be one label named "3,7". Dart's covariance keeps every existing 'Map<String, String>' call site compiling untouched. A duplicate name surfaces as 'ApiException' with status 409 rather than a dedicated type -- the package's own guidance puts the exception hierarchy off limits, since changes there cascade into every consumer. The docs say so explicitly, because a caller wanting to say "that name is taken" has to check the status itself. A pre-labels backend omits 'labels' entirely, which parses as none rather than failing: the lobby still has to list its threads against an older server.
Approved deviation from the design-system rule that colours come from tokens. Label swatches are data -- users pick them -- so no closed 'ChipIntent' vocabulary can express them, and a label is not a status anyway. 'SoliplexChip.colored' takes only the background. The foreground is derived, never supplied, because an open colour field otherwise invites white text on pale yellow; the gallery now carries a near-white and a near-black swatch precisely so that stays visible in the goldens. Two helpers come with it, and their point is that nothing outside this package should have to write 'Color(0x...)' for a colour that arrives as data: - 'colorFromHex' parses the server's '#RRGGBB'. It returns null rather than substituting black, so a caller picks a fallback suited to its own surface instead of rendering something that looks deliberate. - 'hashedHueColor' is 'roomAvatarColor' lifted out of the room rail, which now delegates to it. Rooms and labels demonstrably share one algorithm rather than drifting apart as two copies -- there is a test asserting the delegation, not just the behaviour, since a reimplementation would pass a behavioural test perfectly well while quietly giving the same name two different colours. Goldens regenerated on Linux, which is the CI baseline. The fake API now applies the label and name filters rather than only recording them, so a test can assert what a filtered listing renders. Its call log gained the two new fields, which meant the paging tests were comparing whole records; they now compare the fields they are actually about.
One field, two independent filters. 'Osprey Manual @manuals' narrows by name and by label at once; either half works alone. Keeping them separate here rather than flattening to one string matches how the server treats them -- name-contains and labels-any-of are different questions. Pure functions, so the fiddly parts are cheap to pin down, and there are more of those than the feature suggests: - A token replaced by nothing welds its neighbours together: 'Osprey @manuals Manual' would become 'OspreyManual'. It is replaced by a space, and runs of whitespace collapse afterwards. - Names are lower-cased, because the server folds case. Leaving it alone would make '@urgent' quietly match nothing at all -- the worst kind of search bug, since it looks like an empty result rather than an error. - A bare '@' is text, not a label. It is exactly what has been typed the instant the autocomplete opens, and an empty name would filter to nothing. - Duplicates collapse. 'any of' makes a repeat a no-op server-side, but sending it would misrepresent the question being asked. 'activeLabelToken' and 'completeLabelToken' are the autocomplete's half: what is being typed at the cursor, and how to replace it. Both read at the cursor rather than at the end of the line, so editing mid-string cannot complete against the wrong token. 'activeLabelToken' returns an empty string for a bare '@' and null once a separator ends the token -- the difference between "menu open, show everything" and "menu closed", which a single null would have conflated.
A third lobby tab: list, create, rename, recolour, delete. Non-administrators get no labels tab at all rather than a read-only one. Every control on it is an administrator action, so a read-only version would be a tab of things you cannot do -- and the catalogue stays discoverable where it actually matters, in the '@Label' autocomplete and a thread's properties. That is presentation only; the server refuses a non-administrator's write whether or not the tab was drawn, and the view still handles a refusal because access can be revoked while the page is open. Admin standing keys off the server's own 'requiresAuth' rather than off the profile being absent. A server with authentication disabled answers 404 on '/user_info' -- there is no profile to read a flag from -- and that is single-user or development mode, where the only user is necessarily in charge. Reading "no profile" as "not an administrator" there would make the tab permanently uneditable locally. Keying off 'requiresAuth' also keeps an ordinary fetch failure against a real server from handing out the administrator view, which "profile is null" would have done. Because the tab list is now per-user, the strip's TabController is rebuilt when its length changes -- a controller's length is fixed at construction -- and the active tab is resolved by position within the visible tabs. A persisted preference for the labels tab that outlives someone's access would otherwise index past the end of the strip. Mutations fold the server's response into the held list instead of refetching, for the same reason the API returns whole objects: the backend commits after responding, so a refetch issued immediately can still return the pre-write catalogue. Failures come back as sentences rather than exceptions. The duplicate name is recognised by its 409 status because the client package has no conflict type and treats its exception hierarchy as off limits; it is the one failure a user can actually fix, so it is worth the special case. The dialog stays open with the reason inline rather than closing and losing what was typed. Deleting warns with the usage count and says plainly that the threads survive. A withheld count renders as nothing at all rather than zero -- "0 threads" would present a destructive delete as a harmless one.
A per-thread overflow menu in the aggregated listing -- properties, rename, delete -- and the properties dialog itself: name, description and labels in one place. Those three fields are one dialog on purpose. 'updateThreadMetadata' replaces the metadata row wholesale, so anything editing the name alone has to remember to resend the description or silently erase it. Rename does exactly that resend, and there is a test pinning it, because the failure is invisible until someone notices a description they never deleted is gone. Every write folds the response into the held list instead of refetching. The backend commits after responding, so a listing requested right after a write can still report the old values -- verified live earlier, where a write-then-list loop missed its own write 7 times in 8. Renaming also leaves the row where it is rather than re-sorting, so the thread does not leap elsewhere the instant it is renamed. The menu copies the room sidebar's popup, including the guard that keeps the button mounted while the popup is open. Without it, the pointer leaving the row unmounts the button and the popup's completion callback short-circuits on '!mounted', dropping the selection silently. That trap was already paid for once in the sidebar; no reason to pay again. The dialog takes a saver function rather than a controller, because two unrelated screens will open it -- this listing and a room's thread sidebar -- and they hold their threads in different places. Each passes its own, which is also what folds the result back into the list it owns. Editing a label from the picker is an explicit button, not a right-click or long-press. Those would be invisible affordances on a chip nobody would think to try, and this is the only route from a thread to a label's own settings. It appears only for someone who has a labels tab to land on. The lobby's widget tests now build under the real Soliplex theme. They were using a bare MaterialApp, and the branded chips read their palette from a ThemeData extension -- so an intent-based chip threw a null check the moment one appeared in a dialog. The tests were simply less faithful than production; nothing about the widget was wrong.
Three corrections after driving the tab. The create tile moves to the head of the label list. It keeps the server sidebar's button-as-tile shape, but not its position: a server list is a handful of entries where the tail is fine, while a catalogue can run to hundreds, and burying "new" under all of them means scrolling the lot to add one. The room sidebar's thread menu gains Properties, so a thread offers the same three actions wherever it is met -- it was odd that the aggregated listing could edit a thread's labels and the room holding that very thread could not. 'ThreadListState.saveThreadProperties' takes the description as given rather than re-sending the cached one, which is what 'renameThread' has to do. The properties dialog edits both fields, so what it hands over already is the whole metadata row. Results fold into the held list rather than refetching, for the same commit-after-response reason as everywhere else in this phase. The room screen loads the catalogue on entry rather than when the dialog opens, so opening properties does not stall on a round trip. Its picker offers no "edit label" jump: there is no labels tab to land on from inside a room. The picker's edit button now appears on hover. Painted on every chip it read as a row of identical buttons competing with the labels themselves. The slot stays reserved so chips do not shuffle sideways as the pointer crosses them, and it stays put on touch, where there is no hover to wait for.
'Osprey Manual @manuals' narrows by both at once; either half works alone. Typing '@' opens a menu of matching labels, which only ever selects -- coining a label from here is impossible, because only administrators may create one. An unresolvable name is a first-class outcome, not a dropped token. '@nonsense' cannot be expressed as a filter: an empty label list means *unfiltered*, so quietly discarding the name would widen the listing to everything -- the exact opposite of what was asked. It renders "No such label", names the offending token, and issues no request at all. Similarly, an empty result now says "No matching threads" rather than "Threads you start in any room show up here". Telling someone staring at their own search that they have no threads reads as a bug. Typing is debounced so a word costs one request rather than one per keystroke, but choosing from the menu is not -- that is a decision, not typing. Enter takes the first suggestion while the menu is open, since otherwise it would commit a half-typed '@man' as a name matching nothing. The filter rides on every page, including 'loadMore''s: paging a filtered listing has to stay filtered. Re-emitting an unchanged filter is a no-op, so a rebuild does not restart paging from zero. Labels already in the query are not suggested again -- 'any of' makes a repeat a no-op, so offering it would propose a filter that changes nothing. This is the last piece of phase 2.
The menu rendered and could not be used. Three separate causes. Dismissal keyed off the field losing focus. Tapping a suggestion moves focus off the field, so the menu hid -- and unmounted the row being tapped -- before the tap completed. It now keys off tapping outside a TapRegion shared by the field and the menu, and the suggestion rows decline focus outright, so the caret stays where it was. The widget test that "passed" all along never reproduced this, because a tap in the test environment does not move focus the way a real click does; the new test asserts the field still has focus afterwards. There was no keyboard handling at all. Arrows now move a highlight, Tab and Enter take it, Escape closes. These hang off the field's own focus node so they are seen before the text editor's: otherwise the arrows move the caret and Tab leaves the field entirely. The key-up of anything acted on is swallowed too, so a Tab-up cannot reach traversal after its Tab-down was consumed. And a genuine gap the keyboard test exposed: a label name containing a space could not be written as a token at all. '@v22 Osprey' parsed as label "v22" plus the word "Osprey" -- a different query, matching no label, so the listing showed "No such label" for a name that plainly exists. The seeded catalogue has two such names, so this was not hypothetical. Tokens may now be quoted, completion emits the quoted form when a name needs it, and the menu stays open across the spaces while one is typed.
Merged
5 tasks
Open
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 1 of 3 — frontend half. Depends on soliplex/soliplex#1261; merge that
first.
The lobby listed rooms and nothing else, so reaching a thread meant first
remembering where it lived, opening that room, and scanning its sidebar. Threads
in rooms you had not thought to check were, in practice, lost.
This splits the lobby pane into Rooms and Threads tabs. The threads tab
lists every thread you have on the selected server in one lazily-paged list,
headed by room.
Against a server without the new endpoint the tab renders "Threads need a newer
server" rather than an error — so this can merge before every deployment is
upgraded, it just won't do anything useful until then.
Changes
packages/soliplex_clientThreadPage— a page plus its bounds.hasMorecompares offset against totalrather than treating a short page as the end, so an exactly-full final page
doesn't provoke a wasted request.
threadPageFromJsonpreserves the order it is given: the backend alreadyreturns each room's threads contiguously, and re-sorting would break the
divider-on-room-change the listing is built around. A malformed thread fails
the page instead of being skipped — dropping a row silently would desync the
rendered list from the
totalthe caller pages against.SoliplexApi.getAllThreads({limit, offset})— the first API method to useUrlBuilder's query parameters.Lobby
LobbyTab+LobbyTabStorage, persisted like the existing view/sort modes.Phase 2 adds a third value.
GlobalThreadsState, a signals controller that pages one server's threads:loadMorerefuses to run while a page is in flight. Without that guard ascroll listener firing twice near the end requests the same offset twice and
appends the page twice.
disposed, or switched server — since a late write would show one server's
threads under another's heading.
forever reads as a bug.
GlobalThreadsView— flattens the page in one pass, emitting a heading whenthe room changes, with scroll-triggered paging and a trailing spinner row.
are: sections of the selected server.
Notable decisions
separate backend: aggregating would mean N independent paginated fetches with
no coherent ordering across results, and — once labels land — no shared label
namespace.
visible but inert is worse than removing them.
IndexedStack, not a conditional, so each tab keeps its scroll position —the threads list may be many pages deep by the time you flip back.
this list: chips everywhere is exactly the clutter an aggregated list exists
to avoid.
TabBarin the app — the established idiom isSegmentedButton, but phase 2 adds a third tab, so tabs earn their place.LobbyTabBarowns itsTabControllerso neither layout has to thread onethrough its (already ~20-parameter) constructor.
Test Plan
flutter test— 2,369 passing, including goldensflutter analyze— zero warnings;dart formatcleanflutter build web --releaseswitching, 404-vs-failure, refresh, and page accumulation
room-name fallback, deep-link navigation, empty/error/unsupported states,
scroll-triggered paging, and full-width room rules
the live shape differs from the convenient one in four ways (
thread_idnot
id, name nested in metadata, nullruns, mixed timezone handling)SoliplexBreakpoints, light and darkReviewer notes
_WideLayoutand_NarrowLayoutalready forwarded ~20 identical parameterseach, so the new tab state is bundled into one
LobbyThreadsSectionratherthan adding four more to both.
FakeSoliplexApi.getAllThreadsreturns an empty page when unset rather thanthrowing like its siblings: the lobby builds the threads tab on every render,
so otherwise every rooms-only test would have to stub a listing it never
looks at.
mid-scroll. Accepted for v1; keyset pagination is the fix if it bites.
The 3-phase stack
Both repos, phase by phase. Each phase is two PRs, backend first.
Phase 2 stacks on this branch: it extends
ThreadInfowith labels, adds anarbitrary-colour variant to
SoliplexChip(an approved deviation fromCLAUDE.md, which requires sign-off for design-system changes), and adds athird tab to the
LobbyTabenum introduced here.A decision record covering all three phases, the alternatives rejected, and four
open questions is shared separately as a link.
Related Issues
Fixes #