Skip to content

feat(genai): Collapse and expand a whole conversation - #4459

Open
u7k4rs6 wants to merge 3 commits into
jaegertracing:mainfrom
u7k4rs6:feat/genai-collapse-all
Open

u7k4rs6 wants to merge 3 commits into
jaegertracing:mainfrom
u7k4rs6:feat/genai-collapse-all

Conversation

@u7k4rs6

@u7k4rs6 u7k4rs6 commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Which problem is this PR solving?

#4425 added per-message collapse toggles, but there was no way to collapse or expand the whole conversation at once. For longer conversations, that meant clicking through every message.

Part of #4427.

Description of the changes

The Conversation header now has Collapse all / Expand all controls when there are at least two messages.

The bulk controls and per-message toggles all update the same state, so they can't get out of sync. The collapsed state was also moved from MessageBlock into ConversationDetails, keyed by the existing message keys (system, input-N, output-N).

The initial state is set with a lazy initializer instead of an effect. Since the messages array gets recreated on each render, using an effect here would reset the collapsed state unnecessarily.

The bulk buttons don't use aria-expanded since they don't control a single region. The existing per-message accessibility behavior stays unchanged.

Nothing is persisted. The state only lasts for the mounted tab, same as the existing collapse state and format choice.

Auto-collapsing long conversations is intentionally left out for #4427. That needs a separate decision on what counts as "long" and which messages should be collapsed.

How was this change tested?

Added 5 tests covering:

  • Collapse all
  • Expand all, including individually collapsed messages
  • Individual toggles after bulk actions
  • Hiding the bulk controls for a single message
  • Existing per-message collapse behavior

Full suite: 3822 passing across 280 files (3817 on main).

oxlint exits 0 with the same 311 pre-existing warnings, and tsc-lint is clean.

Copilot AI balanced review requested due to automatic review settings September 10, 2026 06:25
@u7k4rs6
u7k4rs6 requested a review from a team as a code owner September 10, 2026 06:25

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@codecov

codecov Bot commented Sep 10, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.44%. Comparing base (a55f771) to head (c103ec5).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4459   +/-   ##
=======================================
  Coverage   92.43%   92.44%           
=======================================
  Files         366      366           
  Lines       12688    12700   +12     
  Branches     3457     3460    +3     
=======================================
+ Hits        11728    11740   +12     
  Misses        838      838           
  Partials      122      122           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

🟢 Approval recommended

The implementation is consistent, accessible, and adequately tested with no blocking defects found.

Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@u7k4rs6
u7k4rs6 requested a balanced review from Copilot September 19, 2026 21:09

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings September 20, 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings September 20, 2026 17:13
@u7k4rs6
u7k4rs6 force-pushed the feat/genai-collapse-all branch from c02fc99 to 95f464f Compare September 20, 2026 17:13

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings September 21, 2026 06:04

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

#### Which problem is this PR solving?

jaegertracing#4425 gave each message its own collapse toggle, which covers the case where
the reader already knows which message is in the way. It left no way to act on
the section as a whole: returning a folded conversation to normal means
expanding message after message, and folding a long one means the work scales
with the size of the problem.

Part of jaegertracing#4427.

#### Description of the changes

The Conversation section header gains a Collapse all / Expand all pair, shown
once there are at least two messages for it to act on. Below that the message's
own toggle already is the pair. Both controls are absolute: each writes every
message, so the section afterwards is entirely folded or entirely open rather
than partly either.

Making that possible moves the folded flag out of MessageBlock and into
ConversationDetails, held as a record keyed by the key each row already carries
(system, input-N, output-N). A message still owns its toggle, it just no longer
owns the answer, so the section's control and the per-message button cannot
disagree. MessageBlock's button, its aria-expanded contract and its collapsed
preview branch are otherwise unchanged.

The record is seeded in a lazy initializer rather than an effect. The messages
array is rebuilt on every render, so an effect keyed on it would refire on each
new array identity and discard whatever the reader had folded.

Neither bulk control reports aria-expanded. Neither owns a region of its own,
and a third expandable button would also make the per-message expanded counts
ambiguous to a screen reader.

Nothing is persisted. The state lives with the mounted tab, the same as the
per-message toggle and the format choice beside it.

Left out deliberately, and still open in jaegertracing#4427: any size threshold or
auto-collapse for a long conversation. That needs a decision about what counts
as long and which messages should start folded, and it is a separate change.

#### How was this change tested?

Five tests added to the existing collapsing block: collapse all folds every
message, expand all opens every message including ones folded individually, an
individual toggle still works after each bulk action, and the pair is absent
for a single-message conversation. The existing per-message assertions, which
count buttons by aria-expanded, still hold unchanged.

Full suite 3822 passing across 280 files, up 5 from 3817 on main. oxlint exits
0 with the same 311 pre-existing warnings, and tsc-lint is clean.

Claude-Session: https://claude.ai/code/session_01WPkifMsD3qrCAjZ5gLidPZ
Signed-off-by: Utkarsh Bahuguna <utkarshbahuguna10@gmail.com>
Lifting the folded state into ConversationDetails made the two sets of
controls agree, at the cost of turning a chevron click into a re-render
of every message in the conversation: the section re-renders, the message
list is rebuilt, and each MessageBlock re-serializes its JSON and
recompiles its Markdown for a fold it had no part in. On a conversation of
40 messages averaging 20KB that is the whole transcript per click, and
again per Collapse all.

Memoize the list so the message objects keep their identity, memoize each
message's JSON, and wrap MessageBlock in React.memo. The callbacks now
take the message's key rather than closing over it, so they stay stable
across a re-render and the memo actually holds.

No behavior change; the collapse tests cover it unchanged.

Signed-off-by: Utkarsh Bahuguna <utkarshbahuguna10@gmail.com>
Collapse all and Expand all took --text-muted from the labels next to
them, but a label is read where a control is used. That token is
half-transparent text, roughly 3.2:1 over the panel, so at 12px the two
controls sat under the 4.5:1 AA threshold until hovered - and hover is a
state a keyboard or touch reader never reaches.

Use --text-secondary, which keeps them visibly secondary to the heading
without resting below AA.

Signed-off-by: Utkarsh Bahuguna <utkarshbahuguna10@gmail.com>
@u7k4rs6
u7k4rs6 force-pushed the feat/genai-collapse-all branch from e4a8e5e to c103ec5 Compare October 12, 2026 10:52
Copilot AI balanced review requested due to automatic review settings October 12, 2026 10:52

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@u7k4rs6

u7k4rs6 commented Oct 12, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto main (a55f7710) and conflict-free again.

The one conflict was in MessageBlock: this branch had memoized messageJson so that folding
a message would not re-serialize every other message in the conversation. #4544 has since
memoized parsedParts, copyJsons and views together and moved that call site to
copyJsons[i], which covers the same cost more thoroughly, so I dropped the now-unused memo
and kept main's version.

Verified on the rebase: tsc-lint clean, fmt-lint clean, 3873 tests across 278 files pass,
plexus 115 pass, build clean. The two oxlint warnings in these files (walkJson,
PartContent) are present identically on main.

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