Skip to content

feat(inbox): one data-bound grid — select many, Delete and Undo, mesh search, ordering, a switch per source - #6475

Merged
rbuergi merged 21 commits into
mainfrom
feat/unified-inbox
Oct 11, 2026
Merged

rbuergi merged 21 commits into
mainfrom
feat/unified-inbox

Conversation

@rbuergi

@rbuergi rbuergi commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

What

The personal Inbox (/{user}/Inbox) becomes the one place for everything that reaches a person, in the shape of a mail client: one grid, select many, Delete with Undo — and everything above the grid data-bound.

  • One DataGrid over every leg, with the platform's own selection column (WithRowSelection). Delete acts on every ticked row, Undo brings the last batch back.
  • All view state is one InboxFilter in the page's data section, and each control is bound to a field of it:
    • search box → search: the term is appended to every leg and handed to the mesh query engine under the owner's identity (the viewer is an argument of the read, never the ambient context). Not a filter over loaded rows.
    • order select → order: what needs you first (default), newest, oldest, by title, by source.
    • one switch per source → sources/{key}: show or hide what one provider brought.
  • Delete is soft, and what it means belongs to the item's type: a new IInboxItemHandler (Dismiss / Restore), registered as a service by the package that owns the type. Core brings the two it owns:
    • Notification — a new Dismissed flag (the node stays until retention removes it);
    • the user's stored mail ({user}/_Email) — Archived, and Read again on Undo. An outbound mail's send state is never touched.
  • Mail joins the inbox as a built-in leg (inbound, not archived).
  • A row whose type has no handler, or that is still running, is listed with a disabled checkbox that says why. The click re-checks the ticked keys against the rows the grid shows, confines them to the owner's partition, and issues every write under the clicker's access context.
  • A package brings a leg as a service (InboxQueries.All), with an optional SourceLabelKey for its display name. The Threads app uses this in the paired MeshWeaver.Plugins change to add the viewer's conversations per channel.

Requested by the maintainer (2026-10-11): one inbox for mail, WhatsApp and the rest, "in a structure like outlook where we can select many, then delete"; then, on the first cut: "bring it connected to mesh search", "with different ordering and switching on / off categories and alike", "all data bound". Decisions taken with him the same day: Delete is soft with Undo; mail and messages first, the Operations lists (approvals, triage, fleet issues, feedback) in a second change.

Two things worth a reviewer's eye

  • The filter is compared by value (InboxRows.Fingerprint). The data section re-emits an id whenever any id in it changes — the grid's own rows included — as a freshly deserialized record, so read naively the page feeds on its own writes. The page test fails without it.
  • A search term gets its own live read; the idle list does not. With no term a leg is its shared synced query. A synced query is kept per distinct query text for the process's life and a typed term is unbounded, so a term is a live IMeshService.Query, folded from deltas and ended when the term changes.

Why the legs negate

Dismissed defaults to false, Email.Status to New, Direction to Inbound, and a default is omitted from stored JSON. A positive match on any of them selects nothing ever written, so the legs read -content.dismissed:true, -content.status:Archived, -content.direction:Outbound.

Tests

InboxSelectAndDeleteTest (new) and the existing inbox suites:

  • the page end to end as its owner: render, tick one of two rows, Delete (only that one is written), Undo;
  • the bound controls end to end: order, a source off and on, a term through the query engine, clearing it, and that an idle page writes nothing;
  • Delete / Undo of a notification and of a mail, read back through the leg's snapshot query; a faulted write answers without quoting its cause;
  • every ordering, the switch default and key, the delta fold, the fingerprint, the partition confinement, the viewer's time zone and date format across DST and the date line.

Falsified: the mesh-backed delete tests fail when the legs stop excluding put-away items; the bound-controls test fails without the fingerprint.

Local runs: MeshWeaver.Graph.Test 2784 / 0 failed, MeshWeaver.Documentation.Test 824 / 0 failed, MeshWeaver.Messaging.Hub.Test 569 / 0 failed. The page has not been looked at in a browser yet; that check happens on the deployed instance.

Not in this change

