feat(code): inbox toolbar, artefact parsing, and pending_input UX#2073
Open
feat(code): inbox toolbar, artefact parsing, and pending_input UX#2073
Conversation
- Harden signal report artefact normalization (numeric ids, priority case, shape inference, video placeholder when decode fails, schema mismatch logs). - Collapse bulk actions into an overflow menu; reuse InboxBulkActionButton for Snooze/Suppress/Delete; destructive menu highlight styles. - Treat pending_input like ready for summary tooltip and list status badge; prefer suggested-reviewer ordering in API sort key. - Raise Tooltip z-index; inbox sidebar copy and count badge styling tweaks.
Contributor
Prompt To Fix All With AIFix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
apps/code/src/renderer/components/ui/Tooltip.tsx:46-50
The z-index is now specified twice with the same value — `z-[200000]` in the `className` Tailwind string and `zIndex: 200000` in the `style` prop. Inline styles always win over utility classes, so the Tailwind token has no effect and will mislead future readers about what actually controls the stacking order.
```suggestion
className="dark flex items-center gap-[8px] rounded-[6px] border border-(--gray-4) bg-(--gray-2) px-[10px] py-[6px] text-(--gray-12) text-xs leading-[1.4]"
style={{
whiteSpace: isSimpleContent ? "nowrap" : "normal",
boxShadow: "0 4px 12px rgba(0, 0, 0, 0.25)",
zIndex: 200000,
```
### Issue 2 of 3
apps/code/src/renderer/api/posthogClient.ts:401-419
**Inference runs for known non-structured types**
The inference block is entered whenever `dispatchType` is not one of the three explicitly dispatched values, which includes `"video_segment"` and `"suggested_reviewers"`. If a `video_segment` artefact's content object happens to contain a `priority` field matching `P0–P4` (or an `actionability` field), it will be silently reclassified as a `priority_judgment` / `actionability_judgment` before the `video_segment`-specific path at line 443 is ever reached. The comment says "type is missing or does not match the API", so the guard should scope the inference to `dispatchType === null` (or an explicit allowlist of drifted-type values) rather than letting it run for all unmatched strings.
### Issue 3 of 3
apps/code/src/renderer/api/posthogClient.ts:452-470
**Placeholder type may not be `video_segment`**
When the binary-decode fallback fires, the returned object uses `type` set to whatever `dispatchType ?? "unknown"` resolved to. Any consumer that switches on `type` to choose a renderer (e.g. a `video_segment` card) will not match if `type` is `"unknown"` or an unexpected string, so the placeholder content shape (`session_id`, `start_time`, …) would be rendered by a catch-all/unknown branch instead of the video segment UI, potentially blanking the section the PR description says it should preserve. Hardcoding `type: "video_segment" as const` in the placeholder (or only entering this branch when `dispatchType === "video_segment"`) would make the intent explicit.
Reviews (1): Last reviewed commit: "feat(code): inbox toolbar, artefact pars..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+46
to
+50
| className="dark z-[200000] flex items-center gap-[8px] rounded-[6px] border border-(--gray-4) bg-(--gray-2) px-[10px] py-[6px] text-(--gray-12) text-xs leading-[1.4]" | ||
| style={{ | ||
| whiteSpace: isSimpleContent ? "nowrap" : "normal", | ||
| boxShadow: "0 4px 12px rgba(0, 0, 0, 0.25)", | ||
| zIndex: 9999, | ||
| zIndex: 200000, |
Contributor
There was a problem hiding this comment.
z-[200000] in the className Tailwind string and zIndex: 200000 in the style prop. Inline styles always win over utility classes, so the Tailwind token has no effect and will mislead future readers about what actually controls the stacking order.
Suggested change
| className="dark z-[200000] flex items-center gap-[8px] rounded-[6px] border border-(--gray-4) bg-(--gray-2) px-[10px] py-[6px] text-(--gray-12) text-xs leading-[1.4]" | |
| style={{ | |
| whiteSpace: isSimpleContent ? "nowrap" : "normal", | |
| boxShadow: "0 4px 12px rgba(0, 0, 0, 0.25)", | |
| zIndex: 9999, | |
| zIndex: 200000, | |
| className="dark flex items-center gap-[8px] rounded-[6px] border border-(--gray-4) bg-(--gray-2) px-[10px] py-[6px] text-(--gray-12) text-xs leading-[1.4]" | |
| style={{ | |
| whiteSpace: isSimpleContent ? "nowrap" : "normal", | |
| boxShadow: "0 4px 12px rgba(0, 0, 0, 0.25)", | |
| zIndex: 200000, |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/code/src/renderer/components/ui/Tooltip.tsx
Line: 46-50
Comment:
The z-index is now specified twice with the same value — `z-[200000]` in the `className` Tailwind string and `zIndex: 200000` in the `style` prop. Inline styles always win over utility classes, so the Tailwind token has no effect and will mislead future readers about what actually controls the stacking order.
```suggestion
className="dark flex items-center gap-[8px] rounded-[6px] border border-(--gray-4) bg-(--gray-2) px-[10px] py-[6px] text-(--gray-12) text-xs leading-[1.4]"
style={{
whiteSpace: isSimpleContent ? "nowrap" : "normal",
boxShadow: "0 4px 12px rgba(0, 0, 0, 0.25)",
zIndex: 200000,
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+401
to
+419
| // Infer structured artefacts when `type` is missing or does not match the API | ||
| // (shape matches; enums are still validated inside each normalizer). | ||
| const contentForInfer = isObjectRecord(value.content) ? value.content : null; | ||
| if (contentForInfer) { | ||
| if (optionalString(contentForInfer.signal_id)) { | ||
| const inferredFinding = normalizeSignalFindingArtefact(value); | ||
| if (inferredFinding) { | ||
| return inferredFinding; | ||
| } | ||
| } | ||
| const inferredPriority = normalizePriorityJudgmentArtefact(value); | ||
| if (inferredPriority) { | ||
| return inferredPriority; | ||
| } | ||
| const inferredActionability = normalizeActionabilityJudgmentArtefact(value); | ||
| if (inferredActionability) { | ||
| return inferredActionability; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
The inference block is entered whenever dispatchType is not one of the three explicitly dispatched values, which includes "video_segment" and "suggested_reviewers". If a video_segment artefact's content object happens to contain a priority field matching P0–P4 (or an actionability field), it will be silently reclassified as a priority_judgment / actionability_judgment before the video_segment-specific path at line 443 is ever reached. The comment says "type is missing or does not match the API", so the guard should scope the inference to dispatchType === null (or an explicit allowlist of drifted-type values) rather than letting it run for all unmatched strings.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/code/src/renderer/api/posthogClient.ts
Line: 401-419
Comment:
**Inference runs for known non-structured types**
The inference block is entered whenever `dispatchType` is not one of the three explicitly dispatched values, which includes `"video_segment"` and `"suggested_reviewers"`. If a `video_segment` artefact's content object happens to contain a `priority` field matching `P0–P4` (or an `actionability` field), it will be silently reclassified as a `priority_judgment` / `actionability_judgment` before the `video_segment`-specific path at line 443 is ever reached. The comment says "type is missing or does not match the API", so the guard should scope the inference to `dispatchType === null` (or an explicit allowlist of drifted-type values) rather than letting it run for all unmatched strings.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
452
to
470
| // The backend may return empty content objects when binary decode fails. | ||
| if (!content && !sessionId) { | ||
| return null; | ||
| return { | ||
| id, | ||
| type, | ||
| created_at, | ||
| content: { | ||
| session_id: "", | ||
| start_time: optionalString(contentValue.start_time) ?? "", | ||
| end_time: optionalString(contentValue.end_time) ?? "", | ||
| distinct_id: optionalString(contentValue.distinct_id) ?? "", | ||
| content: "", | ||
| distance_to_centroid: | ||
| typeof contentValue.distance_to_centroid === "number" | ||
| ? contentValue.distance_to_centroid | ||
| : null, | ||
| }, | ||
| }; | ||
| } |
Contributor
There was a problem hiding this comment.
video_segment
When the binary-decode fallback fires, the returned object uses type set to whatever dispatchType ?? "unknown" resolved to. Any consumer that switches on type to choose a renderer (e.g. a video_segment card) will not match if type is "unknown" or an unexpected string, so the placeholder content shape (session_id, start_time, …) would be rendered by a catch-all/unknown branch instead of the video segment UI, potentially blanking the section the PR description says it should preserve. Hardcoding type: "video_segment" as const in the placeholder (or only entering this branch when dispatchType === "video_segment") would make the intent explicit.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/code/src/renderer/api/posthogClient.ts
Line: 452-470
Comment:
**Placeholder type may not be `video_segment`**
When the binary-decode fallback fires, the returned object uses `type` set to whatever `dispatchType ?? "unknown"` resolved to. Any consumer that switches on `type` to choose a renderer (e.g. a `video_segment` card) will not match if `type` is `"unknown"` or an unexpected string, so the placeholder content shape (`session_id`, `start_time`, …) would be rendered by a catch-all/unknown branch instead of the video segment UI, potentially blanking the section the PR description says it should preserve. Hardcoding `type: "video_segment" as const` in the placeholder (or only entering this branch when `dispatchType === "video_segment"`) would make the intent explicit.
How can I resolve this? If you propose a fix, please make it concise.
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
1 participant
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.
Summary
Polishes the desktop Inbox experience and makes signal report artefact parsing more tolerant of API shape drift.
Artefact API client
p0→P0).typeis missing or mismatched, infer finding / priority / actionability fromcontentshape before giving up.video_segmentwhen binary decode yields empty content, instead of dropping the artefact.suggested_reviewers; log a sample of raw shapes when the payload parses to zero results (team + report context).Inbox UI
pending_inputis treated like a settled state for the “preliminary description” tooltip and for hiding the inline status badge (alongsideready).Misc
Test plan