Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds v6.1.0: background server/CLI with lifecycle and registry, settings backup/import/export and dashboard customization, packaged-artifact verification and scoped npm publish, many UI/component API updates, new server import endpoints, extensive tests, and documentation/policy updates. Changes
Sequence DiagramssequenceDiagram
participant User
participant CLI
participant Server
participant FS as FileSystem
rect rgba(52, 211, 153, 0.5)
Note over User,Server: Background start and registration
User->>CLI: run `ttdash --background`
CLI->>Server: spawn detached child (no --background)
Server->>FS: create lock, write background-instances.json entry (PID, port, log)
Server-->>CLI: child started (PID, port)
CLI->>User: print running info and `ttdash stop` instruction
end
sequenceDiagram
participant User
participant UI
participant API
participant DB as Storage
rect rgba(59, 130, 246, 0.5)
Note over User,DB: Settings import with conservative merge
User->>UI: Upload settings backup → Import
UI->>API: POST /api/settings/import
API->>DB: read current settings, normalize incoming settings
API->>DB: merge (preserve local when conflicting)
API-->>UI: return merged settings
UI->>User: refresh UI and show import summary
end
sequenceDiagram
participant User
participant UI
participant API
participant DB as Storage
rect rgba(168, 85, 247, 0.5)
Note over User,DB: Usage backup import with conflict accounting
User->>UI: Upload usage backup → Import
UI->>API: POST /api/usage/import
API->>DB: fetch existing daily usage
loop per backup day
alt day missing
API->>DB: insert day (added)
else exact match
API-->>API: mark unchanged
else conflict
API-->>API: mark conflict, skip write
end
end
API-->>UI: return UsageImportSummary (added/unchanged/conflicts/total)
UI->>User: display summary toast
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 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 docstrings
🧪 Generate unit tests (beta)
Comment |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
…tion or class' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
…tion or class' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
src/components/layout/FilterBar.tsx (1)
56-98: Extract preset matching into shared logic to avoid drift.
resolveActivePresetduplicates the same preset date math already used insrc/hooks/use-dashboard-filters.ts. If one changes later, applied filters and highlighted preset can diverge.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/layout/FilterBar.tsx` around lines 56 - 98, resolveActivePreset duplicates date-range calculations from use-dashboard-filters.ts; extract the preset-range logic into a shared helper (e.g., getPresetRange or computePresetRange) that returns { startDate, endDate } for a DashboardDatePreset, import and use that helper inside resolveActivePreset (replace matchesPreset with a comparison against the helper's output) and update use-dashboard-filters.ts to use the same helper so preset math is centralized and cannot drift.src/components/Dashboard.tsx (2)
424-441: Consider wrapping JSON.parse in more specific error handling.When
JSON.parsefails, the error message may not be user-friendly. The current catch block re-throws parse errors to the outer handler, which displayserror.messagedirectly. This works but could show technical JSON syntax errors to users.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Dashboard.tsx` around lines 424 - 441, The JSON.parse call inside handleSettingsImportChange can produce unfriendly SyntaxError messages; update the catch logic to detect parse errors (error instanceof SyntaxError) and show a localized, user-friendly toast (e.g., t('toasts.invalidJsonFile')) instead of error.message, while still handling other errors by showing their message or a generic t('toasts.fileReadFailed'); optionally log the original error for debugging. Make this change in the handleSettingsImportChange function where JSON.parse(await file.text()) is used and where the current catch block calls addToast, so parse failures are handled specifically and other errors fall back to the existing behavior.
483-715: LargerenderSectioncallback has an extensive dependency array.The function and its dependency array are quite large. While this is functionally correct, consider whether some of the dependencies could be stabilized or memoized at a higher level to reduce potential re-renders.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Dashboard.tsx` around lines 483 - 715, The renderSection useCallback has a very large dependency array causing frequent re-creations; stabilize it by memoizing or hoisting volatile inputs (e.g. filteredData, filteredDailyData, metrics, modelCostChartData, modelPieData, costChartData, tokenChartData, requestChartData, providerMetrics, providerLimits, visibleLimitProviders, todayData, allModels, modelCosts, comparisonData, weekdayData) with useMemo/useRef at a higher level or by splitting renderSection into smaller components (e.g. InsightsSection, MetricsSection, CostAnalysisSection) so each component only depends on the data it needs; update renderSection to reference these stabilized/memoized values (or render the new child components directly) and then shrink the useCallback dependency list accordingly.scripts/verify-package.js (1)
66-68: Silent catch on JSON parse may hide issues.The empty
catch {}block silently swallows JSON parse errors during line-by-line fallback parsing. Consider logging a debug message or continuing iteration more explicitly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/verify-package.js` around lines 66 - 68, The empty catch after trying to parse each line (try { return JSON.parse(line); } catch {}) hides JSON parse errors; update the catch to capture the error (e.g., catch (err)) and either log a debug message including the offending line and the error (console.debug or existing logger) before continuing iteration, or explicitly continue to the next line so failures are visible during troubleshooting in the fallback line-by-line parsing logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@install.bat`:
- Around line 89-90: The script uses unquoted file:%cd% in the Bun
global-install command which breaks on paths with spaces and can enable CMD
parsing injection; update both the echo and the call that reference file:%cd%
(the lines printing and running bun add -g file:%cd%) to wrap the value in
quotes so they use "file:%cd%" (i.e., quote the entire file:... argument)
ensuring safe handling of spaces and special characters.
In `@README.md`:
- Line 243: Update the README example command that currently uses an unquoted
path token (bun add -g file:$(pwd)) so it works with repo paths containing
spaces: wrap the file:$(pwd) argument in quotes (i.e., change to bun add -g
"file:$(pwd)") in the code block on the noted line.
In `@scripts/start-test-server.js`:
- Around line 15-19: The server start script sets process.env.PORT to a default
'3015' but ignores Playwright's override; update scripts/start-test-server.js to
map PLAYWRIGHT_TEST_PORT into process.env.PORT (and PLAYWRIGHT_TEST_HOST into
process.env.HOST if present) before the existing defaults so that
process.env.PORT and process.env.HOST reflect Playwright's values when present;
locate the code around process.env.PORT and process.env.HOST and conditionally
assign from process.env.PLAYWRIGHT_TEST_PORT / process.env.PLAYWRIGHT_TEST_HOST
prior to falling back to '3015' / '127.0.0.1'.
In `@server.js`:
- Around line 852-857: The thrown error message when detecting a settings backup
file is in German; update the Error thrown in the block that checks payload.kind
and SETTINGS_BACKUP_KIND (the throw in the same conditional using payload.kind
and SETTINGS_BACKUP_KIND) to use an English message (e.g., "This is a settings
backup file and not a data file.") so messaging is consistent across the
codebase; keep the existing conditional and throw behavior unchanged.
- Around line 833-838: The thrown Error message for the payload.kind check uses
German text; change the message to an English, project-consistent phrasing
(e.g., replace 'Dies ist eine Daten-Backup-Datei und keine Settings-Datei.' in
the throw new Error at the payload.kind === USAGE_BACKUP_KIND check with an
English message like "This is a data backup file and not a settings file.") so
the error thrown when detecting USAGE_BACKUP_KIND matches the rest of the
codebase.
---
Nitpick comments:
In `@scripts/verify-package.js`:
- Around line 66-68: The empty catch after trying to parse each line (try {
return JSON.parse(line); } catch {}) hides JSON parse errors; update the catch
to capture the error (e.g., catch (err)) and either log a debug message
including the offending line and the error (console.debug or existing logger)
before continuing iteration, or explicitly continue to the next line so failures
are visible during troubleshooting in the fallback line-by-line parsing logic.
In `@src/components/Dashboard.tsx`:
- Around line 424-441: The JSON.parse call inside handleSettingsImportChange can
produce unfriendly SyntaxError messages; update the catch logic to detect parse
errors (error instanceof SyntaxError) and show a localized, user-friendly toast
(e.g., t('toasts.invalidJsonFile')) instead of error.message, while still
handling other errors by showing their message or a generic
t('toasts.fileReadFailed'); optionally log the original error for debugging.
Make this change in the handleSettingsImportChange function where
JSON.parse(await file.text()) is used and where the current catch block calls
addToast, so parse failures are handled specifically and other errors fall back
to the existing behavior.
- Around line 483-715: The renderSection useCallback has a very large dependency
array causing frequent re-creations; stabilize it by memoizing or hoisting
volatile inputs (e.g. filteredData, filteredDailyData, metrics,
modelCostChartData, modelPieData, costChartData, tokenChartData,
requestChartData, providerMetrics, providerLimits, visibleLimitProviders,
todayData, allModels, modelCosts, comparisonData, weekdayData) with
useMemo/useRef at a higher level or by splitting renderSection into smaller
components (e.g. InsightsSection, MetricsSection, CostAnalysisSection) so each
component only depends on the data it needs; update renderSection to reference
these stabilized/memoized values (or render the new child components directly)
and then shrink the useCallback dependency list accordingly.
In `@src/components/layout/FilterBar.tsx`:
- Around line 56-98: resolveActivePreset duplicates date-range calculations from
use-dashboard-filters.ts; extract the preset-range logic into a shared helper
(e.g., getPresetRange or computePresetRange) that returns { startDate, endDate }
for a DashboardDatePreset, import and use that helper inside resolveActivePreset
(replace matchesPreset with a comparison against the helper's output) and update
use-dashboard-filters.ts to use the same helper so preset math is centralized
and cannot drift.
🪄 Autofix (Beta)
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
Run ID: 33f3ea03-2060-492e-9883-d0c108f8c009
⛔ Files ignored due to path filters (2)
bun.lockis excluded by!**/*.lockpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (42)
.github/workflows/ci.yml.github/workflows/release.yml.gitignoreAGENTS.mdCHANGELOG.mdCODE_OF_CONDUCT.mdCONTRIBUTING.mdREADME.mdRELEASING.mdSECURITY.mdinstall.batinstall.shpackage.jsonplaywright.config.tsscripts/start-test-server.jsscripts/verify-package.jsserver.jsserver/report/index.jsserver/report/utils.jssrc/components/Dashboard.tsxsrc/components/EmptyState.tsxsrc/components/features/command-palette/CommandPalette.tsxsrc/components/features/insights/UsageInsights.tsxsrc/components/features/limits/LimitsModal.tsxsrc/components/features/settings/SettingsModal.tsxsrc/components/layout/FilterBar.tsxsrc/components/layout/Header.tsxsrc/hooks/use-app-settings.tssrc/hooks/use-dashboard-filters.tssrc/lib/api.tssrc/lib/app-settings.tssrc/lib/calculations.tssrc/lib/dashboard-preferences.tssrc/locales/de/common.jsonsrc/locales/en/common.jsonsrc/types/index.tstests/e2e/dashboard.spec.tstests/frontend/filter-bar.test.tsxtests/frontend/use-dashboard-filters.test.tsxtests/integration/server.test.tstests/unit/analytics.test.tstests/unit/report-utils.test.ts
💤 Files with no reviewable changes (1)
- src/components/features/limits/LimitsModal.tsx
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server.js`:
- Around line 1711-1720: The retry logic in tryListen allows port to increment
past MAX_PORT (65535) causing server.listen to throw before the 'error' handler;
update tryListen/onError so before calling resolve(tryListen(port+1)) you check
port >= MAX_PORT (or port+1 > MAX_PORT) and reject with a clear Error (e.g., "No
free port found (START_PORT-MAX_PORT)") instead of recursing, and ensure
MAX_PORT is set to 65535; reference tryListen, onError, MAX_PORT, START_PORT,
and the server.listen call when applying the guard.
- Around line 209-214: The current return builds fallback chains so setting one
explicitPaths value overrides all three dirs; instead, only assign each output
field when its matching explicitPaths property is present and otherwise leave it
undefined (so platform defaults apply). Update the object returned (dataDir,
configDir, cacheDir) to use explicitPaths.dataDir only for dataDir,
explicitPaths.configDir only for configDir, and explicitPaths.cacheDir only for
cacheDir rather than chaining values across them.
- Around line 820-836: extractSettingsImportPayload currently lets arbitrary
plain objects through which later become defaults via normalizeSettings; change
it to reject any plain object that is not explicitly a settings backup: inside
extractSettingsImportPayload (referencing SETTINGS_BACKUP_KIND and
USAGE_BACKUP_KIND) if payload.kind is not a string or is missing, throw a
descriptive error (e.g. "Uploaded JSON is not a settings backup file"); keep the
existing special-case for USAGE_BACKUP_KIND that throws the usage-specific
message, and only return payload.settings when payload.kind ===
SETTINGS_BACKUP_KIND.
In `@tests/e2e/dashboard.spec.ts`:
- Around line 59-60: The test "manages settings and backup imports through the
settings dialog using isolated test storage" currently only clears /api/usage;
also reset settings to a known baseline by deleting or POSTing a default to
/api/settings before running the UI flow (e.g., call
page.request.delete('/api/settings') or POST default settings), or alternatively
ensure the server for this test uses per-test TTDASH_* dirs so server.js's
SETTINGS_FILE cannot leak prior runs; update the test to perform that reset
(reference the test name and server.js SETTINGS_FILE / TTDASH_* env).
🪄 Autofix (Beta)
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
Run ID: 87290d74-42d6-494d-93c6-01c2f1fcd7d2
📒 Files selected for processing (2)
server.jstests/e2e/dashboard.spec.ts
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
tests/e2e/dashboard.spec.ts (1)
400-422:⚠️ Potential issue | 🟡 MinorReset persisted settings before asserting the locale switch.
This test only clears usage. Earlier scenarios in this file persist
language: 'en', so Line 421 can pass even if the click on Line 416 stops working. Reset/api/settingshere, or assert the initial locale is German before switching.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/dashboard.spec.ts` around lines 400 - 422, The test relies on clearing usage but not persisted settings so the locale may already be 'en'; before simulating the locale switch (the page.getByTitle(...).click() call in this test) clear persisted settings by issuing a request to reset settings (e.g., call page.request.delete('/api/settings') or equivalent) so the click actually changes the locale; keep the existing reportRequest handling and uploadSampleUsage usage unchanged and place the settings reset after clearing usage and before the getByTitle click.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server.js`:
- Around line 821-825: The current SETTINGS_BACKUP_KIND branch only checks for
the presence of the settings key but accepts invalid shapes (e.g., an array)
which can reset user prefs; update the guard to validate payload.settings is a
non-null plain object (typeof payload.settings === 'object', payload.settings
!== null, !Array.isArray(payload.settings)) and optionally require
Object.keys(payload.settings).length > 0, throwing an Error if the validation
fails; apply the same validation change to the other SETTINGS_BACKUP_KIND
handling (the second occurrence that currently mirrors this logic) and ensure
normalizeSettings() only receives a properly shaped settings object.
- Around line 293-304: isProcessRunning currently treats any live PID as a valid
TTDash instance which risks signaling unrelated processes; change
isProcessRunning to perform an instance-specific ownership check (not just
process.kill(pid,0)). For example, after verifying the PID exists, read the
target process identity (on Linux via /proc/<pid>/cmdline or on other platforms
via an equivalent) and compare it to the expected TTDash binary/command or an
instance-specific token (e.g., an exported env var, unique socket path, or
lockfile content) that your registry writes when starting an instance; if the
identity/token does not match, return false. Update all call sites that rely on
PID existence (including the stop/prune logic referenced by isProcessRunning and
the other locations noted in the comment) to rely on the new ownership-aware
isProcessRunning so that stop only sends SIGTERM to verified TTDash instances.
Ensure any platform-specific checks degrade safely (return false) if the
identity cannot be verified.
In `@tests/integration/server.test.ts`:
- Around line 128-137: createCliEnv currently only overrides HOME and XDG_* vars
so Windows runs still resolve real profile paths via APPDATA/LOCALAPPDATA;
update createCliEnv (in tests/integration/server.test.ts) to also set APPDATA
and LOCALAPPDATA (and optionally USERPROFILE) to directories under the provided
root (e.g., path.join(root, 'AppData', 'Roaming') and path.join(root, 'AppData',
'Local')) so server.js resolves profiles into the test root and prevents writes
to the real user profile during Windows test runs.
---
Duplicate comments:
In `@tests/e2e/dashboard.spec.ts`:
- Around line 400-422: The test relies on clearing usage but not persisted
settings so the locale may already be 'en'; before simulating the locale switch
(the page.getByTitle(...).click() call in this test) clear persisted settings by
issuing a request to reset settings (e.g., call
page.request.delete('/api/settings') or equivalent) so the click actually
changes the locale; keep the existing reportRequest handling and
uploadSampleUsage usage unchanged and place the settings reset after clearing
usage and before the getByTitle click.
🪄 Autofix (Beta)
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
Run ID: 12c76071-7160-4044-ab59-75e7e729695a
📒 Files selected for processing (7)
README.mdinstall.batscripts/start-test-server.jsscripts/verify-package.jsserver.jstests/e2e/dashboard.spec.tstests/integration/server.test.ts
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (2)
- scripts/start-test-server.js
- scripts/verify-package.js
This was referenced Apr 12, 2026
Merged
This was referenced Apr 20, 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
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 by CodeRabbit
New Features
Bug Fixes
Documentation
Tests & CI