Recorded in Doc/Architecture/CommunicationHub §11: legs contributed as nodes (needed by node-native packages such as the Hosting approvals), read state and the channel glyph on a row, and conversations that live in another partition.

Mirror sync

Mirror-sync: npm run sync:i18n -- --ref <merged core sha> runs in the paired pull request Systemorph/MeshWeaver.Plugins#3369 once this change is merged (new inbox.* keys, en + de).

Deploy

The Inbox area is served by the User hub from the platform image, so it arrives with the next image roll through CD; no node type is recompiled and no address needs a recycle.

🤖 Generated with Claude Code

…cations, mail and what packages bring

The personal Inbox (/{user}/Inbox) listed notifications and activities read-only. It is now the
one place for everything that reaches a person, in the shape of a mail client: every band is a
DataGrid with the platform's own selection column, and two toolbar buttons act on every ticked
row of every band.

- Delete is SOFT and its meaning belongs to the item's type: a new IInboxItemHandler (Dismiss /
  Restore) per node type, registered as a service by the package that owns the type. Core brings
  the two it owns - Notification (a new Dismissed flag) and the user's stored mail (Archived, and
  Read again on Undo; an outbound mail's send state is never touched). Nothing is removed from the
  mesh, and Undo brings the last batch back through the same handlers.
- A row whose type has no handler, or that is still running, is listed with a disabled checkbox
  that says why. The click re-checks the ticked keys against the rows as they stand and issues
  every write under the clicker's access context, opened around the handler call.
- Mail joins the inbox: a built-in leg over {viewer}/_Email (inbound, not archived).
- A package brings a leg as a service (InboxQueries.All), which is how the Threads app adds the
  viewer's conversations per channel in the paired MeshWeaver.Plugins change.

Each leg negates the STAMPED value it excludes (dismissed:true, status:Archived,
direction:Outbound): the defaults are omitted from stored JSON, so a positive match selects
nothing ever written.

Tests: InboxSelectAndDeleteTest - the page end to end as its owner (render, tick one row, Delete,
Undo), Delete/Undo of a notification and of a mail read back through the leg's snapshot query,
the selectability and merge rules, the viewer's time zone across DST and the date line. Both
mesh-backed delete tests fail when the legs stop excluding put-away items.

Doc: Architecture/CommunicationHub, section 6 "Select many, then act", and what is still owed in
section 11 (node-contributed legs, read state and channel glyph, other partitions).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 11, 2026 10:18
@github-actions

github-actions Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

    20 files   -   4      20 suites   - 4   52m 53s ⏱️ - 1m 6s
11 500 tests  - 242  11 500 ✅  - 49  0 💤  - 193  0 ❌ ±0 
11 514 runs   - 242  11 514 ✅  - 49  0 💤  - 193  0 ❌ ±0 

Results for commit 05ecf33. ± Comparison against base commit 670c855.

