Show a record's full text in the evidence table - #2518
Conversation
The title column carries a git commit's whole message — a subject, a blank line, then a body that can run to dozens of lines. A single-line cell collapsed all of it into one run of words and cut it a few words in, so what read as a truncated title was really a subject with its body jammed onto the end. No column width fixes that: the longest message in a sample ran past a thousand characters, and wrapping it would make one row taller than the dialog. The cell now shows the subject alone, and a row expands to a panel holding every field in full with its line breaks intact. Expansion is keyed on the record rather than its position, so a re-sort leaves the open record open. Width was rebalanced to suit: columns holding a number or a date no longer grow, since a wider column shows no more of a six-digit figure, and a numeric heading wraps rather than claiming width the title has better use for. Signed-off-by: Alexey Panfilov <alexey.panfilov@constructor.tech>
📝 WalkthroughWalkthroughThe metric evidence table now shows first-line summaries, stable row identities, expandable full-record details, dynamic virtualized heights, and isolated copy interactions. The dialog resets expansion state when the active metric changes. ChangesMetric evidence expansion
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The PR changes how full record text is displayed and expanded without any supplied evidence of an actionable merge-blocking risk; it is merge-ready after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant MetricEvidenceTable
participant evidenceRowKeys
participant summaryLine
participant Virtualizer
MetricEvidenceTable->>evidenceRowKeys: Generate stable row keys
MetricEvidenceTable->>summaryLine: Summarize multiline values
MetricEvidenceTable->>Virtualizer: Render and measure rows
MetricEvidenceTable->>MetricEvidenceTable: Toggle expanded record
MetricEvidenceTable->>Virtualizer: Re-measure expanded detail panel
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Expansion was reachable only from a chevron a couple of characters wide at the far left, so clicking the text that is visibly cut did nothing. The whole row now toggles, and shows a pointer. The chevron stays as the control a keyboard and a screen reader use; the copy button stops the click from reaching the row. The hover text on a cell drops to the line the cell shows. It had been the whole field, which for a commit message is a thousand characters in a native tooltip — the expanded row is where that belongs. Signed-off-by: Alexey Panfilov <alexey.panfilov@constructor.tech>
…ce table Three defects a review found in the expandable row, each reproduced: The virtualizer cached a measured height under the row's index while React keyed the row by its identity, so a re-sort or a search keystroke left a tall measurement on whatever row inherited the position. It now measures under the same key. `ref` is a PR or issue number, unique only within a repository, so two records could share a key and expand together. Identity is now the whole value set, with an occurrence suffix for rows that match to the last field. A click inside the open panel bubbled to the row and closed it, so the text the panel exists to show could not be selected. Also: the row carries `aria-expanded` and the panel spans the columns, so the grid reads correctly; switching metric no longer inherits the previous one's expansion; and `summaryLine` skips leading blank lines rather than rendering an empty cell. Signed-off-by: Alexey Panfilov <alexey.panfilov@constructor.tech>
A long commit message made one row taller than the table, so reading a single record meant scrolling a list. The panel now scrolls inside itself, and the cap is a fixed height rather than a viewport fraction — vh is the window, which can exceed the table's own height and leave the record filling it anyway. Signed-off-by: Alexey Panfilov <alexey.panfilov@constructor.tech>
There was a problem hiding this comment.
🔇 Additional comments (5)
src/frontend/src/lib/metrics/evidence-rows.ts (1)
27-52: LGTM!src/frontend/src/lib/metrics/evidence-rows.test.ts (1)
6-8: LGTM!Also applies to: 132-186
src/frontend/src/components/metric-evidence-table.tsx (1)
1-11: LGTM!Also applies to: 28-47, 79-82, 119-144, 186-192, 225-234, 250-343
src/frontend/src/components/metric-evidence-table.test.tsx (1)
9-9: LGTM!Also applies to: 20-20, 60-60, 73-104, 116-276
src/frontend/src/components/metric-evidence-dialog.tsx (1)
341-344: 📐 Maintainability & Code Quality
⚠️ Unverified finding
Sandbox verification was unavailable.Verify coverage for the metric-change remount path.
Add or confirm a test that expands a record, changes the active metric, and verifies that the replacement table starts collapsed. The supplied context does not establish coverage for this path. Confirm that changed lines meet the required 80% coverage threshold. As per coding guidelines, “Ensure new and changed lines achieve at least 80% test coverage.”
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a9ef0a7b-6fee-419f-94e0-84d6a87249f6
📒 Files selected for processing (5)
src/frontend/src/components/metric-evidence-dialog.tsxsrc/frontend/src/components/metric-evidence-table.test.tsxsrc/frontend/src/components/metric-evidence-table.tsxsrc/frontend/src/lib/metrics/evidence-rows.test.tssrc/frontend/src/lib/metrics/evidence-rows.ts
Follow-up to #2514.
What was actually wrong
The title column does not carry a title. It carries a git commit's whole
message — a subject, a blank line, then a body. A single-line cell collapses
that into one run of words and cuts it a few words in, so what reads as a
truncated title is a subject with its body jammed onto the end.
Measured against a running instance, one sample of eleven commit records:
First lines were 32-100 characters. So no column width fixes this: a
thousand-character message will not fit a cell, and wrapping it would make one
row taller than the whole dialog.
What changed
The cell shows the subject. Everything up to the first newline. That alone
removes most of the visible truncation, because subjects are short.
A row expands to the full record. Every field, in full, with line breaks
intact, in a panel below the row. Expansion is keyed on the record rather than
its position, so re-sorting leaves the open record open instead of handing the
open state to whatever now sits in that slot. Row heights are measured, since
an expanded record is as tall as its message needs.
Width was rebalanced. Columns holding a number or a date no longer grow — a
wider column shows no more of a six-digit figure — and a numeric heading wraps
to two lines rather than claiming width the title has better use for. The title
column gains about a third more room.
The expander is offered on every row, not only rows with a long field: the
panel is a full-record view, useful for reading and copying any field without
scrolling the table sideways, and a control that appears only sometimes is
harder to find than one that is always there.
Verification
pnpm typecheck,eslint,vitest run --project unit(1210 passing)heights sum exactly to the body height with no overlap, and the tallest row
measured to fit its message. Re-sorting kept the same record open. No column
header or cell clips any more, except the title, which is what expansion is
for.
Summary by CodeRabbit
New Features
Bug Fixes