Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds a new interactive "quickstart" command: a first-run setup wizard that checks authentication, suggests three starter interests, creates them sequentially, triggers tweet ingestion, and displays an inbox preview via a React Ink component. Changes
Sequence DiagramsequenceDiagram
participant User
participant Quickstart as Quickstart Component
participant GraphQL as GraphQL API
User->>Quickstart: Launch quickstart command
Quickstart->>GraphQL: BOOTSTRAP_QUERY (auth check, fetch user/projects)
GraphQL-->>Quickstart: User & projects data
alt Projects exist
Quickstart-->>User: Navigate to inbox
else No projects
Quickstart-->>User: Show starter suggestions
User->>Quickstart: Confirm suggestions
Quickstart->>GraphQL: CREATE_MUTATION (interest A)
GraphQL-->>Quickstart: Created
Quickstart->>GraphQL: CREATE_MUTATION (interest B)
GraphQL-->>Quickstart: Created
Quickstart->>GraphQL: CREATE_MUTATION (interest C)
GraphQL-->>Quickstart: Created
Quickstart->>GraphQL: INGEST_MUTATION (index tweets)
GraphQL-->>Quickstart: Indexed
Quickstart->>GraphQL: INBOX_QUERY (fetch initial items)
GraphQL-->>Quickstart: Inbox items
Quickstart-->>User: Display inbox
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 8-22: Update the 0.2.0 changelog entry to include the new
quickstart command by adding a bullet under the "Added" section that mentions
the new "sonar quickstart" command (e.g., "feat(quickstart): add sonar
quickstart command to bootstrap projects"), ensuring it appears alongside the
existing "feat(config data): add sqlite backup/restore/verify commands" entry
and references the same "## [0.2.0] - 2026-02-23" header so release notes match
the PR changes.
In `@src/commands/quickstart.tsx`:
- Around line 101-109: The empty-inbox messaging is misleading because we don't
distinguish whether interests were just created vs the user already had none;
update the Phase union so the 'inbox-empty' variant carries context (e.g., {
type: 'inbox-empty'; created?: boolean } or createdCount:number), then update
the code paths that set phase to 'inbox-empty' (the places that currently route
to 'inbox' or 'inbox-empty' around the confirm/creation flow) to pass the
appropriate context (created=true when interests were just created, false when
none existed), and finally change the UI rendering for the 'inbox-empty' branch
to choose copy based on that new field; reference: Phase type, 'inbox-empty'
phase, and the branches that set phase to 'inbox'/'inbox-empty'.
|
@1a35e1 both CodeRabbit threads resolved — ready for re-review. Changes in 6872ef5:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/commands/quickstart.tsx (1)
101-110: Remove or implement the unusedinbox-emptyphase.
It’s declared but never set, and the render switch has no case for it. Consider dropping it to avoid dead code (or add a render path if you intend to use it).♻️ Proposed cleanup
type Phase = | { type: 'loading' } | { type: 'unauthenticated' } | { type: 'error'; message: string } | { type: 'confirm'; me: Account; suggestions: InterestDraft[] } | { type: 'creating'; suggestions: InterestDraft[]; progress: number } | { type: 'ingesting' } | { type: 'inbox'; items: Suggestion[]; created: boolean } - | { type: 'inbox-empty' }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/quickstart.tsx` around lines 101 - 110, The union type Phase declares a variant '{ type: 'inbox-empty' }' that is never set or handled in the render switch; remove this unused 'inbox-empty' member from the Phase type to eliminate dead code, or alternatively implement a render branch and any state transitions that set Phase to { type: 'inbox-empty' } (update places that construct Phase values and the switch in the component rendering logic to include a case for 'inbox-empty', e.g., in the quickstart component that consumes Phase and the switch that renders Suggestion/inbox states).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/quickstart.tsx`:
- Around line 1-2: Replace any direct process.exit() calls in the quickstart
component with Ink's graceful shutdown via useApp().exit(): import useApp from
'ink' (add to the existing import list) and call const { exit } = useApp()
inside the component where you currently call process.exit(), then invoke exit()
instead of process.exit() to allow Ink to run its cleanup routines; update all
occurrences (e.g., places tied to the quickstart component lifecycle or
handlers) to use exit() rather than process.exit().
---
Nitpick comments:
In `@src/commands/quickstart.tsx`:
- Around line 101-110: The union type Phase declares a variant '{ type:
'inbox-empty' }' that is never set or handled in the render switch; remove this
unused 'inbox-empty' member from the Phase type to eliminate dead code, or
alternatively implement a render branch and any state transitions that set Phase
to { type: 'inbox-empty' } (update places that construct Phase values and the
switch in the component rendering logic to include a case for 'inbox-empty',
e.g., in the quickstart component that consumes Phase and the switch that
renders Suggestion/inbox states).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CHANGELOG.mdsrc/commands/quickstart.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- CHANGELOG.md
|
@1a35e1 all threads resolved — ready for re-review. |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/commands/quickstart.tsx (1)
259-278: Past review feedback addressed — contextual empty-inbox copy looks good.The
createdboolean cleanly distinguishes between newly-created interests and pre-existing ones, showing appropriate messaging in each case.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/quickstart.tsx` around lines 259 - 278, The empty-inbox copy logic in InboxView correctly uses the created boolean to toggle messages; no functional fix required—keep the conditional rendering in function InboxView({ items, created }) as-is, ensuring the created prop is passed where InboxView is rendered so the two branches (created true vs false) display the intended messages.
🧹 Nitpick comments (3)
src/commands/quickstart.tsx (3)
12-36: Over-fetching PII and unused fields in the bootstrap query.
me.email(line 17) is fetched but never used in this component — the only field consumed isxHandle. Similarly, mostprojectsfields are unused; onlyprojects.lengthis checked. Trimming the query to only the fields you need reduces the PII surface area and payload size.♻️ Suggested trimmed query
const BOOTSTRAP_QUERY = ` query QuickstartBootstrap { me { - accountId - email xHandle - xid - isPayingCustomer - indexingAccounts - indexedTweets - pendingEmbeddings - twitterIndexedAt - refreshedSuggestionsAt } projects { id: nanoId - name - description - keywords - relatedTopics - version - createdAt - updatedAt } } `🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/quickstart.tsx` around lines 12 - 36, The BOOTSTRAP_QUERY is over-fetching PII and unused project fields; change the GraphQL query string BOOTSTRAP_QUERY to only request me.xHandle (remove me.email and other me fields not used) and only the minimal projects data required (remove all project subfields since the component only uses projects.length), then update any references to those removed fields in this file (ensure only xHandle and projects.length are relied upon).
143-150:relativeTimecan produce"NaNd"for invalid dates.If
dateStris malformed,new Date(dateStr).getTime()isNaN, and the function returns"NaNd". A defensive guard would improve the user-facing output.♻️ Suggestion
function relativeTime(dateStr: string): string { const diff = Date.now() - new Date(dateStr).getTime() + if (Number.isNaN(diff)) return '?' const mins = Math.floor(diff / 60000) if (mins < 60) return `${mins}m`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/quickstart.tsx` around lines 143 - 150, The relativeTime function can return "NaNd" for invalid dates; update relativeTime to validate the parsed timestamp (e.g., const ts = new Date(dateStr).getTime()) and if Number.isNaN(ts) return a safe fallback string like "—" or "unknown" instead of continuing; otherwise compute diff/mins/hours/days as before. Ensure you update the variable names used in the guard (ts/diff/mins) in the function so the early return prevents any further math on NaN.
358-390: Consider guarding against double-invocation ofhandleConfirm.If a user presses Enter/Y rapidly,
handleConfirmcould potentially be called twice before React re-renders and unmounts theConfirmView(and itsuseInput). This would create duplicate interests. A simple ref guard would prevent it.♻️ Suggestion
+ const confirmedRef = useRef(false) + const handleConfirm = async (suggestions: InterestDraft[]) => { + if (confirmedRef.current) return + confirmedRef.current = true setPhase({ type: 'creating', suggestions, progress: 0 })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/quickstart.tsx` around lines 358 - 390, Add a simple re-entrancy guard to prevent double-invocation of handleConfirm by creating a ref (e.g., confirmingRef) and checking it at the top of handleConfirm (if confirmingRef.current return), set confirmingRef.current = true immediately before the work begins, and clear it in a finally block after the try/catch so subsequent calls are allowed; reference handleConfirm and abortedRef to integrate the guard (keep the existing abortedRef checks) so repeated rapid submissions cannot create duplicate interests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/quickstart.tsx`:
- Around line 399-432: The switch over phase.type in quickstart.tsx currently
lacks handling for the 'inbox-empty' variant and has no default/exhaustiveness
check; update the switch in the component that returns various views (the switch
that returns Spinner, UnauthenticatedView, ConfirmView, CreatingView, InboxView)
to either add a case 'inbox-empty' that renders an appropriate empty-state view
(e.g., a Text or EmptyInboxView) or add a default branch (or a TypeScript
exhaustiveness assertion using never) so any future phase types will fail
loudly; reference the existing cases (e.g., 'inbox', 'creating', 'confirm') and
the handlers used (handleConfirm, handleAbort) when adding the new case/default
to keep behavior consistent.
- Around line 241-254: The Spinner is rendering its fallback "Loading..." text
next to the interest name; in the suggestions.map block replace the bare
<Spinner /> with one that supplies an empty label to suppress the fallback
(e.g., <Spinner label="" />) so only the spinner glyph appears next to the
sibling <Text> showing s.name; update the Spinner usage in the suggestions.map
JSX accordingly (keep the surrounding conditional logic intact).
- Around line 188-194: The handler passed to useInput is treating input === ''
as confirmation which causes non-printable keys (arrows, Tab, etc.) to trigger
onConfirm; remove the empty-string check and only call onConfirm for key.return
or explicit 'y'/'Y' input, while keeping onAbort for 'n'/'N' and key.escape;
update the useInput callback (the anonymous function that calls
onConfirm/onAbort) to stop treating input === '' as confirmation so
non-printable keys no longer cause accidental confirms.
- Around line 101-109: The Phase union includes an unused 'inbox-empty' variant
which makes the switch on phase.type non-exhaustive; either remove the '{ type:
"inbox-empty" }' member from the Phase type, or update the render switch that
branches on phase.type to handle it (add a case 'inbox-empty' that returns the
appropriate UI or add a final default/exhaustiveness branch that returns a safe
fallback). Locate the discriminated union named Phase and the component switch
on phase.type (the render logic handling 'loading', 'unauthenticated', 'error',
'confirm', 'creating', 'ingesting', 'inbox') and implement one of these two
fixes so the union and switch remain consistent.
---
Duplicate comments:
In `@src/commands/quickstart.tsx`:
- Around line 259-278: The empty-inbox copy logic in InboxView correctly uses
the created boolean to toggle messages; no functional fix required—keep the
conditional rendering in function InboxView({ items, created }) as-is, ensuring
the created prop is passed where InboxView is rendered so the two branches
(created true vs false) display the intended messages.
---
Nitpick comments:
In `@src/commands/quickstart.tsx`:
- Around line 12-36: The BOOTSTRAP_QUERY is over-fetching PII and unused project
fields; change the GraphQL query string BOOTSTRAP_QUERY to only request
me.xHandle (remove me.email and other me fields not used) and only the minimal
projects data required (remove all project subfields since the component only
uses projects.length), then update any references to those removed fields in
this file (ensure only xHandle and projects.length are relied upon).
- Around line 143-150: The relativeTime function can return "NaNd" for invalid
dates; update relativeTime to validate the parsed timestamp (e.g., const ts =
new Date(dateStr).getTime()) and if Number.isNaN(ts) return a safe fallback
string like "—" or "unknown" instead of continuing; otherwise compute
diff/mins/hours/days as before. Ensure you update the variable names used in the
guard (ts/diff/mins) in the function so the early return prevents any further
math on NaN.
- Around line 358-390: Add a simple re-entrancy guard to prevent
double-invocation of handleConfirm by creating a ref (e.g., confirmingRef) and
checking it at the top of handleConfirm (if confirmingRef.current return), set
confirmingRef.current = true immediately before the work begins, and clear it in
a finally block after the try/catch so subsequent calls are allowed; reference
handleConfirm and abortedRef to integrate the guard (keep the existing
abortedRef checks) so repeated rapid submissions cannot create duplicate
interests.
First-run setup wizard that guides new users through: 1. Auth check — friendly exit with setup instructions if no API key 2. Bootstraps in a single GraphQL call (me + projects) 3. If interests already exist, jumps straight to inbox display 4. If no interests: proposes 3 starter interest drafts based on the user's X profile (sensible defaults for the tech/AI crowd) 5. Interactive Y/n confirmation prompt via Ink useInput 6. Creates each interest sequentially with live progress display 7. Triggers indexTweets ingest mutation 8. Shows current inbox items, or an empty-state message with actionable next steps if indexing hasn't completed yet Also adds 'quickstart' to the top-level command list in src/commands/index.tsx.
- CHANGELOG.md: add sonar quickstart command to 0.2.0 release notes - quickstart.tsx: distinguish 'created' vs 'pre-existing' interests in InboxView; 'Interests created and indexing triggered!' now only shows when interests were just created; pre-existing interests path shows 'Your interests are set up — indexing is in progress.' instead
Avoids skipping Ink's cleanup which can leave the terminal in a bad state. useApp() is called at component level per React hooks rules and exit() is destructured for use inside the handleAbort callback.
c29e0ca to
b46d8e7
Compare
- Fix non-exhaustive switch: add 'inbox-empty' render case and wire up the phase in handleConfirm when inbox returns empty results - Fix useInput confirm guard: remove empty-string match so non-printable keys (arrows, Tab, etc.) no longer accidentally trigger onConfirm - Fix Spinner in CreatingView: pass label="" to suppress default 'Loading…' text - Remove dead code: inbox-empty union variant is now fully wired up
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/commands/quickstart.tsx (1)
108-109:inbox-emptyis now wired but remains a redundant phase variant.
case 'inbox-empty'at line 438 renders<InboxView items={[]} created={true} />— which is identical to what theinboxphase already renders whenitems.length === 0andcreated === true. The separate type member adds an extra variant to the union, and itscreated={true}hardcode would silently produce wrong copy if another code path ever set this phase. Collapsing it into the existinginboxbranch removes the coupling:♻️ Proposed simplification — drop
inbox-emptytype Phase = | { type: 'loading' } | { type: 'unauthenticated' } | { type: 'error'; message: string } | { type: 'confirm'; me: Account; suggestions: InterestDraft[] } | { type: 'creating'; suggestions: InterestDraft[]; progress: number } | { type: 'ingesting' } | { type: 'inbox'; items: Suggestion[]; created: boolean } - | { type: 'inbox-empty' }- setPhase( - inbox.suggestions.length === 0 - ? { type: 'inbox-empty' } - : { type: 'inbox', items: inbox.suggestions, created: true } - ) + setPhase({ type: 'inbox', items: inbox.suggestions, created: true })- case 'inbox-empty': - return <InboxView items={[]} created={true} /> } }Also applies to: 386-390, 437-439
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/quickstart.tsx` around lines 108 - 109, The union variant "inbox-empty" is redundant and causes duplicated rendering; remove the "inbox-empty" member from the phase union and any code that sets that phase (replace with { type: 'inbox', items: [], created: true }), delete the `case 'inbox-empty'` branch in the switch that renders <InboxView ...> and let the existing `case 'inbox'` (which already handles items.length === 0 && created === true) render the empty state; update any creators/dispatchers that emitted 'inbox-empty' to emit the 'inbox' shape instead and ensure InboxView usage remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/commands/quickstart.tsx`:
- Around line 108-109: The union variant "inbox-empty" is redundant and causes
duplicated rendering; remove the "inbox-empty" member from the phase union and
any code that sets that phase (replace with { type: 'inbox', items: [], created:
true }), delete the `case 'inbox-empty'` branch in the switch that renders
<InboxView ...> and let the existing `case 'inbox'` (which already handles
items.length === 0 && created === true) render the empty state; update any
creators/dispatchers that emitted 'inbox-empty' to emit the 'inbox' shape
instead and ensure InboxView usage remains unchanged.
inbox-empty was a dead variant that rendered <InboxView items={[]} created={true} />
— identical to what the inbox phase already produces when items is an empty array.
Collapse the two by passing items: [] directly into the inbox phase.
Closes CodeRabbit nitpick on PR 1a35e1#7.
|
Addressed the |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
… re-entrancy guard in handleConfirm - BOOTSTRAP_QUERY: trim to xHandle + projects.id only (removes email, xid, isPayingCustomer, and all other unused me fields; projects trimmed to id only since we only check .length) - relativeTime(): add isNaN guard — invalid date strings now return '?' instead of 'NaNd' - handleConfirm: add confirmedRef to prevent double-invoke on rapid keypresses - Drop now-unused Account and Interest type imports
|
Addressed remaining CodeRabbit feedback in f3d07cc:
@coderabbitai review |
|
I'll review the changes now. ✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/commands/quickstart.tsx (1)
35-44: Trim mutation selection set to fields actually needed.The quickstart flow ignores the returned project payload; requesting full project fields increases response size per create step without benefit.
♻️ Suggested simplification
createOrUpdateProject(input: { nanoId: $nanoId name: $name description: $description keywords: $keywords relatedTopics: $relatedTopics }) { id: nanoId - name - description - keywords - relatedTopics - version - createdAt - updatedAt }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/quickstart.tsx` around lines 35 - 44, The mutation selection set in the quickstart flow is requesting full project fields (id, name, description, keywords, relatedTopics, version, createdAt, updatedAt) even though the returned payload is ignored; update the GraphQL mutation used in quickstart.tsx (the create project mutation where nanoId is passed) to request only the fields the client actually uses (e.g., id and name or just id) to reduce response size and latency. Locate the create-project mutation invocation in quickstart.tsx and remove the unnecessary fields from its selection set so only the minimal required fields remain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/quickstart.tsx`:
- Around line 124-133: The relativeTime function can return negative values when
the provided date is in the future; update relativeTime (the function named
relativeTime(dateStr: string): string) to clamp negative time differences to
zero before computing mins/hours/days—e.g., compute diff then if diff < 0 set
diff = 0 (or use Math.max(0, diff)) so mins/hours/days never become negative,
leaving the existing isNaN handling intact.
- Around line 135-139: The hasToken function currently treats whitespace-only
API keys as valid; update it to consider trimmed values only by checking
process.env.SONAR_API_KEY and config.token after trimming (e.g., use .trim() and
verify non-empty) so that pure-whitespace strings are treated as
unauthenticated; refer to the hasToken function and the readConfig() result
(config.token) when making this change.
---
Nitpick comments:
In `@src/commands/quickstart.tsx`:
- Around line 35-44: The mutation selection set in the quickstart flow is
requesting full project fields (id, name, description, keywords, relatedTopics,
version, createdAt, updatedAt) even though the returned payload is ignored;
update the GraphQL mutation used in quickstart.tsx (the create project mutation
where nanoId is passed) to request only the fields the client actually uses
(e.g., id and name or just id) to reduce response size and latency. Locate the
create-project mutation invocation in quickstart.tsx and remove the unnecessary
fields from its selection set so only the minimal required fields remain.
- clamp relativeTime diff to Math.max(0, diff) to prevent negative values when passed a future date - trim SONAR_API_KEY and config.token in hasToken() so whitespace-only strings are treated as unauthenticated - trim createOrUpdateProject selection set to only the consumed nanoId field (return value is unused)
Overview
Adds a
sonar quickstartcommand — a first-run setup wizard that gets new users from zero to a populated inbox in a single command.What it does
Auth check — if no API key is configured (no
SONAR_API_KEYenv var, no~/.sonar/config.json), exits with a friendly message linking to the sign-up page and showing how to set up.Bootstrap — fetches
me+projectsin a single GraphQL call.Existing interests — if the user already has interests set up, skips onboarding and jumps straight to showing their current inbox.
No interests — proposes 3 starter interest drafts tailored to the typical Sonar user (AI/ML, software engineering, tech startups). Shows name, description, and keywords for each.
Interactive confirmation — renders a
[Y/n]prompt via InkuseInput. Enter /yconfirms;n/ Escape aborts cleanly.Create interests — creates each interest sequentially with a live progress display (✓ / spinner / · pending).
Trigger ingest — fires the
indexTweetsmutation.Show inbox — renders the current inbox. If it's empty (indexing hasn't completed yet), shows a friendly message with next steps (
sonar inbox,sonar monitor).Implementation notes
src/commands/quickstart.tsx— no new shared components needed.gqlclient,readConfig/hasTokenhelpers, existing GraphQL mutations/queries (createOrUpdateProject,indexTweets,suggestions).Testing
npm run build # type-checks cleanManual test scenarios:
unset SONAR_API_KEY && sonar quickstartFuture work
editoption in the confirmation prompt to let users tweak suggestions inline before creatingSummary by CodeRabbit
New Features
Documentation