This pull request removes 347 and adds 81 tests. Note that renamed tests count towards both.

   --- End of inner exception stack trace ---
   --- End of inner exception stack trace ---, expected: True)
   --- End of inner exception stack trace ---, isDenial: True)
 ---> (Inner Exception #1) MeshWeaver.Mesh.QueryProviderStalledException: Query provider(s) [pg] did not emit an Initial within the query fan-in's 16s bound for query 'nodeType:NodeType' (user 'system'). The merged Initial gates on EVERY provider, so this query has NO snapshot to answer with — it is reported as unavailable (retryable) rather than left hanging with no error. This is an availability failure, never a permission verdict: a consumer deciding access must fail CLOSED and say it could not establish the answer. Fix the stalled provider; never bump the consumer's timeout.<---
 ---> (Inner Exception #1) MeshWeaver.Messaging.Hub.Test.InfrastructureFaultTest+ProviderException (0x80004005): Failed to connect to 10.42.18.4:5432<---
 ---> (Inner Exception #1) System.ArgumentException: Value does not fall within the expected range.<---
 ---> (Inner Exception #1) System.InvalidOperationException: boom<---
 ---> (Inner Exception #1) System.InvalidOperationException: source B is misconfigured<---
 ---> (Inner Exception #1) System.Net.Sockets.SocketException (0xFFFDFFFF): Name or service not known<---
…
Memex.Portal.Shared.Test.InstanceIdRulesMatchTheRegistryTest ‑ TheSetupHostAgreesWithTheRegistry(candidate: "7b9983a7-1ca6-4993-ad20-03638afd29a0")
Memex.Portal.Shared.Test.SessionDenialIsAnAnswerTest ‑ OnlyAVerdictReadsAsADenial(shape: "the same verdict nested, as a late denial dispatch"···, failure: System.InvalidOperationException: write failed
 ---> System.UnauthorizedAccessException: Access denied
   --- End of inner exception stack trace ---, isDenial: True)
MeshWeaver.Compiler.Pipeline.Test.SourcesWatcherStallBackoffTest ‑ The_production_classifier_recognises_a_stall_through_every_wrapping(because: "a stall as an aggregate's FIRST member", fault: System.AggregateException: One or more errors occurred. (Query provider(s) [pg] did not emit an Initial within the query fan-in's 16s bound for query 'nodeType:NodeType' (user 'system'). The merged Initial gates on EVERY provider, so this query has NO snapshot to answer with — it is reported as unavailable (retryable) rather than left hanging with no error. This is an availability failure, never a permission verdict: a consumer deciding access must fail CLOSED and say it could not establish the answer. Fix the stalled provider; never bump the consumer's timeout.)
 ---> MeshWeaver.Mesh.QueryProviderStalledException: Query provider(s) [pg] did not emit an Initial within the query fan-in's 16s bound for query 'nodeType:NodeType' (user 'system'). The merged Initial gates on EVERY provider, so this query has NO snapshot to answer with — it is reported as unavailable (retryable) rather than left hanging with no error. This is an availability failure, never a permission verdict: a consumer deciding access must fail CLOSED and say it could not establish the answer. Fix the stalled provider; never bump the consumer's timeout.
   --- End of inner exception stack trace ---, expected: True)
MeshWeaver.Compiler.Pipeline.Test.SourcesWatcherStallBackoffTest ‑ The_production_classifier_recognises_a_stall_through_every_wrapping(because: "a stall as an aggregate's SECOND member", fault: System.AggregateException: One or more errors occurred. (The operation has timed out.) (Query provider(s) [pg] did not emit an Initial within the query fan-in's 16s bound for query 'nodeType:NodeType' (user 'system'). The merged Initial gates on EVERY provider, so this query has NO snapshot to answer with — it is reported as unavailable (retryable) rather than left hanging with no error. This is an availability failure, never a permission verdict: a consumer deciding access must fail CLOSED and say it could not establish the answer. Fix the stalled provider; never bump the consumer's timeout.)
 ---> System.TimeoutException: The operation has timed out.
   --- End of inner exception stack trace ---
 ---> (Inner Exception #1) MeshWeaver.Mesh.QueryProviderStalledException: Query provider(s) [pg] did not emit an Initial within the query fan-in's 16s bound for query 'nodeType:NodeType' (user 'system'). The merged Initial gates on EVERY provider, so this query has NO snapshot to answer with — it is reported as unavailable (retryable) rather than left hanging with no error. This is an availability failure, never a permission verdict: a consumer deciding access must fail CLOSED and say it could not establish the answer. Fix the stalled provider; never bump the consumer's timeout.<---
, expected: True)
MeshWeaver.Compiler.Pipeline.Test.SourcesWatcherStallBackoffTest ‑ The_production_classifier_recognises_a_stall_through_every_wrapping(because: "a stall nested in an aggregate inside a wrapper", fault: System.InvalidOperationException: outer
 ---> System.AggregateException: One or more errors occurred. (Value does not fall within the expected range.) (Query provider(s) [pg] did not emit an Initial within the query fan-in's 16s bound for query 'nodeType:NodeType' (user 'system'). The merged Initial gates on EVERY provider, so this query has NO snapshot to answer with — it is reported as unavailable (retryable) rather than left hanging with no error. This is an availability failure, never a permission verdict: a consumer deciding access must fail CLOSED and say it could not establish the answer. Fix the stalled provider; never bump the consumer's timeout.)
 ---> System.ArgumentException: Value does not fall within the expected range.
   --- End of inner exception stack trace ---
 ---> (Inner Exception #1) MeshWeaver.Mesh.QueryProviderStalledException: Query provider(s) [pg] did not emit an Initial within the query fan-in's 16s bound for query 'nodeType:NodeType' (user 'system'). The merged Initial gates on EVERY provider, so this query has NO snapshot to answer with — it is reported as unavailable (retryable) rather than left hanging with no error. This is an availability failure, never a permission verdict: a consumer deciding access must fail CLOSED and say it could not establish the answer. Fix the stalled provider; never bump the consumer's timeout.<---

   --- End of inner exception stack trace ---, expected: True)
MeshWeaver.Compiler.Pipeline.Test.SourcesWatcherStallBackoffTest ‑ The_production_classifier_recognises_a_stall_through_every_wrapping(because: "a stall wrapped as InnerException", fault: System.InvalidOperationException: outer
 ---> MeshWeaver.Mesh.QueryProviderStalledException: Query provider(s) [pg] did not emit an Initial within the query fan-in's 16s bound for query 'nodeType:NodeType' (user 'system'). The merged Initial gates on EVERY provider, so this query has NO snapshot to answer with — it is reported as unavailable (retryable) rather than left hanging with no error. This is an availability failure, never a permission verdict: a consumer deciding access must fail CLOSED and say it could not establish the answer. Fix the stalled provider; never bump the consumer's timeout.
   --- End of inner exception stack trace ---, expected: True)
MeshWeaver.Compiler.Pipeline.Test.SourcesWatcherStallBackoffTest ‑ The_production_classifier_recognises_a_stall_through_every_wrapping(because: "an aggregate of unrelated faults", fault: System.AggregateException: One or more errors occurred. (The operation has timed out.) (Value does not fall within the expected range.)
 ---> System.TimeoutException: The operation has timed out.
   --- End of inner exception stack trace ---
 ---> (Inner Exception #1) System.ArgumentException: Value does not fall within the expected range.<---
, expected: False)
MeshWeaver.Graph.Test.InboxLegsAreAnchoredTest ‑ AUnicodeViewer_GetsEveryBuiltInLeg(viewer: "müller")
MeshWeaver.Graph.Test.InboxLegsAreAnchoredTest ‑ AUnicodeViewer_GetsEveryBuiltInLeg(viewer: "zoë-ångström")
MeshWeaver.Graph.Test.InboxLegsAreAnchoredTest ‑ OnlyALegReadingTheViewersOwnPartition_IsRun(query: "namespace:*/_Thread nodeType:Thread", run: False)
…

♻️ This comment has been updated with latest results.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Data-ID collisions and raw exception exposure can cause incorrect behavior and disclose internal details.

7 open findings
What changed in this PR

Adds a unified, mail-style personal inbox with multi-select, soft Delete, and Undo.

Changes:

  • Replaces inbox lists with selectable data grids and handler-based actions.
  • Adds inbound mail and package-contributed inbox legs.
  • Adds localization, documentation, and end-to-end coverage.
File Description
test/​MeshWeaver.Graph.Test/​InboxSelectAndDeleteTest.cs Tests selection, handlers, merging, and Undo.
test/​MeshWeaver.Graph.Test/​InboxAppTest.cs Updates distinct-band assertion.
src/​MeshWeaver.Messaging.Hub/​Localization/​strings.en.json Adds English inbox text.
src/​MeshWeaver.Messaging.Hub/​Localization/​strings.de.json Adds German inbox text.
src/​MeshWeaver.Mesh.Contract/​Notification.cs Adds notification dismissal state.
src/​MeshWeaver.Mesh.Contract/​InboxItemHandler.cs Defines extensible inbox actions.
src/​MeshWeaver.Graph/​InboxRows.cs Models and merges grid rows.
src/​MeshWeaver.Graph/​InboxQueries.cs Adds mail and contributed query legs.
src/​MeshWeaver.Graph/​InboxLayoutArea.cs Implements grids, Delete, and Undo.
src/​MeshWeaver.Graph/​InboxItemHandlers.cs Implements notification and mail handlers.
src/​MeshWeaver.Graph/​Configuration/​GraphConfigurationExtensions.cs Registers core handlers.
src/​MeshWeaver.Documentation/​Data/​Architecture/​CommunicationHub.md Documents inbox behavior and limitations.

🧠 Review effort: Balanced


💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/MeshWeaver.Graph/InboxItemHandlers.cs Outdated
Comment thread src/MeshWeaver.Graph/InboxLayoutArea.cs Outdated
Comment thread src/MeshWeaver.Graph/InboxRows.cs Outdated
Comment thread src/MeshWeaver.Graph/InboxLayoutArea.cs Outdated
Comment thread src/MeshWeaver.Graph/InboxRows.cs Outdated
Comment thread src/MeshWeaver.Graph/InboxRows.cs Outdated
Comment thread src/MeshWeaver.Messaging.Hub/Localization/strings.en.json
rbuergi and others added 4 commits October 11, 2026 12:48
…witch per source

Maintainer, 2026-10-11, on the first cut: "bring in grid with multi select and stuff", "bring it
connected to mesh search", "with different ordering and switching on / off categories and alike",
"all data bound".

The three stacked band grids become ONE grid, and everything above it is bound to one piece of
view state - an InboxFilter in the page's data section:

- search box -> `search`: the term is appended to every leg and handed to the query engine under
  the OWNER's identity (the viewer is an argument of the read, never the ambient context). With no
  term a leg is its shared synced query; a term gets its own live read, folded from deltas, ended
  when the term changes - a synced query is kept per distinct query text for the process's life,
  and a typed term is unbounded.
- order select -> `order`: what needs you first (default: band rank, newest within), newest,
  oldest, by title, by source. An order the page does not know falls back to importance.
- one switch per source -> `sources/{key}`: shows or hides what one provider brought. A source
  with no entry is shown, so a leg a package brings later never arrives switched off.

The bands are a column and a rank instead of three lists. A bulk act is confined to the owner's
partition: a search may list a node elsewhere, a ticked row outside {owner}/ is not a target.

The filter is compared by VALUE (InboxRows.Fingerprint). The data section re-emits an id whenever
any id in it changes - the grid's own rows included - as a freshly deserialized record, so read
naively the page feeds on its own writes. Caught by the page test: without the fingerprint the
search never reaches the grid.

Tests: the bound controls end to end (order, a source off and on, a term through the query
engine, clearing it, and that an idle page writes nothing), every ordering, the source switch
default, the delta fold, the fingerprint, the partition confinement. Falsified against the
missing fingerprint.

Also drops a duplicate catalog key (inbox.column.status already existed).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…lized source names and dates, unique switch keys

Six findings, all taken:

- A faulted write no longer puts its exception message in front of the viewer. An outcome's
  Reason is shown verbatim and a typed-write failure can quote the content it could not read: the
  core handlers and the page's own catch now LOG the fault and answer with no reason, and the
  summary says only that the item could not be changed.
- A source has an id and a display name. InboxQueryLeg gains SourceLabelKey (an init property, no
  signature change); the row and the switch show the catalog text in the viewer's language, the
  switch and the by-source order key on the id. Core's three legs carry keys.
- A row's date is written in the viewer's date format as well as the viewer's zone; a language tag
  the runtime does not know falls back to invariant rather than faulting the render.
- The switch key of a source ends in a stable hash of the exact id, so two ids that read the same
  once punctuation is dropped no longer share one switch. (The per-band grid and selection ids the
  finding named are gone: the page is one grid with constant ids.)
- The bulk act aggregates straight into an ImmutableList.
- The mirror hand-over is declared in the pull request body.

Tests: a faulted write answers without a reason, the label/id split, the switch key, the viewer's
date format across DST and the date line, an unreadable locale.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A node read right after an answered write can still emit the previous value first; the
assertions on a dismissed notification and an archived mail now wait for the value.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@rbuergi rbuergi changed the title feat(inbox): select many, then Delete and Undo — one inbox for notifications, mail and what packages bring feat(inbox): one data-bound grid — select many, Delete and Undo, mesh search, ordering, a switch per source Oct 11, 2026
Copilot AI balanced review requested due to automatic review settings October 11, 2026 11:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/MeshWeaver.Graph/InboxLayoutArea.cs Outdated
Comment thread src/MeshWeaver.Graph/InboxLayoutArea.cs Outdated
Comment thread src/MeshWeaver.Graph/InboxItemHandlers.cs Outdated
Comment thread src/MeshWeaver.Graph/InboxLayoutArea.cs Outdated
Comment thread src/MeshWeaver.Graph/InboxRows.cs Outdated
Comment thread src/MeshWeaver.Graph/InboxRows.cs Outdated
Comment thread src/MeshWeaver.Graph/InboxLayoutArea.cs Outdated
Comment thread src/MeshWeaver.Graph/InboxLayoutArea.cs Outdated
rbuergi and others added 2 commits October 11, 2026 14:21
…ecides a bulk act

Eight findings, all taken:

- Delete no longer prunes the client's ticked keys against the grid's rows in the data section,
  which the client can write too. The keys are intersected with the OWNER's own feed, re-read
  from the legs at the click; that feed also supplies the node type that picks each handler.
- The Undo batch is a host variable, not layout data: kept in the two-way data section it was
  whatever the client said it was, and Undo would restore paths the viewer never deleted.
- A handler answers Done from the state the content is in AFTER the write. A no-op write (an
  outbound mail, an item changed first) completed like a real one and was counted as deleted and
  added to Undo; it now answers "not done".
- A leg that cannot be read is no longer one empty emission that ends the leg. It is logged,
  named in a bound notice above the grid, and read again after 30 s - an unreadable source must
  not look like an empty one.
- A source's switch key is the id itself, hex-encoded: unique by construction. The 32-bit hash
  suffix only made a collision rarer (the review produced one; it is now a test case).
- The by-source order follows the display name and says so (the id only breaks a tie).
- No mutable List or HashSet left in the pipeline: Sections is a GroupBy into an ImmutableArray.

Tests: a ticked key the owner's feed does not list is never a target and never comes back with
Undo; a no-op write answers not-done for Dismiss and Restore; the colliding ids get distinct keys.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 11, 2026 12:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/MeshWeaver.Graph/InboxItemHandlers.cs Outdated
Comment thread src/MeshWeaver.Graph/InboxLayoutArea.cs Outdated
Comment thread src/MeshWeaver.Graph/InboxLayoutArea.cs
Comment thread src/MeshWeaver.Graph/InboxRows.cs Outdated
rbuergi and others added 2 commits October 11, 2026 15:37
… both reads run as the viewer, sources are bounded

Four findings, all taken:

- A handler answers Done only when THIS call changed the content and the content is now in the
  asked-for state. The final state alone counted an item someone else had already put away, put
  it in this click's Undo batch, and Undo would then reverse the other writer's act.
- The idle read runs as the viewer too. A leg is (re)subscribed on a data emission or a retry
  timer, where the ambient identity is gone, and a synced query created with no identity hands
  back the System-loaded stream unfiltered; the caller captured on the render turn is
  re-established around that call. (The searched read already took the viewer as an argument.)
- Each source is asked for its newest 100 rows (InboxQueries.LegLimit; a provider's own limit is
  kept), and a bound notice says so when a source has more. Unbounded, the page folded every
  historical row into memory and into the client's payload.
- "By title" and "by source" are alphabetical in the viewer's language (a culture comparer), not
  by code point.

Tests: a raced Dismiss and a Restore of something never put away both answer not-done, with the
changing calls as control; the bound and a provider's own limit; German title order against the
code-point order it replaced.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 11, 2026 13:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/MeshWeaver.Graph/InboxItemHandlers.cs Outdated
Comment thread src/MeshWeaver.Graph/InboxLayoutArea.cs Outdated
Comment thread src/MeshWeaver.Graph/InboxLayoutArea.cs
Comment thread src/MeshWeaver.Graph/InboxLayoutArea.cs Outdated
Comment thread src/MeshWeaver.Graph/InboxLayoutArea.cs Outdated
rbuergi and others added 2 commits October 11, 2026 17:50
…ch as free text, pinned bound, no timer retry, one act at a time

- InboxItemHandlers.Write builds its Update in the call (no Defer): Update captures the
  caller when constructed, and the inbox holds the clicker's scope only around the call;
  pinned by AHandlersWrite_CarriesTheIdentityOfTheCall_NotOfTheSubscription (falsified
  against the deferred build). Act also answers a handler that throws synchronously.
- The search term reaches the parser as one quoted free-text phrase, and the leg's bound
  is pinned on MeshQueryRequest.Limit — a typed OR / limit:all / namespace: cannot
  replace a leg's constraints (theory over five injections).
- A leg that cannot be read is named and re-read on a search change or reopen, not on a
  30 s timer.
- Delete and Undo hold the page one at a time (InboxActs: atomic claim + batch), so
  neither overwrites the Undo batch the other is writing.
- The page test now deletes and restores TWO rows.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 11, 2026 15:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/MeshWeaver.Graph/InboxItemHandlers.cs
Comment thread src/MeshWeaver.Graph/InboxLayoutArea.cs
Comment thread src/MeshWeaver.Graph/InboxQueries.cs Outdated
Comment thread src/MeshWeaver.Documentation/Data/Architecture/CommunicationHub.md
…pped notice, hidden obligations counted, superseded filter-bar section rewritten

- InboxQueries.EffectiveLimit reads the bound through the parser: limit:all, non-positive
  and over-cap limits are capped at LegLimit; a smaller positive limit is kept. Bounded
  and the searched read's MeshQueryRequest.Limit both use it, and the capped notice
  judges each leg by its own bound.
- A source switched off that hides Needs-you rows is counted in a bound notice
  (inboxHidden) — the doc's 'never silently hide an obligation' rule, now enforced.
- CommunicationHub: the MeshSearchControl filter-bar section is rewritten as what
  shipped and why; §11 records the mirror-judged Done race.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 11, 2026 16:04
Copilot AI balanced review requested due to automatic review settings October 11, 2026 16:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The UI retains a forbidden hand-written concurrency serializer and has incorrect progress and singular-count rendering.

1 open finding
1 resolved since last review
Previously missed (2)

In code that hasn't changed since last review

Medium severity Use one-based progress numerator to report accurate completion

src/​MeshWeaver.Graph/​InboxLayoutArea.cs:465

The status says item i + 1 is being processed, but the fraction reports i / count; a one-item operation therefore reports 0%, and the final item never reports 100%. Use the same one-based numerator as the displayed status.

Low severity Use plural-aware localization for hidden item counts

src/​MeshWeaver.Graph/​InboxLayoutArea.cs:236

This count uses a non-plural-aware catalog key, so hidden == 1 renders grammatically incorrect text in both catalogs (1 item(s) ... are / 1 Eintrag/Einträge ... sind). Add .one/.other variants and call host.LocalizePlural(...), the repository's existing pluralization surface (LayoutAreaLocalizationExtensions.cs:49-51).

🧠 Review effort: Balanced

Comment thread src/MeshWeaver.Graph/InboxLayoutArea.cs Outdated
…o Interlocked

Answers review round 8: batch, fresh flag and the busy claim are folded from a subject
of changes (Synchronize().Scan) into a BehaviorSubject; the claim and its release are
changes on the same channel; changes after the page closed are dropped. The concurrency
test fails without the Synchronize step (3/3).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 11, 2026 16:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Host teardown has a late-callback disposal race, and the public handler outcome contract is documented incorrectly.

1 open finding
1 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Low severity Document Done's required state transition semantics

src/​MeshWeaver.Mesh.Contract/​InboxItemHandler.cs:9

The public Done documentation omits the transition requirement enforced by Write: an item already in the requested state must return false, otherwise it enters this click's Undo batch and Undo can reverse another writer's action. State that Done requires both a change by this call and the requested final state so package handlers implement the same contract.

🧠 Review effort: Balanced

Comment thread src/MeshWeaver.Graph/InboxLayoutArea.cs Outdated
rbuergi and others added 2 commits October 11, 2026 18:48
…is ignored, never thrown

Answers review round 9: IsDisposed-then-OnNext raced Dispose. Dispose now completes the
subject (which ignores later OnNext) and ends the fold; pinned by
AChangeAfterThePageClosed_IsDropped_NotThrown.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 11, 2026 16:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Contributed legs lack runtime anchoring and projection enforcement, while custom bands cannot be localized.

5 open findings
1 resolved since last review

🧠 Review effort: Balanced

Comment thread src/MeshWeaver.Graph/InboxQueries.cs
Comment thread src/MeshWeaver.Graph/InboxLayoutArea.cs Outdated
Comment thread src/MeshWeaver.Graph/InboxQueries.cs Outdated
Comment thread src/MeshWeaver.Graph/InboxRows.cs
Comment thread src/MeshWeaver.Mesh.Contract/InboxItemHandler.cs Outdated
… projection, provider band labels, closed order set, Restore contract

- InboxQueries.Resolve runs only legs whose first term reads exactly the viewer's own
  partition (no wildcard, alternation or partitions:all); Refused names the rest and the
  page logs them as errors.
- Bounded appends RowProjection to every leg unless it already projects exactly that.
- InboxQueryLeg.BandLabelKey localizes a provider-defined band.
- InboxOrder documented as a closed set; IInboxItemHandler.Restore returns the item into
  its leg, not an exact inverse (doc page too).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 11, 2026 17:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/MeshWeaver.Graph/InboxQueries.cs Outdated
…ural term, effective path in the viewer's partition

Answers review round 11: a later namespace:/path: replaced the first, so a leg could pass
on its first anchor and run on its second. IsAnchored now requires exactly one
namespace:/path: term and validates ParsedQuery.Path / Paths / CrossPartition.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 11, 2026 17:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unicode partition users lose all inbox legs, and cancellation can leave committed changes outside the Undo batch.

2 open findings
1 resolved since last review

🧠 Review effort: Balanced

Comment thread src/MeshWeaver.Graph/InboxLayoutArea.cs Outdated
Comment thread src/MeshWeaver.Graph/InboxQueries.cs Outdated
…d click; Unicode-safe anchor guard

- Act runs each item's write on its own subscription (Detached): cancelling the click
  stops items not yet started but never the bookkeeping of a write already posted.
- IsAnchored compares the first term's path to the viewer's id itself (no ASCII regex),
  so a Unicode partition key such as müller keeps every leg.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 11, 2026 17:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Idle inbox queries accumulate process-lifetime subscriptions and snapshots for every visitor and leg.

1 open finding
2 resolved since last review

🧠 Review effort: Balanced

Comment thread src/MeshWeaver.Graph/InboxLayoutArea.cs Outdated
…cess-wide synced query

Answers review round 13: GetQuery is cached for the process's life per query text, and
legs are owner-resolved, so it kept a subscription + snapshot per leg for every person who
ever opened their inbox. The idle branch now uses the same IMeshService.Query + Fold as
the searched one, ended with the page; QueryId is gone.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 11, 2026 17:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Search text handling, pluralized UI output, and the public Undo contract need correction.

0 open findings

1 resolved since last review
Previously missed (3)

In code that hasn't changed since last review

Medium severity Use pluralized localization for hidden-row count messages

src/​MeshWeaver.Graph/​InboxLayoutArea.cs:240

This count is formatted with a single non-pluralized template, so one hidden row renders as “1 item(s) needing you are…” (and the German catalog similarly renders “Eintrag/Einträge”). Use LocalizePlural with .one/.other catalog entries so both singular and plural messages are grammatical.

Medium severity Preserve apostrophes in quoted mesh search phrases

src/​MeshWeaver.Graph/​InboxLayoutArea.cs:314

This also strips apostrophes even though the generated phrase is double-quoted, so a search such as O'Brien is sent to the mesh as OBrien. The SQL/text-search path can then miss the stored title or sender. Only double quotes can terminate this phrase; preserve single quotes and add that case to the injection theory.

Low severity Correct Done semantics for Undo batch transitions

src/​MeshWeaver.Mesh.Contract/​InboxItemHandler.cs:9

Done is also the signal that adds an item to the Undo batch, so “already in the requested state” must return false. The current public contract says the opposite and can lead package handlers to make Undo reverse another writer’s action. Document that only a transition performed by this invocation counts.

🧠 Review effort: Balanced

@rbuergi
rbuergi merged commit 0b96842 into main Oct 11, 2026
72 of 74 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants