Rebuild the shell as a QRH bench instrument - #113
Conversation
Replace the marketing homepage with a procedure-first chassis: create-link operations, a carbon-transfer result slip, compact reference samples, and operating-state diagnostics. Keep payload, renderer, and accessibility contracts unchanged. Co-authored-by: Aanish Bhirud <baanish@users.noreply.github.com>
|
Warning Review limit reached
Next review available in: 46 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
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 (11)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughChangesThe pull request redesigns the viewer as an instrument-style interface. It adds shared shell components, staged link creation, compact artifact inspection, updated reference pages, design specifications, and revised UI tests. Instrument UI redesign
Estimated code review effort: 4 (Complex) | ~60 minutes Mergeability Score: 🔵 Low · up to The redesign is broadly mergeable, but it still has bounded follow-up risk: some sample data may be less accessible, the FAIL view may expose an incorrect heading hierarchy, and certain generated or printed headings may be duplicated or empty. Sequence Diagram(s)sequenceDiagram
actor Operator
participant ViewerShell
participant InstrumentHeader
participant ChassisRail
participant LinkCreator
participant StatusFlag
participant InstrumentFooter
Operator->>ViewerShell: Open homepage or fragment
ViewerShell->>InstrumentHeader: Render navigation and theme control
ViewerShell->>ChassisRail: Pass operating state and fragment metrics
Operator->>LinkCreator: Enter artifact data and submit
LinkCreator->>StatusFlag: Display transfer state
LinkCreator-->>ViewerShell: Render generated transfer result
ViewerShell->>InstrumentFooter: Render retention disclosure and links
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
Deploying agent-render with
|
| Latest commit: |
e2ddcb7
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://69f00deb.agent-render.pages.dev |
| Branch Preview URL: | https://cursor-qrh-carbon-instrument.agent-render.pages.dev |
Fill status flags, shorten the empty transfer slot, make the carbon slip read as a perforated copy, and stop repeating rail diagnostics in the hash bay. Co-authored-by: Aanish Bhirud <baanish@users.noreply.github.com>
Unlit STANDBY, filled READY/FAIL/HOLD, a QRH procedure plate, and a perforated carbon slip replace leftover card chrome. The viewer leads with the filename and a recessed artifact well; diagnostics stay in the chassis instead of a supplementary card. Co-authored-by: Aanish Bhirud <baanish@users.noreply.github.com>
Filename and procedure titles stay IBM Plex Sans so Fraunces remains document-only. The homepage unit test waits for the theme rocker so Next dynamic import does not throw after unmount. Co-authored-by: Aanish Bhirud <baanish@users.noreply.github.com>
|
| Filename | Overview |
|---|---|
| src/components/renderers/markdown-renderer.tsx | The revised print header fixes the previously reported duplicate title by rendering the filename label only when it exists and differs from the derived heading. |
| src/components/viewer-shell.tsx | Reorganizes the viewer around artifact identity, operating status, controls, content, and diagnostics without an accepted follow-up defect. |
| src/components/home/link-creator.tsx | Rebuilds link creation as a staged instrument workflow and normalizes optional title and filename inputs. |
| src/app/globals.css | Introduces the bench-instrument visual system and print styling without an accepted follow-up defect. |
Reviews (3): Last reviewed commit: "Restore dark contrast, print ink, and no..." | Re-trigger Greptile
Code Review SummaryStatus: No Issues Found | Recommendation: Merge OverviewThis incremental review covers the changes since the previous review at Changes reviewed: dark-theme operating color adjustments in All changed code was verified:
Files Reviewed (6 files)
Previous Review Summaries (2 snapshots, latest commit fd8ea7b)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit fd8ea7b)Status: No Issues Found | Recommendation: Merge Files Reviewed (11 files)
Previous review (commit e5a4a07)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (selected changed files in PR 113)
Reviewed by deepseek-v4-flash · Input: 27.9K · Output: 7.5K · Cached: 484.9K |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
src/components/shell/instrument-header.tsx (1)
42-57: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse
homePathas the href in both cases.When
onGoHomeis set, the href is"#". The click handler prevents default, so normal clicks work. Middle-click, open-in-new-tab, and no-JS navigation follow the raw href and land on the current URL with a bare fragment instead of the homepage. KeephomePathas the href and let the handler intercept the click.♻️ Proposed refactor
- href={onGoHome ? "#" : homePath} + href={homePath}🤖 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/components/shell/instrument-header.tsx` around lines 42 - 57, Update the anchor in the instrument header to always use homePath for href, removing the conditional "#" value while retaining the onGoHome click interception for normal clicks.src/components/viewer-shell.tsx (1)
335-335: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unreachable branch.
This block renders only when
viewerState === "error", and that value requiresparsed.ok === false. Theparsed.okbranch is unreachable, andoperating.messagealready equalsparsed.messagein the fail case.♻️ Proposed refactor
- <p>{parsed.ok ? operating.message : parsed.message}</p> + <p>{operating.message}</p>🤖 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/components/viewer-shell.tsx` at line 335, In the error-state rendering of viewer-shell, replace the conditional expression using parsed.ok with the direct parsed.message value. The viewerState === "error" path already guarantees the failed parse result, so remove the unreachable operating.message branch while preserving the displayed error text.src/components/shell/chassis-rail.tsx (1)
5-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueShare one
OperatingStateunion.
OperatingStateis declared here and again insrc/components/shell/status-flag.tsx(Line 1). The two declarations must stay in sync becausestateis forwarded straight toStatusFlag. Export the type fromstatus-flag.tsxand import it here.♻️ Proposed refactor
import { StatusFlag } from "`@/components/shell/status-flag`"; +import type { OperatingState } from "`@/components/shell/status-flag`"; import { numberFormatter } from "`@/lib/format`"; import { MAX_FRAGMENT_LENGTH } from "`@/lib/payload/schema`"; -type OperatingState = "standby" | "ready" | "fail" | "hold"; - type ChassisRailProps = {Then export the union in
src/components/shell/status-flag.tsx:-type OperatingState = "standby" | "ready" | "fail" | "hold"; +export type OperatingState = "standby" | "ready" | "fail" | "hold";🤖 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/components/shell/chassis-rail.tsx` around lines 5 - 13, Remove the local OperatingState declaration from ChassisRailProps and import the shared OperatingState type exported by status-flag.tsx. Update status-flag.tsx to export its union if it is not already exported, and keep the state prop typed with this shared definition.src/components/shell/instrument-footer.tsx (1)
49-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDeclare external links in the data instead of sniffing the href.
link.href.startsWith("http")infers the target from the string.withBasePathoutput depends on the configured base path, so this inference couples navigation behavior to configuration. Add an explicitexternalflag to each entry inplateLinks.♻️ Proposed refactor
-const plateLinks = [ - { - href: "https://github.com/baanish/agent-render", - label: "GitHub", - }, +const plateLinks = [ + { + href: "https://github.com/baanish/agent-render", + label: "GitHub", + external: true, + },Apply the same flag to the other entries, then render with:
- {...(link.href.startsWith("http") - ? { target: "_blank", rel: "noreferrer" } - : {})} + {...(link.external ? { target: "_blank", rel: "noreferrer" } : {})}🤖 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/components/shell/instrument-footer.tsx` around lines 49 - 59, Update the plateLinks entries to include an explicit external boolean for every link, then change the anchor rendering in the plateLinks.map block to use link.external for target and rel attributes instead of checking link.href.startsWith("http").
🤖 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/components/home/link-creator.tsx`:
- Around line 187-190: Update the transferName fallback chain around
generatedLink.artifact.filename, generatedLink.artifact.title, and
draft.filename to use truthiness-based fallback so empty strings are skipped and
the next non-empty value is selected. Preserve the existing fallback order.
In `@src/components/home/sample-links.tsx`:
- Around line 21-52: Update the table rendering in the sampleLinkCards map to
preserve the three-column structure declared by thead: emit separate td cells
for kind, title, and character count, placing the anchor in the title cell and
associating the count with its header. Adjust the sample-row and sample-table
styling as needed so each row remains visually unified.
In `@src/components/renderers/markdown-renderer.tsx`:
- Around line 279-282: Update the print header near the artifact heading so the
field-label kicker is rendered only when artifact.filename exists and differs
from heading; otherwise omit the <p> element entirely. Keep the <h1> rendering
of heading unchanged.
In `@src/components/viewer-shell.tsx`:
- Around line 331-342: Fix heading order in the viewerState error branch: ensure
the page’s h1 appears before the fail-panel heading, or promote the fail-panel
text to the sole h1. Update the markup around fail-panel and procedure-title
while preserving the visible layout and error-state content.
---
Nitpick comments:
In `@src/components/shell/chassis-rail.tsx`:
- Around line 5-13: Remove the local OperatingState declaration from
ChassisRailProps and import the shared OperatingState type exported by
status-flag.tsx. Update status-flag.tsx to export its union if it is not already
exported, and keep the state prop typed with this shared definition.
In `@src/components/shell/instrument-footer.tsx`:
- Around line 49-59: Update the plateLinks entries to include an explicit
external boolean for every link, then change the anchor rendering in the
plateLinks.map block to use link.external for target and rel attributes instead
of checking link.href.startsWith("http").
In `@src/components/shell/instrument-header.tsx`:
- Around line 42-57: Update the anchor in the instrument header to always use
homePath for href, removing the conditional "#" value while retaining the
onGoHome click interception for normal clicks.
In `@src/components/viewer-shell.tsx`:
- Line 335: In the error-state rendering of viewer-shell, replace the
conditional expression using parsed.ok with the direct parsed.message value. The
viewerState === "error" path already guarantees the failed parse result, so
remove the unreachable operating.message branch while preserving the displayed
error text.
🪄 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: 035e9162-ab60-4109-845d-0c928f975c5f
⛔ Files ignored due to path filters (1)
src/app/icon.svgis excluded by!**/*.svg
📒 Files selected for processing (28)
.impeccable/live/config.jsonDESIGN.mdPRODUCT.mdsrc/app/globals.csssrc/app/security/page.tsxsrc/app/url-explainer/page.tsxsrc/components/artifact-kind-icons.tssrc/components/home/link-creator.tsxsrc/components/home/sample-links.tsxsrc/components/renderers/code-renderer.tsxsrc/components/renderers/csv-renderer.tsxsrc/components/renderers/diff-renderer.tsxsrc/components/renderers/json-renderer.tsxsrc/components/renderers/markdown-renderer.tsxsrc/components/shell/chassis-rail.tsxsrc/components/shell/instrument-footer.tsxsrc/components/shell/instrument-header.tsxsrc/components/shell/status-flag.tsxsrc/components/theme-toggle.tsxsrc/components/viewer-shell.tsxsrc/components/viewer/artifact-selector.tsxsrc/components/viewer/artifact-stage.tsxsrc/components/viewer/fragment-details-disclosure.tsxsrc/components/viewer/hash-preview.tstests/components/artifact-selector.test.tsxtests/components/fragment-details-disclosure.test.tsxtests/components/viewer-shell.test.tsxtests/e2e/viewer.spec.ts
💤 Files with no reviewable changes (3)
- src/components/artifact-kind-icons.ts
- src/components/renderers/json-renderer.tsx
- tests/components/artifact-selector.test.tsx
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5a4a07278
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
FAIL is the page heading in the error state. Sample rows expose real table cells. Empty filenames no longer blank the carbon slip. Print headers stop repeating the same title. Home, footer, and status types match the review notes. Co-authored-by: Aanish Bhirud <baanish@users.noreply.github.com>
Dark operating fills now keep white legend text above 4.5:1, markdown print restores ink on headings and chips, and the carbon slip falls back to the envelope title when title and filename are empty. Co-authored-by: Aanish Bhirud <baanish@users.noreply.github.com>
Rebuilds the agent-render interface as a single bench instrument instead of a marketing page.
The homepage starts with the create-link procedure (Identify → Load → Encode). Generated output is a carbon-transfer record, not a generic result card. The viewer leads with the artifact filename; operating state, fragment budget, codec, controls, and diagnostics sit in the chassis. Failures are a promoted FAIL plate.
Payload protocol, codecs, renderers, security model, and accessibility contracts are unchanged.
Review comments on this revision: FAIL is the page heading in the error state; sample rows use real table cells; empty filenames no longer blank the transfer slip; print headers no longer repeat the same title.
Summary by CodeRabbit
New Features
Improvements