feat: implement interactive Git Time Machine & resolve cross-platform Windows analysis compatibility#970
feat: implement interactive Git Time Machine & resolve cross-platform Windows analysis compatibility#970Kajol1906 wants to merge 8 commits into
Conversation
|
@Kajol1906 is attempting to deploy a commit to the Nisshchaya's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
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:
📝 WalkthroughWalkthroughAdds a client-side GitTimeMachine D3 visualization with playback controls and integrates it as a new RepositoryAnalysis tab; also updates git branch detection, prisma indexes, README and package.json, dashboard skeleton styling, UI icons, and several small type/logic fixes. ChangesGit Time Machine Feature with Supporting Infrastructure
Sequence DiagramsequenceDiagram
participant User as Developer UI
participant Playback as Playback Controller
participant Engine as Filesystem Engine
participant D3 as D3 Force Simulation
participant SVG as SVG Canvas
User->>Playback: play / pause / scrub / speed
Playback->>Engine: request graph at commit index
Engine->>Playback: return nodes + links
Playback->>D3: update simulation data
D3->>D3: compute node positions (ticks)
D3->>SVG: render nodes/links/halos
SVG->>User: interactive zoom/drag/tooltips
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Suggested Labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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.
Actionable comments posted: 3
🤖 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 `@lib/services/gitService.ts`:
- Around line 190-194: The call in getBranches that runs execPromise(`git
symbolic-ref refs/remotes/origin/HEAD`) can fail when origin/HEAD isn't set;
wrap that execPromise in a try/catch and compute defaultBranchName
deterministically: first attempt the remote HEAD as now, if that throws check
for existence of "main" (e.g., `git rev-parse --verify origin/main`), then
"master" (`origin/master`), and if neither exists fall back to the first branch
returned by your branch listing; use this.repoPath, execPromise and
DEFAULT_GIT_TIMEOUT_MS to run the probes and set defaultBranchName accordingly
without letting a thrown error hard-fail getBranches().
In `@src/components/visualizations/GitTimeMachine.tsx`:
- Around line 532-535: The current cache sync loop in simNodes.forEach skips
valid zero coordinates because it checks `if (n.x && n.y)`; change the condition
in the forEach that writes to coordinateCacheRef.current.set to allow 0 by
checking for presence rather than truthiness (e.g., test for null/undefined or
use Number.isFinite) so that nodes with x=0 or y=0 are cached; adjust the
condition near the simNodes.forEach callback that references n.x and n.y (and
leave the coordinateCacheRef.current.set(n.id, { x: n.x, y: n.y }) line
unchanged).
- Around line 448-469: The tooltip currently injects unescaped repository
strings via tooltip.html(...) using d.name and d.id which creates an XSS risk;
update the rendering in GitTimeMachine.tsx to stop using .html() with raw
template interpolation and instead build the tooltip content safely (e.g.,
create elements via D3/DOM APIs or set textContent/text() for user-derived
values), escaping or sanitizing d.name and d.id (and any other user-derived
values like additionText/deletionText) before insertion; locate the tooltip
usage (the tooltip.style(...).html(`...`) block) and replace it with
constructing the same structure while assigning safe text for d.name and d.id
and only using innerHTML for static, trusted markup if necessary.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b669ed23-6f62-44ab-ab85-acc99d7cfee4
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
README.mdlib/services/gitService.tspackage.jsonsrc/components/visualizations/GitTimeMachine.tsxsrc/pages/RepositoryAnalysis.tsx
| tooltip | ||
| .style("opacity", "1") | ||
| .style("display", "block") | ||
| .style("left", `${event.clientX}px`) | ||
| .style("top", `${event.clientY}px`).html(` | ||
| <div class="space-y-1.5"> | ||
| <div class="flex items-center"> | ||
| <div class="font-semibold text-xs text-white truncate max-w-[200px]">${d.name}</div> | ||
| ${changeMetaHtml} | ||
| </div> | ||
| <div class="text-[10px] text-gray-400 font-mono break-all max-w-[250px]">${d.id}</div> | ||
| <div class="text-[10px] text-gray-500 capitalize font-medium">${d.type === "dir" ? "Directory" : "File"}</div> | ||
| ${ | ||
| d.status && (d.additions > 0 || d.deletions > 0) | ||
| ? `<div class="flex items-center gap-2 text-[10px] font-mono mt-1"> | ||
| ${d.additions > 0 ? `<span class="text-green-500">${additionText}</span>` : ""} | ||
| ${d.deletions > 0 ? `<span class="text-red-500">${deletionText}</span>` : ""} | ||
| </div>` | ||
| : "" | ||
| } | ||
| </div> | ||
| `); |
There was a problem hiding this comment.
Avoid unsanitized .html() injection in tooltip rendering.
d.name and d.id are repository-derived strings; inserting them via .html() creates an XSS path in the browser.
Suggested fix
+ const escapeHtml = (value: string) =>
+ value
+ .replace(/&/g, "&")
+ .replace(/</g, "<")
+ .replace(/>/g, ">")
+ .replace(/"/g, """)
+ .replace(/'/g, "&`#39`;");
...
- tooltip
+ const safeName = escapeHtml(String(d.name ?? ""));
+ const safeId = escapeHtml(String(d.id ?? ""));
+
+ tooltip
.style("opacity", "1")
.style("display", "block")
.style("left", `${event.clientX}px`)
.style("top", `${event.clientY}px`).html(`
<div class="space-y-1.5">
<div class="flex items-center">
- <div class="font-semibold text-xs text-white truncate max-w-[200px]">${d.name}</div>
+ <div class="font-semibold text-xs text-white truncate max-w-[200px]">${safeName}</div>
${changeMetaHtml}
</div>
- <div class="text-[10px] text-gray-400 font-mono break-all max-w-[250px]">${d.id}</div>
+ <div class="text-[10px] text-gray-400 font-mono break-all max-w-[250px]">${safeId}</div>
<div class="text-[10px] text-gray-500 capitalize font-medium">${d.type === "dir" ? "Directory" : "File"}</div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| tooltip | |
| .style("opacity", "1") | |
| .style("display", "block") | |
| .style("left", `${event.clientX}px`) | |
| .style("top", `${event.clientY}px`).html(` | |
| <div class="space-y-1.5"> | |
| <div class="flex items-center"> | |
| <div class="font-semibold text-xs text-white truncate max-w-[200px]">${d.name}</div> | |
| ${changeMetaHtml} | |
| </div> | |
| <div class="text-[10px] text-gray-400 font-mono break-all max-w-[250px]">${d.id}</div> | |
| <div class="text-[10px] text-gray-500 capitalize font-medium">${d.type === "dir" ? "Directory" : "File"}</div> | |
| ${ | |
| d.status && (d.additions > 0 || d.deletions > 0) | |
| ? `<div class="flex items-center gap-2 text-[10px] font-mono mt-1"> | |
| ${d.additions > 0 ? `<span class="text-green-500">${additionText}</span>` : ""} | |
| ${d.deletions > 0 ? `<span class="text-red-500">${deletionText}</span>` : ""} | |
| </div>` | |
| : "" | |
| } | |
| </div> | |
| `); | |
| const escapeHtml = (value: string) => | |
| value | |
| .replace(/&/g, "&") | |
| .replace(/</g, "<") | |
| .replace(/>/g, ">") | |
| .replace(/"/g, """) | |
| .replace(/'/g, "&`#39`;"); | |
| const safeName = escapeHtml(String(d.name ?? "")); | |
| const safeId = escapeHtml(String(d.id ?? "")); | |
| tooltip | |
| .style("opacity", "1") | |
| .style("display", "block") | |
| .style("left", `${event.clientX}px`) | |
| .style("top", `${event.clientY}px`).html(` | |
| <div class="space-y-1.5"> | |
| <div class="flex items-center"> | |
| <div class="font-semibold text-xs text-white truncate max-w-[200px]">${safeName}</div> | |
| ${changeMetaHtml} | |
| </div> | |
| <div class="text-[10px] text-gray-400 font-mono break-all max-w-[250px]">${safeId}</div> | |
| <div class="text-[10px] text-gray-500 capitalize font-medium">${d.type === "dir" ? "Directory" : "File"}</div> | |
| ${ | |
| d.status && (d.additions > 0 || d.deletions > 0) | |
| ? `<div class="flex items-center gap-2 text-[10px] font-mono mt-1"> | |
| ${d.additions > 0 ? `<span class="text-green-500">${additionText}</span>` : ""} | |
| ${d.deletions > 0 ? `<span class="text-red-500">${deletionText}</span>` : ""} | |
| </div>` | |
| : "" | |
| } | |
| </div> | |
| `); |
🤖 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/visualizations/GitTimeMachine.tsx` around lines 448 - 469, The
tooltip currently injects unescaped repository strings via tooltip.html(...)
using d.name and d.id which creates an XSS risk; update the rendering in
GitTimeMachine.tsx to stop using .html() with raw template interpolation and
instead build the tooltip content safely (e.g., create elements via D3/DOM APIs
or set textContent/text() for user-derived values), escaping or sanitizing
d.name and d.id (and any other user-derived values like
additionText/deletionText) before insertion; locate the tooltip usage (the
tooltip.style(...).html(`...`) block) and replace it with constructing the same
structure while assigning safe text for d.name and d.id and only using innerHTML
for static, trusted markup if necessary.
| simNodes.forEach((n: any) => { | ||
| if (n.x && n.y) { | ||
| coordinateCacheRef.current.set(n.id, { x: n.x, y: n.y }); | ||
| } |
There was a problem hiding this comment.
Cache sync should allow zero coordinates.
if (n.x && n.y) skips valid 0 values, so some settled node positions won’t be cached.
Suggested fix
- if (n.x && n.y) {
+ if (n.x != null && n.y != null) {
coordinateCacheRef.current.set(n.id, { x: n.x, y: n.y });
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| simNodes.forEach((n: any) => { | |
| if (n.x && n.y) { | |
| coordinateCacheRef.current.set(n.id, { x: n.x, y: n.y }); | |
| } | |
| simNodes.forEach((n: any) => { | |
| if (n.x != null && n.y != null) { | |
| coordinateCacheRef.current.set(n.id, { x: n.x, y: n.y }); | |
| } |
🤖 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/visualizations/GitTimeMachine.tsx` around lines 532 - 535, The
current cache sync loop in simNodes.forEach skips valid zero coordinates because
it checks `if (n.x && n.y)`; change the condition in the forEach that writes to
coordinateCacheRef.current.set to allow 0 by checking for presence rather than
truthiness (e.g., test for null/undefined or use Number.isFinite) so that nodes
with x=0 or y=0 are cached; adjust the condition near the simNodes.forEach
callback that references n.x and n.y (and leave the
coordinateCacheRef.current.set(n.id, { x: n.x, y: n.y }) line unchanged).
🎉 Thanks for your contribution, @Kajol1906!Your PR has passed our automated GSSoC quality checks. Here's a quick summary:
A maintainer will review your PR soon. Please be patient and available for feedback. 💪 GSSoC'26 automation · Maintainer: @nisshchayarathi |
🎉 Thanks for your contribution, @Kajol1906!Your PR has passed our automated GSSoC quality checks. Here's a quick summary:
A maintainer will review your PR soon. Please be patient and available for feedback. 💪 GSSoC'26 automation · Maintainer: @nisshchayarathi |
🎉 Thanks for your contribution, @Kajol1906!Your PR has passed our automated GSSoC quality checks. Here's a quick summary:
A maintainer will review your PR soon. Please be patient and available for feedback. 💪 GSSoC'26 automation · Maintainer: @nisshchayarathi |
1 similar comment
🎉 Thanks for your contribution, @Kajol1906!Your PR has passed our automated GSSoC quality checks. Here's a quick summary:
A maintainer will review your PR soon. Please be patient and available for feedback. 💪 GSSoC'26 automation · Maintainer: @nisshchayarathi |
|
Hi @nisshchayarathi , just wanted to gently ping you on this PR whenever you have a moment to review it! Since it's been a few days. |
…rm Windows compatibility
…ck, zero-coord cache
Kajol1906
force-pushed
the
git-time-machine
branch
from
f381422 to
c77f48c
Compare
🎉 Thanks for your contribution, @Kajol1906!Your PR has passed our automated GSSoC quality checks. Here's a quick summary:
A maintainer will review your PR soon. Please be patient and available for feedback. 💪 GSSoC'26 automation · Maintainer: @nisshchayarathi |
1 similar comment
🎉 Thanks for your contribution, @Kajol1906!Your PR has passed our automated GSSoC quality checks. Here's a quick summary:
A maintainer will review your PR soon. Please be patient and available for feedback. 💪 GSSoC'26 automation · Maintainer: @nisshchayarathi |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@README.md`:
- Around line 260-275: The README has duplicated Vercel deployment instructions
(the "🚀 Deployment Steps" block and a second near the later checklist) and
duplicated Environment Variables/checklist content; consolidate by keeping a
single canonical "🚀 Deployment Steps" section and a single "Environment
Variables" checklist, remove the duplicate blocks (the second steps list and the
repeated checklist), update any cross-references to point to the retained
sections, and add a short note/link in the removed locations that points readers
to the canonical "🚀 Deployment Steps" and "Environment Variables" headings so
references remain valid.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2852d8ae-6062-476b-8b72-64c22e1161d8
📒 Files selected for processing (2)
README.mdlib/services/gitService.ts
💤 Files with no reviewable changes (1)
- lib/services/gitService.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🤖 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 `@README.md`:
- Around line 260-275: The README has duplicated Vercel deployment instructions
(the "🚀 Deployment Steps" block and a second near the later checklist) and
duplicated Environment Variables/checklist content; consolidate by keeping a
single canonical "🚀 Deployment Steps" section and a single "Environment
Variables" checklist, remove the duplicate blocks (the second steps list and the
repeated checklist), update any cross-references to point to the retained
sections, and add a short note/link in the removed locations that points readers
to the canonical "🚀 Deployment Steps" and "Environment Variables" headings so
references remain valid.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2852d8ae-6062-476b-8b72-64c22e1161d8
📒 Files selected for processing (2)
README.mdlib/services/gitService.ts
💤 Files with no reviewable changes (1)
- lib/services/gitService.ts
🛑 Comments failed to post (1)
README.md (1)
260-275:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winConsolidate duplicated Vercel deployment guidance into one canonical section.
Lines 260–275 and Lines 301–304 repeat deployment steps, and Lines 245–259 vs. Lines 306–334 duplicate checklist content with overlapping scope. This duplication will drift over time and can confuse setup. Keep one checklist + one steps block, then cross-link to it.
Also applies to: 301-304, 306-334
🤖 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 `@README.md` around lines 260 - 275, The README has duplicated Vercel deployment instructions (the "🚀 Deployment Steps" block and a second near the later checklist) and duplicated Environment Variables/checklist content; consolidate by keeping a single canonical "🚀 Deployment Steps" section and a single "Environment Variables" checklist, remove the duplicate blocks (the second steps list and the repeated checklist), update any cross-references to point to the retained sections, and add a short note/link in the removed locations that points readers to the canonical "🚀 Deployment Steps" and "Environment Variables" headings so references remain valid.
🎉 Thanks for your contribution, @Kajol1906!Your PR has passed our automated GSSoC quality checks. Here's a quick summary:
A maintainer will review your PR soon. Please be patient and available for feedback. 💪 GSSoC'26 automation · Maintainer: @nisshchayarathi |
1 similar comment
🎉 Thanks for your contribution, @Kajol1906!Your PR has passed our automated GSSoC quality checks. Here's a quick summary:
A maintainer will review your PR soon. Please be patient and available for feedback. 💪 GSSoC'26 automation · Maintainer: @nisshchayarathi |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/analysisWorker.ts (1)
144-157: ⚖️ Poor tradeoffShutdown handler does not wait for in-flight jobs to complete.
When
SIGTERM/SIGINTis received,process.exit(0)is called immediately after disconnecting Prisma, even ifrunJob()is mid-execution. This abruptly terminates in-progress work and relies solely on the lock expiration (5 minutes) for recovery.Consider deferring
process.exit(0)until the current job finishes or a grace-period timeout elapses.♻️ Sketch of graceful shutdown
let stopping = false; +let currentJobPromise: Promise<void> | null = null; const shutdown = async (signal: string) => { if (stopping) return; stopping = true; console.log(`received ${signal}, shutting down...`); + + // Wait for current job to finish (with timeout) + if (currentJobPromise) { + const timeout = new Promise((r) => setTimeout(r, 30_000)); + await Promise.race([currentJobPromise, timeout]); + } + try { await prisma.$disconnect(); } catch { // ignore } process.exit(0); };Then wrap the
runJobcall:currentJobPromise = runJob(job, { workerId, lockMs, heartbeatIntervalMs }); await currentJobPromise; currentJobPromise = null;🤖 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 `@scripts/analysisWorker.ts` around lines 144 - 157, The shutdown handler currently disconnects Prisma and calls process.exit(0) immediately, which can kill an in-flight runJob; add a mechanism to wait for the current job to finish (or a bounded grace-period) before exiting by tracking the promise for the active job (e.g., set a module-scoped currentJobPromise when invoking runJob in the worker loop) and modifying shutdown(signal: string) to await Promise.race([currentJobPromise ?? Promise.resolve(), timeoutPromise(graceMs)]) before calling prisma.$disconnect() and process.exit(0); ensure shutdown still short-circuits when stopping is true and that currentJobPromise is cleared (set to null) after runJob completes so subsequent shutdowns don’t wait unnecessarily.
🤖 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.
Nitpick comments:
In `@scripts/analysisWorker.ts`:
- Around line 144-157: The shutdown handler currently disconnects Prisma and
calls process.exit(0) immediately, which can kill an in-flight runJob; add a
mechanism to wait for the current job to finish (or a bounded grace-period)
before exiting by tracking the promise for the active job (e.g., set a
module-scoped currentJobPromise when invoking runJob in the worker loop) and
modifying shutdown(signal: string) to await Promise.race([currentJobPromise ??
Promise.resolve(), timeoutPromise(graceMs)]) before calling prisma.$disconnect()
and process.exit(0); ensure shutdown still short-circuits when stopping is true
and that currentJobPromise is cleared (set to null) after runJob completes so
subsequent shutdowns don’t wait unnecessarily.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 58678602-b1ff-44e0-b966-31309d687916
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
app/layout.tsxlib/services/repositoryService.tsprisma/schema.prismascripts/analysisWorker.tssrc/pages/Dashboard.tsxsrc/pages/RepositoryAnalysis.tsxsrc/pages/SearchPage.tsx
💤 Files with no reviewable changes (4)
- src/pages/Dashboard.tsx
- prisma/schema.prisma
- src/pages/SearchPage.tsx
- app/layout.tsx
✅ Files skipped from review due to trivial changes (1)
- lib/services/repositoryService.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/RepositoryAnalysis.tsx
🎉 Thanks for your contribution, @Kajol1906!Your PR has passed our automated GSSoC quality checks. Here's a quick summary:
A maintainer will review your PR soon. Please be patient and available for feedback. 💪 GSSoC'26 automation · Maintainer: @nisshchayarathi |
1 similar comment
🎉 Thanks for your contribution, @Kajol1906!Your PR has passed our automated GSSoC quality checks. Here's a quick summary:
A maintainer will review your PR soon. Please be patient and available for feedback. 💪 GSSoC'26 automation · Maintainer: @nisshchayarathi |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
53-53:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix
cross-envto be treated as a runtime dependency (lockfile currently marks it dev-only)
cross-envis declared in bothdependenciesanddevDependencies, butpackage-lock.jsonrecordspackages["node_modules/cross-env"].dev = true. Since runtime scripts (start,worker,worker:server, etc.) invokecross-env,npm ci --omit=devcan omit it and break startup.Suggested fix
"devDependencies": { @@ - "cross-env": "^10.1.0",Regenerate
package-lock.jsonand ensure thenode_modules/cross-enventry is no longer marked"dev": true.🤖 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 `@package.json` at line 53, package-lock.json marks node_modules/cross-env as dev-only while runtime scripts (start, worker, worker:server) use cross-env; remove the duplicate cross-env entry from devDependencies in package.json so it only appears in dependencies, then regenerate the lockfile (run npm install or npm ci appropriately) so package-lock.json records node_modules/cross-env with "dev": false; verify the lockfile's packages["node_modules/cross-env"].dev is no longer true and commit the updated package.json and package-lock.json.
🤖 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.
Outside diff comments:
In `@package.json`:
- Line 53: package-lock.json marks node_modules/cross-env as dev-only while
runtime scripts (start, worker, worker:server) use cross-env; remove the
duplicate cross-env entry from devDependencies in package.json so it only
appears in dependencies, then regenerate the lockfile (run npm install or npm ci
appropriately) so package-lock.json records node_modules/cross-env with "dev":
false; verify the lockfile's packages["node_modules/cross-env"].dev is no longer
true and commit the updated package.json and package-lock.json.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 01880c05-5f3d-4a18-9a7e-84007f512100
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
package.jsonprisma/schema.prismasrc/pages/Dashboard.tsxsrc/pages/RepositoryAnalysis.tsx
✅ Files skipped from review due to trivial changes (1)
- src/pages/Dashboard.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/pages/RepositoryAnalysis.tsx
- prisma/schema.prisma
…y on git-time-machine branch
🎉 Thanks for your contribution, @Kajol1906!Your PR has passed our automated GSSoC quality checks. Here's a quick summary:
A maintainer will review your PR soon. Please be patient and available for feedback. 💪 GSSoC'26 automation · Maintainer: @nisshchayarathi |
1 similar comment
🎉 Thanks for your contribution, @Kajol1906!Your PR has passed our automated GSSoC quality checks. Here's a quick summary:
A maintainer will review your PR soon. Please be patient and available for feedback. 💪 GSSoC'26 automation · Maintainer: @nisshchayarathi |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/visualizations/GitTimeMachine.tsx (1)
103-103:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReconcile the selected node after each graph rebuild.
The inspector keeps the D3 node object captured on click, but
graphDatarecreates nodes on every commit change. After scrubbing, the side panel can show a file that is no longer in the current graph, or stale data for one that still exists. Re-select byidfromgraphData.nodes, or clear the selection when that node disappears.Possible fix
+ useEffect(() => { + setSelectedNode((prev) => { + if (!prev) return null; + return graphData.nodes.find((node) => node.id === prev.id) ?? null; + }); + }, [graphData.nodes]);Also applies to: 513-515, 766-779
🤖 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/visualizations/GitTimeMachine.tsx` at line 103, selectedNode (and setSelectedNode) is holding a stale D3 node object across graph rebuilds because graphData recreates nodes; add a reconciliation step that runs when graphData (or the commits/graph rebuild trigger) changes: look up the currently selectedNode.id in graphData.nodes and call setSelectedNode(foundNode || null) so the inspector always references the current node instance (or clears selection if the id no longer exists). Implement this with a useEffect that depends on graphData (or the same deps that rebuild the graph) to re-select by id or clear, and apply the same fix to the other selection-handling sites noted around the blocks referenced (lines ~513-515 and ~766-779) where selection is stored/reused.
🧹 Nitpick comments (1)
src/components/visualizations/GitTimeMachine.tsx (1)
108-251: 🏗️ Heavy liftAvoid replaying the entire history on every tick.
graphDatarebuilds the tree from commit0throughactiveCommitIndexon every scrub/playback step, then prunes folders again. That makes autoplay quadratic in commit count and will start blocking the main thread on larger histories. Cache per-commit snapshots/deltas, or incrementally apply just the next/previous commit when the index moves.🤖 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/visualizations/GitTimeMachine.tsx` around lines 108 - 251, graphData currently reconstructs the full tree from commit 0 to activeCommitIndex on every render (using sortedCommits, activeCommitIndex, nodesMap, linksSet and coordinateCacheRef), causing quadratic time during playback; instead implement incremental snapshot/delta application: maintain a commitSnapshotsRef (or a Map keyed by commit index) that stores the nodes/links state for each committed index and functions applyCommitDelta(nextIndex) and rollbackCommitDelta(prevIndex) which apply only that commit's fileChanges to a cached nodesMap/linksSet and update coordinateCacheRef as needed; change the useMemo for graphData to read from commitSnapshotsRef[activeCommitIndex] (or apply one incremental step from the last cached index) rather than replaying from 0, and ensure you update the cache when sortedCommits changes (invalidate/update snapshots) and when activeCommitIndex moves forward/backward during scrub/playback.
🤖 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/visualizations/GitTimeMachine.tsx`:
- Around line 707-713: The DiceBear seed is built directly from
activeCommit.authorName which can contain characters like &, #, ?, or / and
break the URL; update the Image src construction in the GitTimeMachine component
to URL-encode the seed (use encodeURIComponent(activeCommit.authorName)) when
building the DiceBear URL while keeping alt and other props unchanged so the
avatar request reliably represents a single seed value.
In `@src/pages/compare.tsx`:
- Around line 87-93: The effect currently calls fetchRepositories() even while
auth is unresolved; update the useEffect so it first returns early while
isAuthLoading is true, then if not isAuthenticated do router.push("/login") and
return, and only call fetchRepositories() when isAuthLoading is false and
isAuthenticated is true; adjust the conditional flow in the useEffect containing
isAuthLoading, isAuthenticated, router.push, and fetchRepositories accordingly.
In `@src/pages/simulate-pr.tsx`:
- Around line 74-80: The effect currently calls fetchRepositories() even while
isAuthLoading is true; update the useEffect so fetchRepositories is only invoked
when auth loading has finished and the user is authenticated (e.g., inside the
branch where !isAuthLoading && isAuthenticated) and keep the existing redirect
(router.push("/login")) when !isAuthLoading && !isAuthenticated; specifically
modify the useEffect that references isAuthLoading, isAuthenticated, router, and
fetchRepositories so it guards fetchRepositories behind the auth-ready
condition.
---
Outside diff comments:
In `@src/components/visualizations/GitTimeMachine.tsx`:
- Line 103: selectedNode (and setSelectedNode) is holding a stale D3 node object
across graph rebuilds because graphData recreates nodes; add a reconciliation
step that runs when graphData (or the commits/graph rebuild trigger) changes:
look up the currently selectedNode.id in graphData.nodes and call
setSelectedNode(foundNode || null) so the inspector always references the
current node instance (or clears selection if the id no longer exists).
Implement this with a useEffect that depends on graphData (or the same deps that
rebuild the graph) to re-select by id or clear, and apply the same fix to the
other selection-handling sites noted around the blocks referenced (lines
~513-515 and ~766-779) where selection is stored/reused.
---
Nitpick comments:
In `@src/components/visualizations/GitTimeMachine.tsx`:
- Around line 108-251: graphData currently reconstructs the full tree from
commit 0 to activeCommitIndex on every render (using sortedCommits,
activeCommitIndex, nodesMap, linksSet and coordinateCacheRef), causing quadratic
time during playback; instead implement incremental snapshot/delta application:
maintain a commitSnapshotsRef (or a Map keyed by commit index) that stores the
nodes/links state for each committed index and functions
applyCommitDelta(nextIndex) and rollbackCommitDelta(prevIndex) which apply only
that commit's fileChanges to a cached nodesMap/linksSet and update
coordinateCacheRef as needed; change the useMemo for graphData to read from
commitSnapshotsRef[activeCommitIndex] (or apply one incremental step from the
last cached index) rather than replaying from 0, and ensure you update the cache
when sortedCommits changes (invalidate/update snapshots) and when
activeCommitIndex moves forward/backward during scrub/playback.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 52878156-443c-4f27-87a9-2a1ca2888c32
📒 Files selected for processing (14)
next.config.jssrc/components/repository/ContributionPathGenerator.tsxsrc/components/repository/FileStructure.tsxsrc/components/repository/FirstPRSimulator.tsxsrc/components/repository/GoodFirstIssueGenerator.tsxsrc/components/ui/Badge.tsxsrc/components/ui/index.tssrc/components/visualizations/CodeDependencyGraph.tsxsrc/components/visualizations/GitTimeMachine.tsxsrc/lib/changeImpact.tssrc/pages/compare.tsxsrc/pages/simulate-pr.tsxsrc/services/architectureDriftService.tssrc/utils/pathGenerator.ts
💤 Files with no reviewable changes (1)
- src/components/repository/FileStructure.tsx
✅ Files skipped from review due to trivial changes (7)
- src/components/ui/index.ts
- src/services/architectureDriftService.ts
- src/components/repository/FirstPRSimulator.tsx
- src/components/repository/ContributionPathGenerator.tsx
- src/components/repository/GoodFirstIssueGenerator.tsx
- next.config.js
- src/components/visualizations/CodeDependencyGraph.tsx
| <Image | ||
| src={`https://api.dicebear.com/7.x/avataaars/svg?seed=${activeCommit.authorName}`} | ||
| alt={activeCommit.authorName} | ||
| width={36} | ||
| height={36} | ||
| className="h-9 w-9 rounded-full bg-white/10 p-0.5 border border-white/20 shadow-md glow-author" | ||
| /> |
There was a problem hiding this comment.
Encode the DiceBear seed before building the image URL.
Author names are repo-derived. If a name contains &, #, ?, or /, this URL stops representing a single seed value and the HUD can request the wrong avatar.
Possible fix
- src={`https://api.dicebear.com/7.x/avataaars/svg?seed=${activeCommit.authorName}`}
+ src={`https://api.dicebear.com/7.x/avataaars/svg?seed=${encodeURIComponent(activeCommit.authorName)}`}🤖 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/visualizations/GitTimeMachine.tsx` around lines 707 - 713, The
DiceBear seed is built directly from activeCommit.authorName which can contain
characters like &, #, ?, or / and break the URL; update the Image src
construction in the GitTimeMachine component to URL-encode the seed (use
encodeURIComponent(activeCommit.authorName)) when building the DiceBear URL
while keeping alt and other props unchanged so the avatar request reliably
represents a single seed value.
| useEffect(() => { | ||
| if (!isAuthLoading && !isAuthenticated) { | ||
| router.push("/login"); | ||
| return; | ||
| } | ||
| fetchRepositories(); | ||
| }, [isAuthLoading, isAuthenticated, router, fetchRepositories]); |
There was a problem hiding this comment.
Avoid fetching repositories during unresolved auth state.
At Line 92, the effect calls fetchRepositories() before auth loading is complete, which can create unnecessary failed calls and transient UI noise for signed-out sessions.
Suggested fix
useEffect(() => {
- if (!isAuthLoading && !isAuthenticated) {
+ if (isAuthLoading) return;
+ if (!isAuthenticated) {
router.push("/login");
return;
}
fetchRepositories();
}, [isAuthLoading, isAuthenticated, router, fetchRepositories]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| if (!isAuthLoading && !isAuthenticated) { | |
| router.push("/login"); | |
| return; | |
| } | |
| fetchRepositories(); | |
| }, [isAuthLoading, isAuthenticated, router, fetchRepositories]); | |
| useEffect(() => { | |
| if (isAuthLoading) return; | |
| if (!isAuthenticated) { | |
| router.push("/login"); | |
| return; | |
| } | |
| fetchRepositories(); | |
| }, [isAuthLoading, isAuthenticated, router, fetchRepositories]); |
🤖 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/pages/compare.tsx` around lines 87 - 93, The effect currently calls
fetchRepositories() even while auth is unresolved; update the useEffect so it
first returns early while isAuthLoading is true, then if not isAuthenticated do
router.push("/login") and return, and only call fetchRepositories() when
isAuthLoading is false and isAuthenticated is true; adjust the conditional flow
in the useEffect containing isAuthLoading, isAuthenticated, router.push, and
fetchRepositories accordingly.
…i#970 Resolved conflicts in 7 files: - README.md: Use upstream's detailed scripts table format - next.config.js: Include upstream's SVG safety configuration - package-lock.json: Include dependencies from both branches - FileStructure.tsx: Remove duplicate Change Summary section - Badge.tsx: Keep cva-based Badge component (more feature-rich) - changeImpact.ts: Use nullish coalescing (??) operator - RepositoryAnalysis.tsx: Keep timemachine tab type for Git Time Machine feature
🎉 Thanks for your contribution, @Kajol1906!Your PR has passed our automated GSSoC quality checks. Here's a quick summary:
A maintainer will review your PR soon. Please be patient and available for feedback. 💪 GSSoC'26 automation · Maintainer: @nisshchayarathi |
|
Hi @nisshchayarathi , just wanted to gently ping you on this PR whenever you have a moment to review it! Since it's been a few days. |
1 participant
Description
This pull request implements the Git Time Machine & Repo Evolution Player dashboard feature. This is a 100% frontend and client-side visualization system that lets developers play back repository commit history sequentially and observe file-tree structures evolve in real-time.
Additionally, this PR resolves key cross-platform Windows compatibility bugs in the repository parser, ensuring that local repository analyses execute smoothly under PowerShell and CMD.
Key Changes
GitTimeMachine.tsx):coordinateCacheRef) to prevent visual layout explosions on playback ticks.0.5x,1x,2x,5x), and a timeline range scrubber.RepositoryAnalysis.tsx):Activityicon.gitService.ts):sedwith native JavaScript regex replacements to get default branches on Windows.--format="..."ingit for-each-refto prevent CMD/PowerShell percent symbol parser errors.README.mdto document the new Time Machine capabilities and the background worker script (npm run worker:dev).Related Issue
Closes #444
Type of Change
Screenshots
recorded-video
https://github.com/user-attachments/assets/ef86c47e-da54-4665-95dd-94efaaeada57
image
Testing
Describe the commands you ran and any manual verification performed.
npx tsc -p tsconfig.json --noEmitgit diff --checkSpoon-Knifeandexpressjs/session) and verified real-time HUD and D3 graph updates across playback ticks in the local browser athttp://localhost:3000.N/Awith a reasonChecklist
Summary by CodeRabbit
New Features
Documentation
Style
Chores