Skip to content

Add legacy diff messages with inline and expanded viewing - #202

Merged
wesbillman merged 4 commits into
mainfrom
brain/diff-messages
Sep 24, 2026
Merged

wesbillman merged 4 commits into
mainfrom
brain/diff-messages

Conversation

@wesbillman

Copy link
Copy Markdown
Collaborator

Opened by Brain on Wes's behalf.

Summary

  • Admit legacy kind-40008 diff messages through existing history/live/thread/exact/search/unread paths, preserving raw patch bytes and untrusted display metadata.
  • Add the bundled Diff viewer through an owned whole-message contribution: inline colored diffs and expanded Unified/Split viewing, reusing the shared Dialog.
  • Keep readable escaped raw patches when the plugin is disabled, fails, or cannot safely parse the patch; retain truncation warnings and validate source links.
  • Normalize explicit zero-count hunk sides from the original header (the pinned parser treats ,0 as 1), with added/deleted-file and zero-context regressions.

Sidebar panels, sending/applying patches, and repository fetching are intentionally deferred. Scoped FOUNDATION edits were authorized. Rebased onto 12a957c2, preserving the incoming reaction controls.

Validation

Checked head: a3231491d86149c47fbc4f431086f542fad827f8.

  • Normal pre-commit/pre-push hooks passed, including corporate checks, formatting/lint, TypeScript, 2,573 Vitest tests across 251 files, design types and all design guards.
  • bin/pnpm test:browser tests/browser/diffs.spec.mjs --project chromium --project webkit --no-deps --workers=1: 2 passed. Confirms inline → expanded Unified/Split, focus return, and narrow/dark scroll containment.
  • Eight zero-count parser/mounted-rich-renderer regressions failed before the fix and passed afterward. Live test asserts 40008 in the actual WebSocket REQ, not only injected callbacks.
  • Independent read-only review found the zero-count issue; it is fixed. Human feedback on the working fixture was positive.
  • git diff --check passed. Preview server stopped; isolated browser servers clean up after testing.

Test-layer accounting and remaining gates

One new browser scenario (two engines), none removed: geometry, focus and scrolling need a real browser. Parser matrices and plugin fallback/lifecycle stay in Vitest; no existing coverage was replaced. The browser case has positive pass evidence, not a browser mutation/fail-then-pass experiment.

CI is pending. Native catalog/build acceptance and real-relay acceptance were not exercised; the browser fixture uses synthetic signed events. Broader existing browser/native suites are left to CI, not claimed as locally passed. No native release certification or merge approval is implied.

Originating Buzz channel: 3f0be6cc-5221-4113-bd4e-7991388b832b (thread fdb6d7b8cbaca1df61a56a26c8a98dfd0d2d319442ac8b28a3eb04443da83b98).

Brain added 2 commits September 23, 2026 21:00
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
@wesbillman
wesbillman requested review from a team and comp615 as code owners September 24, 2026 03:01
Brain added 2 commits September 23, 2026 21:12
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
@wesbillman

Copy link
Copy Markdown
Collaborator Author

Brain, on Wes's behalf:

Fixed the diff-owned CI failure in 2bb53693; current head is 756e80687dc90caadd6997835c5d64b5bb97ed09 after integrating main e9e557c5 without rewriting published commits.

  • diffs.spec.mjs now reuses tests/browser/vite-server.mjs, removing duplicate cache setup/cleanup. The guard and all assertions are unchanged.
  • Reproduced the CI integration guard failure before editing; all three tests in tests/integration/vite-fixture.test.mjs pass afterward.
  • Diff browser scenario still passes Chromium and WebKit. No cases added/removed. Local elapsed: integration file 0.23s, browser run 8.3s; this is validation, not a controlled performance comparison.
  • Normal push hooks passed TypeScript, 2,610 Vitest tests / 252 files, design types and guards. DCO passes at the new head.

The prior run also finished with a separate WebKit failure: typeahead.spec.mjs:972, waiting for the “Delayed suggestions” popup in the offscreen-composer geometry case. That failure has not been diagnosed or fixed by this patch; no assertion, timeout or retry was relaxed. Failed job.

