fix(frontend): a section says which kind of empty it is - #2445
Merged
dzarlax merged 3 commits intoAug 12, 2026
Conversation
added 2 commits
August 11, 2026 20:47
A section a person did none of, and a section nothing feeds, are opposite findings: the first is about them, the second is about the install and is nobody's performance. The sections page already draws that line for the whole list, but the section screen itself asked only whether any metric held a value, so both collapsed into "Nothing recorded here for the selected period" — sending a reader to look for missing work that was never being measured. It now reads the same tenant-wide listing the section navigation already asks for, and says which of the two it is. That listing is part of the answer rather than decoration, so the screen waits for it instead of showing one sentence and swapping it for the other; and when the request fails it claims neither absence, because with no listing every section looks unreachable. Signed-off-by: Alexey Panfilov <Alexey.Panfilov@constructor.tech>
The section navigation drew one grey dot for both kinds of empty, so the distinction the screen behind it now makes was invisible until the reader opened the section. The standings hook has carried the flag that tells them apart since the coverage work landed; nothing read it. A grey dot now means the section reads and holds nothing for this person this period. A hollow ring means nothing feeds it — not worth opening at all until that changes. The tooltip says which. Signed-off-by: Alexey Panfilov <Alexey.Panfilov@constructor.tech>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe portal now uses metric reachability and peer data to distinguish unavailable sections, person-specific empty results, pending sections, and populated sections. ChangesSection no-data states
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant SingleGroupView
participant MetricDefinitions
participant CoverageHelpers
participant SectionUI
SingleGroupView->>MetricDefinitions: request tenant metric definitions
MetricDefinitions-->>SingleGroupView: return definitions, loading, or error
SingleGroupView->>CoverageHelpers: evaluate reachable metrics and section data
CoverageHelpers-->>SingleGroupView: return coverage state
SingleGroupView->>SectionUI: render empty state or drilldown
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/frontend/src/components/portal/context-pane.test.tsx (1)
184-187: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the exact hollow-marker styling.
The test accepts any
borderclass. A different filled background could still pass. Assert the intendedborder-muted-foreground/40class.Suggested assertion
- expect(mark.className).toContain("border"); + expect(mark.className).toContain("border-muted-foreground/40");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/frontend/src/components/portal/context-pane.test.tsx` around lines 184 - 187, Update the marker styling assertion in the relevant test to require the exact border-muted-foreground/40 class instead of the generic border class, while preserving the existing check that the marker lacks bg-muted-foreground.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/frontend/src/components/portal/context-pane.tsx`:
- Around line 526-531: Update the standing tooltip expression in the context
pane to prioritize standing.phrase whenever standing.isPending is true, before
evaluating hasData, while preserving the existing non-pending fallback behavior.
Add a title assertion in the context-pane tests verifying that pending standings
display standing.phrase.
---
Nitpick comments:
In `@src/frontend/src/components/portal/context-pane.test.tsx`:
- Around line 184-187: Update the marker styling assertion in the relevant test
to require the exact border-muted-foreground/40 class instead of the generic
border class, while preserving the existing check that the marker lacks
bg-muted-foreground.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 89600010-e74a-429c-be25-390172108909
📒 Files selected for processing (4)
src/frontend/src/components/portal/context-pane.test.tsxsrc/frontend/src/components/portal/context-pane.tsxsrc/frontend/src/components/portal/single-group-view.test.tsxsrc/frontend/src/components/portal/single-group-view.tsx
Comment on lines
526
to
531
| standing?.hasData !== false | ||
| ? standing?.phrase | ||
| : standing.peersHaveData | ||
| ? "No data this period" | ||
| : "No data reaches us for this section" | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -A18 -B5 'isPending:[[:space:]]*true' src/frontend/src/components/portal/context-pane.test.tsxRepository: constructorfabric/insight
Length of output: 543
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '500,540p' src/frontend/src/components/portal/context-pane.tsx
sed -n '130,215p' src/frontend/src/components/portal/context-pane.test.tsx
rg -n -A12 -B8 'title=|standing\?\.phrase|No data this period|No data reaches us' src/frontend/src/components/portal/context-pane.tsx src/frontend/src/components/portal/context-pane.test.tsxRepository: constructorfabric/insight
Length of output: 11120
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '540,590p' src/frontend/src/components/portal/context-pane.tsx
rg -n -A12 -B12 'isPending' src/frontend/src/components/portal/context-pane.tsx src/frontend/src/queries src/frontend/src/apiRepository: constructorfabric/insight
Length of output: 39532
Preserve the pending tooltip state.
When standing.isPending is true, use standing.phrase before evaluating hasData. Otherwise, the pending state can show "No data this period" while the marker is hidden. Add a title assertion to src/frontend/src/components/portal/context-pane.test.tsx.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/frontend/src/components/portal/context-pane.tsx` around lines 526 - 531,
Update the standing tooltip expression in the context pane to prioritize
standing.phrase whenever standing.isPending is true, before evaluating hasData,
while preserving the existing non-pending fallback behavior. Add a title
assertion in the context-pane tests verifying that pending standings display
standing.phrase.
Both coverage flags read false while the standings queries are in flight, so the section tooltip fell through to the strongest of the three claims — that nothing feeds this section — on an answer the hook had not given yet. The mark beside the row is already hidden for that reason; the words now follow it. Signed-off-by: Alexey Panfilov <Alexey.Panfilov@constructor.tech>
Contributor
Author
|
Fixed in cce2a43, and the case is worse than described: while the Falling back to The pending test asserts the absent title. |
dzarlax
enabled auto-merge
August 12, 2026 08:11
hello1101n
approved these changes
Aug 12, 2026
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
2 participants
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.
A section with nothing in it can mean two opposite things:
work in the period — a fact about them, worth asking about;
nobody's performance.
The distinction exists (
partStateinlib/insight/coverage.ts) and thesections page already draws it for the whole list via
PersonCoverage.It was lost in the two places a reader meets one section at a time.
The section screen
SingleGroupViewaskedgroupHasData— whether any metric of the groupholds a value for this person — and rendered one sentence for both
answers. Opening the one section a reader cared enough to click threw
away what the page they came from had told them.
It now reads the same tenant-wide definitions listing the section
navigation already asks for (so it is served from cache) and says which
absence it is. Two consequences worth stating:
for it rather than showing one sentence and swapping it for the other;
every section looks unreachable, and announcing that nothing is
measured here for anyone on the strength of a request that never
arrived is the same class of fabrication the coverage helper exists to
avoid.
The navigation mark
The sections nav drew one grey dot for both, so the distinction was
invisible until the section was opened. The standings hook has computed
peersHaveDatafor exactly this since the coverage work landed andnothing read it.
A filled grey dot now means the section reads and holds nothing for this
person this period. A hollow ring means nothing feeds it — not worth
opening at all until that changes. The tooltip says which.
Copy
The first two reuse the vocabulary
PersonCoveragealready uses on thesections page, so the two screens do not describe the same state in
different words.
Verification
Four new tests: the section screen names each of the three states, and
the nav marks the two empties apart. Full unit suite green (1087),
typecheck and lint clean.
Summary by CodeRabbit
New Features
Bug Fixes