fix: review-panel fixes for #85#86
Open
bird-m wants to merge 7 commits intokelsonpw/terminal-lib-upgradesfrom
Open
fix: review-panel fixes for #85#86bird-m wants to merge 7 commits intokelsonpw/terminal-lib-upgradesfrom
bird-m wants to merge 7 commits intokelsonpw/terminal-lib-upgradesfrom
Conversation
`startsWith('[')` matched every log line because the wizard log format
is `[timestamp] msg`. Narrow `[` check to `[{` and `["` so only JSON
arrays are highlighted, not timestamp-prefixed lines.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Table.tsx was introduced with no consumers. Remove it to avoid dead code on merge — can be re-added in the PR that actually needs it. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…marked instance Replace hardcoded ANSI RGB values with Brand.blueOnDark and Brand.lilac via a hexToAnsi helper, keeping heading colors in sync with styles.ts. Also scope marked config to a local Marked instance instead of mutating the global singleton, preventing side effects on other marked consumers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
All other TerminalLink usages in the PR show the URL as display text. The Stripe tip was the only one using a descriptive label, which hides the URL in fallback terminals. Use the raw URL for consistency. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The gradient for the constant string "Amplitude Wizard" was recomputed on every HeaderBar render. Hoist to a module-level constant so the gradient-string interpolation runs once. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
fs.watch can fire multiple times per write. Cache the previous raw content and skip the marked.parse call when it hasn't changed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Previously hardcoded to Colors.accent. Add a `color` prop defaulting to Colors.accent so callers can use different colors (e.g. Colors.accentSecondary) without wrapping in an extra <Text>. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
🧙 Wizard CIRun the Wizard CI and test your changes against wizard-workbench example apps by replying with a GitHub comment using one of the following commands: Test all apps:
Test all apps in a directory:
Test an individual app:
Show more apps
Results will be posted here when complete. |
4 tasks
Collaborator
Trivial fix — unblocks CI
-import marked from 'marked';
+import marked, { Marked } from 'marked';
...
-const md = new marked.Marked();
+const md = new Marked();Looks like you already have this locally — just needs pushing. |
6 tasks
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.
Summary
Review-panel fixes for PR #85 (terminal rendering libraries). Each commit is independently cherry-pickable.
Confirmed Findings — Fixed
96ad5e3[timestamp]starts with[)[check to[{and["42744d4Tableprimitive had no consumers — dead code on merge7e7f2d4hexToAnsi()helper usingBrand.blueOnDarkandBrand.lilac7e7f2d4marked.use()mutated global singletonnew marked.Marked()instance52224183a66fbcbrandGradient('Amplitude Wizard')recomputed on every HeaderBar renderf03fc42renderMarkdownran full marked.parse on every fs.watch event without diffing29cc151Colors.accentwith no overridecolorpropPre-existing / FYI (not fixed — pre-dates this PR)
AmplitudeLogo.tsx:37—buildGradientsilently skips last gradient segment (8 anchors, 7 steps)AmplitudeLogo.tsx:62— Comment claims 64 stops but code sums to 58Considered & Dropped (12 findings)
as stringcast —marked.parse()is sync by default in v12Process
🤖 Generated with Claude Code review-panel
Note
Low Risk
Mostly UI/terminal rendering tweaks and dead-code removal; main risk is minor regressions in markdown/log highlighting behavior or link color defaults.
Overview
Fixes several TUI rendering issues and small performance nits.
LogViewernow uses a narrower JSON detection heuristic (avoiding false positives on timestamped lines) andReportViewerskips re-rendering markdown when file contents haven’t changed.Markdown rendering in
terminal-renderingis made more token-driven by deriving ANSI colors fromBrandviahexToAnsi()and by switching to a scopednew marked.Marked()instance instead of mutating the globalmarkedsingleton. Also hoists the header gradient title to a module constant, adds an optionalcoloroverride toTerminalLink, updates the Stripe tip to display the raw URL, and removes the unusedTableprimitive and its barrel exports.Reviewed by Cursor Bugbot for commit 29cc151. Bugbot is set up for automated code reviews on this repo. Configure here.