New CI run is pending; this is not an all-green claim. Real-relay and native desktop/catalog acceptance remain unverified.

@wesbillman
wesbillman merged commit fd8f037 into main Sep 24, 2026
12 checks passed
@wesbillman
wesbillman deleted the brain/diff-messages branch September 24, 2026 03:26

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Follow-up fixes required. This PR merged at 03:26:05 UTC while the review was in progress; these findings apply to its unchanged reviewed head 756e80687dc90caadd6997835c5d64b5bb97ed09 against base e9e557c5a78cacf84358a2b72897d798050b689a. Four P2 findings are explained inline: malformed-content loss, unsupported edit selection, keyboard-inaccessible scrolling, and the relay search-index dependency. Resolve those contracts with regression evidence; patch editing and unrelated hardening are not requested.

Validation: exact-head source tracing, focused execution of SHA-verified parser/edit helpers, and an independently reviewed reduced-DOM WebKit reproduction. No broad suites rerun. At closeout, all executed hosted checks pass, including the four browser shards and CI required; Windows native validation is skipped. The PR’s local test-count narrative is for an older head. Full-app keyboard, native catalog launch and deployed-relay acceptance remain unverified.

GitHub rejects formal changes-requested reviews from the PR author’s account, so this is a comment review. The code-review verdict remains changes required.

);
}),
);
return files.length && complete ? files : undefined;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

P2: Do not discard unparsed lines when accepting a rich diff.

Append UNPARSED_TRAILER\n to a complete patch such as:

diff --git a/a.ts b/a.ts
--- a/a.ts
+++ b/a.ts
@@ -1 +1 @@
-old
+new

The pinned parser ignores that trailing line; the count predicate still accepts the file. Focused execution of this exact-head helper with installed react-diff-view@3.3.2 returned one file containing only old and new, not raw fallback. Both views render only those parsed changes, so signed malformed content disappears. This contradicts the documented malformed-content fallback. Reject unconsumed malformed content (or otherwise retain it visibly) and cover this case, while preserving valid patch metadata.

