fix: update after sync - #131
Merged
Merged
Conversation
- fixed the weird issue of the local data not being properly updated after a sync with the server, that could cause stale data locally, by handling the two cases (file frozen or not) - fixed some comments issues in `types-schemas`, `update-files-data-frozen-states` and `update-files-data-elapsed-time` - used the `logError` utility function instead of a raw console.error in the `server-dashboard-prod` and `serve-dashboard-dev` functions - in the `get-global-state-data` function, we now schema.safeParse to validate the data instead of schema.parse and handle it directly to avoid using a try/catch block
- only kept the necessary properties in the `todayFilesData` object sent to the api in the `periodicSyncData` function
- renamed the `timeSpentPerProject` to `timeSpentPerProjectToday` for clarity in the `periodicSyncData` function
- bumped the extension version to `v0.0.63`
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 46 minutes and 32 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughVersion bump to 0.0.63 accompanied by logging refactoring (replacing Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (5)
apps/vscode-extension/src/utils/dashboard/serve-dashboard/serve-dashboard-prod.ts (2)
57-57: logError change LGTM; consider including validation issues.Same suggestion as in
serve-dashboard-dev.ts: attachvalidated.error.issuesfor better diagnosability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/vscode-extension/src/utils/dashboard/serve-dashboard/serve-dashboard-prod.ts` at line 57, The logError call in serve-dashboard-prod.ts currently logs "Invalid message shape" without details; update the error logging where logError("Invalid message shape") is called (the message validation branch) to include the validation failure details from validated.error.issues so callers can diagnose issues—e.g., pass or append validated.error.issues to the processLogger/logError invocation used in this file (same pattern as in serve-dashboard-dev.ts), ensuring the log message includes both the descriptive string and the validated.error.issues payload.
61-64: Inconsistency withserve-dashboard-dev.ts: logvalidated.data, not rawdata.The dev variant logs
logDir(validated.data)(line 43), while this file still logslogDir(data)— the raw, unvalidated JSON. Since the rest of the handler usesvalidated.data(line 64), logging the same validated/transformed object keeps both paths consistent and matches what the code actually consumes.♻️ Suggested change
logInfo("Received from dashboard:"); - logDir(data); + logDir(validated.data);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/vscode-extension/src/utils/dashboard/serve-dashboard/serve-dashboard-prod.ts` around lines 61 - 64, The code logs the raw JSON variable data but then uses validated.data downstream; update the logging to reflect the validated/transformed object by replacing the call to logDir(data) with logDir(validated.data) so logInfo/logDir show the same object your handler uses (change the logDir invocation in serve-dashboard-prod.ts just before const { type } = validated.data).apps/vscode-extension/src/utils/files/update-files-data-after-sync.ts (1)
14-22: Minor: drop the redundant trailingreturn.After the early-
returnat line 18, the only remaining path through theif (filesData[filePath])block is the frozen branch which ends at line 21; thereturnon line 22 is unreachable as a control-flow exit (theforEachcallback would return implicitly). Not a bug — just dead code now that the function shape changed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/vscode-extension/src/utils/files/update-files-data-after-sync.ts` around lines 14 - 22, Remove the redundant trailing `return` at the end of the forEach callback in update-files-data-after-sync: inside the block that updates filesData[filePath].elapsedTime, keep the early return when !filesData[filePath].isFrozen (which sets startTime) and then for the frozen branch set filesData[filePath].frozenTime = file.timeSpent and let the function exit implicitly; no change to variable names (filesData, filePath, isFrozen, startTime, frozenTime, elapsedTime) — just delete the unreachable final `return`.apps/vscode-extension/src/utils/dashboard/serve-dashboard/serve-dashboard-dev.ts (1)
37-40: Consider including validation error details.
logError("Invalid message shape")discards the Zod issues which are the most actionable info for debugging malformed messages. Consider attachingvalidated.error.issues(orz.treeifyError(validated.error)) to the log for diagnosability.♻️ Suggested change
- logError("Invalid message shape"); + logError( + `Invalid message shape: ${JSON.stringify(validated.error.issues)}`, + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/vscode-extension/src/utils/dashboard/serve-dashboard/serve-dashboard-dev.ts` around lines 37 - 40, The current early return discards Zod validation details; update the failure branch where validated is checked (the validated variable and logError call in serve-dashboard-dev.ts) to include the validation error details—e.g., pass validated.error.issues or z.treeifyError(validated.error) (or JSON.stringify thereof) into logError so the log contains actionable diagnostics before returning; keep the existing message ("Invalid message shape") and append or include the error payload for clarity.apps/vscode-extension/src/utils/periodic-sync-data.ts (1)
46-57: Optional: collapse.map().reduce()into a singlereduce.The intermediate array allocation isn't needed — you can accumulate directly. Minor readability/perf nit.
♻️ Proposed simplification
- const timeSpentPerProjectToday = Object.entries(filesDataToUpsert) - .map(([, fileData]) => ({ - project: fileData.projectPath, - timeSpent: fileData.elapsedTime, - })) - .reduce( - (acc, curr) => { - acc[curr.project] = (acc[curr.project] || 0) + curr.timeSpent; - return acc; - }, - {} as Record<string, number>, - ); + const timeSpentPerProjectToday = Object.values(filesDataToUpsert).reduce( + (acc, { projectPath, elapsedTime }) => { + acc[projectPath] = (acc[projectPath] ?? 0) + elapsedTime; + return acc; + }, + {} as Record<string, number>, + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/vscode-extension/src/utils/periodic-sync-data.ts` around lines 46 - 57, Collapse the intermediate array allocation by replacing the `.map().reduce()` sequence that builds timeSpentPerProjectToday with a single `.reduce()` over `Object.entries(filesDataToUpsert)`: iterate entries, extract `fileData.projectPath` and `fileData.elapsedTime`, and directly accumulate into the accumulator `Record<string, number>` (the same shape currently used) so you avoid creating the mapped array; update the identifier `timeSpentPerProjectToday` to use this single-pass reduction.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/vscode-extension/src/utils/dashboard/serve-dashboard/serve-dashboard-dev.ts`:
- Around line 37-40: The current early return discards Zod validation details;
update the failure branch where validated is checked (the validated variable and
logError call in serve-dashboard-dev.ts) to include the validation error
details—e.g., pass validated.error.issues or z.treeifyError(validated.error) (or
JSON.stringify thereof) into logError so the log contains actionable diagnostics
before returning; keep the existing message ("Invalid message shape") and append
or include the error payload for clarity.
In
`@apps/vscode-extension/src/utils/dashboard/serve-dashboard/serve-dashboard-prod.ts`:
- Line 57: The logError call in serve-dashboard-prod.ts currently logs "Invalid
message shape" without details; update the error logging where logError("Invalid
message shape") is called (the message validation branch) to include the
validation failure details from validated.error.issues so callers can diagnose
issues—e.g., pass or append validated.error.issues to the processLogger/logError
invocation used in this file (same pattern as in serve-dashboard-dev.ts),
ensuring the log message includes both the descriptive string and the
validated.error.issues payload.
- Around line 61-64: The code logs the raw JSON variable data but then uses
validated.data downstream; update the logging to reflect the
validated/transformed object by replacing the call to logDir(data) with
logDir(validated.data) so logInfo/logDir show the same object your handler uses
(change the logDir invocation in serve-dashboard-prod.ts just before const {
type } = validated.data).
In `@apps/vscode-extension/src/utils/files/update-files-data-after-sync.ts`:
- Around line 14-22: Remove the redundant trailing `return` at the end of the
forEach callback in update-files-data-after-sync: inside the block that updates
filesData[filePath].elapsedTime, keep the early return when
!filesData[filePath].isFrozen (which sets startTime) and then for the frozen
branch set filesData[filePath].frozenTime = file.timeSpent and let the function
exit implicitly; no change to variable names (filesData, filePath, isFrozen,
startTime, frozenTime, elapsedTime) — just delete the unreachable final
`return`.
In `@apps/vscode-extension/src/utils/periodic-sync-data.ts`:
- Around line 46-57: Collapse the intermediate array allocation by replacing the
`.map().reduce()` sequence that builds timeSpentPerProjectToday with a single
`.reduce()` over `Object.entries(filesDataToUpsert)`: iterate entries, extract
`fileData.projectPath` and `fileData.elapsedTime`, and directly accumulate into
the accumulator `Record<string, number>` (the same shape currently used) so you
avoid creating the mapped array; update the identifier
`timeSpentPerProjectToday` to use this single-pass reduction.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 94dcbcf8-61fb-40cf-b67c-23e6badea743
📒 Files selected for processing (10)
apps/vscode-extension/package.jsonapps/vscode-extension/src/types-schemas.tsapps/vscode-extension/src/utils/commands/init-extension-commands.tsapps/vscode-extension/src/utils/dashboard/serve-dashboard/serve-dashboard-dev.tsapps/vscode-extension/src/utils/dashboard/serve-dashboard/serve-dashboard-prod.tsapps/vscode-extension/src/utils/files/update-files-data-after-sync.tsapps/vscode-extension/src/utils/files/update-files-data-elapsed-time.tsapps/vscode-extension/src/utils/files/update-files-data-frozen-states.tsapps/vscode-extension/src/utils/global-state/get-global-state-data.tsapps/vscode-extension/src/utils/periodic-sync-data.ts
- in the `periodicSyncData` simplified the `timeSpentPerProjectToday` calculation - logged the validated data in the `serveDashboardProd` function
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.
Commits
fix: update after sync
types-schemas,update-files-data-frozen-statesandupdate-files-data-elapsed-timelogErrorutility function instead of a raw console.error in theserver-dashboard-prodandserve-dashboard-devfunctionsget-global-state-datafunction, we now schema.safeParse to validate the data instead of schema.parse and handle it directly to avoid using a try/catch blockfix: data payload
todayFilesDataobject sent to the api in theperiodicSyncDatafunctionchore: variable naming
timeSpentPerProjecttotimeSpentPerProjectTodayfor clarity in theperiodicSyncDatafunctionchore: extension version
v0.0.63Summary by CodeRabbit
Chores
Bug Fixes
Documentation