content: projected.content,
...(projected.content !== content ? { sourceContent: content } : {}),
...(event.kind === 40002 ? { agentEnvelope: true as const } : {}),
...(event.kind === 40008

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

P2: Exclude newly admitted diff rows from ordinary message editing.

When the viewer's newest message is a kind-40008 diff, pressing Up in an empty channel/thread composer selects it: lastEditableMessage excludes membership and agent envelopes, but not row.diff. Saving any change then fails because messages.edit accepts only kinds 9 and 40002 (src/features/relay/messages.ts:81–84). It also prevents Up from reaching the previous editable message. Exact-head helper execution confirms selection of the diff followed by “Load the message before editing it”. Since sending/editing patches is out of scope, the minimal fix is to skip diff rows in edit-target selection, with a regression covering an ordinary own message followed by an own diff.

<section
className={styles.file}
key={`${file.oldPath}:${file.newPath}:${file.oldRevision}:${file.newRevision}`}
aria-label={label}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

P2: Make the overflowing diff region keyboard reachable.

This named section owns horizontal overflow (.file { overflow: auto }), including the 48rem split table at narrow widths, but has no focus target. In a reduced real-WebKit reproduction of this structure at 390px, keyboard traversal skipped it, and ArrowRight from Split left its scroll position at zero. Keyboard-only users cannot pan to the clipped code. This was a DOM/CSS reproduction, not a full-app run; the production structure matches it. Give the relevant rich/raw scroll owners keyboard focusability, accessible naming and visible focus, then exercise actual keyboard traversal/scrolling in both browser engines. The current browser test writes scrollLeft via JavaScript and therefore cannot detect this failure.

[
{
kinds: [9, 40002],
kinds: [9, 40002, 40008],

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

P2: Resolve the relay indexing dependency before claiming diff search.

Adding 40008 to the request cannot return diff hits on an allowlist-indexed relay. Current block/buzz source at d01e5f82058463709a22e93bb4cd795da5f53e10 creates search_tsv only for 0, 9, 40002, 45001 and 45003; its maintenance script has the same set. Kind-40008 rows get NULL, so the search query's search_tsv @@ ... predicate excludes them regardless of this filter. This is a source-proven fresh-install/allowlist-maintained relay limitation, not a claim about the deployed database. Coordinate the relay index change and real-relay search evidence, or explicitly defer diff search from this delivery contract. Synthetic search responses do not establish that integration.

morgmart pushed a commit that referenced this pull request Sep 24, 2026
* origin/main:
  fix: restore avatar presence controls and active-input sensing (#198)
  Add legacy diff messages with inline and expanded viewing (#202)
  Edit the latest own message with Up in the existing composer (#192)

Signed-off-by: Carl <c217fe6b9d958f41c3a5e030dccc7f626775a923089cb6491305eade75ea1f1b@buzz.block.builderlab.xyz>
morgmart pushed a commit that referenced this pull request Sep 24, 2026
…embers-dialog

* morganm/channel-members-support:
  Preserve member-add recovery across dialog lifetimes
  fix: restore avatar presence controls and active-input sensing (#198)
  Add legacy diff messages with inline and expanded viewing (#202)
  Edit the latest own message with Up in the existing composer (#192)
  Add complete reaction toggles to the message menu (#185)

Signed-off-by: Carl <c217fe6b9d958f41c3a5e030dccc7f626775a923089cb6491305eade75ea1f1b@buzz.block.builderlab.xyz>

# Conflicts:
#	src/shared/design-system/ui/Dialog.tsx
zrmarley added a commit that referenced this pull request Sep 24, 2026
…o-player-polish

* origin/main: (38 commits)
  Fix diff content fallback, keyboard scrolling and edit selection (#205)
  Standardize form controls and field feedback across Buzz (#174)
  Keep image review downloads and external opens distinct (#144)
  Verify media review comments (#166)
  Follow system appearance (#210)
  Add rich composer formatting and spoiler rendering (#203)
  feat: show roster-backed channels and managed instances in profiles (#188)
  Add new direct message flow (#156)
  Remove Home, start in Messages, and keep Channels enabled (#194)
  fix: restore avatar presence controls and active-input sensing (#198)
  Add legacy diff messages with inline and expanded viewing (#202)
  Edit the latest own message with Up in the existing composer (#192)
  Add complete reaction toggles to the message menu (#185)
  feat: add persistent community navigation rail (#191)
  test: add margin to warm-switch performance gate (#195)
  Add composer attachments and compatible media preparation (#183)
  Add reply and copying to the shared message menu (#182)
  fix: avoid idle workspace re-renders from activity and label churn (#186)
  feat: add devtools trace capture to web profiling (#180)
  Add optional channel templates, teams and personal group defaults (#181)
  ...

Signed-off-by: Zach Marley <zmarley@squareup.com>
zrmarley added a commit that referenced this pull request Sep 24, 2026
…-content-compat

* origin/main: (38 commits)
  Fix diff content fallback, keyboard scrolling and edit selection (#205)
  Standardize form controls and field feedback across Buzz (#174)
  Keep image review downloads and external opens distinct (#144)
  Verify media review comments (#166)
  Follow system appearance (#210)
  Add rich composer formatting and spoiler rendering (#203)
  feat: show roster-backed channels and managed instances in profiles (#188)
  Add new direct message flow (#156)
  Remove Home, start in Messages, and keep Channels enabled (#194)
  fix: restore avatar presence controls and active-input sensing (#198)
  Add legacy diff messages with inline and expanded viewing (#202)
  Edit the latest own message with Up in the existing composer (#192)
  Add complete reaction toggles to the message menu (#185)
  feat: add persistent community navigation rail (#191)
  test: add margin to warm-switch performance gate (#195)
  Add composer attachments and compatible media preparation (#183)
  Add reply and copying to the shared message menu (#182)
  fix: avoid idle workspace re-renders from activity and label churn (#186)
  feat: add devtools trace capture to web profiling (#180)
  Add optional channel templates, teams and personal group defaults (#181)
  ...

Signed-off-by: Zach Marley <zmarley@squareup.com>

# Conflicts:
#	docs/channels.md
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.

1 participant