Revert PRs #60, #65, #73 and clarify project scope#87
Conversation
…n guidelines Added a section outlining the project's philosophy, priorities, and scope, emphasizing the focus on context visibility and stability. Included guidelines for contributions to ensure alignment with the project's roadmap.
Summary of ChangesHello @matt1398, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refines the project's direction by reverting several previously introduced features, specifically the session analysis reports, related export options, and the unified cost calculation engine. The primary motivation is to align the application more closely with its core mission of providing direct visibility into Claude Code's session data, rather than expanding into derivative analysis or complex custom features that add maintenance overhead. Concurrently, new guidelines have been added to the contribution documentation to explicitly define the project's philosophy and scope, promoting stability and direct integration with Claude Code's capabilities. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR removes the session analysis/report feature and associated cost/pricing functionality across code, types, UI, styling, scripts, and tests, and updates components to drop cost-related props and report navigation. Changes
Possibly related PRs
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Comment |
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)
src/renderer/components/layout/MoreMenu.tsx (1)
36-36:⚠️ Potential issue | 🔴 CriticalRemove unused
activeTabIdparameter to fix ESLint error.The pipeline is failing because
activeTabIdis defined but never used. This parameter was likely used by the removedopenSessionReportfunctionality.🐛 Proposed fix
export const MoreMenu = ({ activeTab, activeTabSessionDetail, - activeTabId, }: Readonly<MoreMenuProps>): React.JSX.Element => {Also update the interface:
interface MoreMenuProps { activeTab: Tab | undefined; activeTabSessionDetail: SessionDetail | null; - activeTabId: string | null; }And update any call sites that pass this prop.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/layout/MoreMenu.tsx` at line 36, Remove the unused activeTabId parameter from the MoreMenu component signature and from its props interface (e.g., the Props or MoreMenuProps type) and update any places that pass activeTabId into <MoreMenu /> to stop providing it; ensure the component's export and internal references no longer mention activeTabId and run the TypeScript/ESLint check to confirm the unused-parameter error is resolved.
🧹 Nitpick comments (1)
src/main/utils/jsonl.ts (1)
231-264: Optional: Simplify deadcostUsdcode.Since
costUsdis a constant0and never updated in the loop, the conditioncostUsd > 0is always false. This could be simplified to directly returnundefined.Understand this may be intentional to preserve the structure for future re-enablement—if so, consider adding a brief comment to clarify.
♻️ Suggested simplification
- const costUsd = 0; // ... token accumulation loop ... return { durationMs: maxTime - minTime, totalTokens: inputTokens + cacheCreationTokens + cacheReadTokens + outputTokens, inputTokens, outputTokens, cacheReadTokens, cacheCreationTokens, messageCount: messages.length, - costUsd: costUsd > 0 ? costUsd : undefined, + costUsd: undefined, // Cost calculation removed in revert of PRs `#60`, `#65`, `#73` };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/utils/jsonl.ts` around lines 231 - 264, The local constant costUsd is always 0 and the check costUsd > 0 will never be true, so remove the unused costUsd variable and directly return costUsd: undefined in the result (or if you want to keep the placeholder for future re-enablement, replace the const costUsd = 0 with const costUsd = undefined and add a one-line comment explaining it's intentionally disabled); update the return object in the function that computes duration/tokens (the block that constructs durationMs/totalTokens/inputTokens/...) to reflect this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/renderer/components/layout/MoreMenu.tsx`:
- Line 36: Remove the unused activeTabId parameter from the MoreMenu component
signature and from its props interface (e.g., the Props or MoreMenuProps type)
and update any places that pass activeTabId into <MoreMenu /> to stop providing
it; ensure the component's export and internal references no longer mention
activeTabId and run the TypeScript/ESLint check to confirm the unused-parameter
error is resolved.
---
Nitpick comments:
In `@src/main/utils/jsonl.ts`:
- Around line 231-264: The local constant costUsd is always 0 and the check
costUsd > 0 will never be true, so remove the unused costUsd variable and
directly return costUsd: undefined in the result (or if you want to keep the
placeholder for future re-enablement, replace the const costUsd = 0 with const
costUsd = undefined and add a one-line comment explaining it's intentionally
disabled); update the return object in the function that computes
duration/tokens (the block that constructs
durationMs/totalTokens/inputTokens/...) to reflect this change.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (47)
CONTRIBUTING.mdpackage.jsonresources/pricing.jsonscripts/fetch-pricing-data.tssrc/main/types/chunks.tssrc/main/utils/jsonl.tssrc/renderer/components/chat/AIChatGroup.tsxsrc/renderer/components/chat/ChatHistory.tsxsrc/renderer/components/chat/SessionContextPanel/components/FlatInjectionList.tsxsrc/renderer/components/chat/SessionContextPanel/components/SessionContextHeader.tsxsrc/renderer/components/chat/SessionContextPanel/index.tsxsrc/renderer/components/chat/SessionContextPanel/types.tssrc/renderer/components/chat/items/linkedTool/renderHelpers.tsxsrc/renderer/components/common/TokenUsageDisplay.tsxsrc/renderer/components/layout/MoreMenu.tsxsrc/renderer/components/layout/PaneContent.tsxsrc/renderer/components/layout/SortableTab.tsxsrc/renderer/components/report/AssessmentBadge.tsxsrc/renderer/components/report/ReportSection.tsxsrc/renderer/components/report/SessionReportTab.tsxsrc/renderer/components/report/sections/CostSection.tsxsrc/renderer/components/report/sections/ErrorSection.tsxsrc/renderer/components/report/sections/FrictionSection.tsxsrc/renderer/components/report/sections/GitSection.tsxsrc/renderer/components/report/sections/InsightsSection.tsxsrc/renderer/components/report/sections/KeyTakeawaysSection.tsxsrc/renderer/components/report/sections/OverviewSection.tsxsrc/renderer/components/report/sections/QualitySection.tsxsrc/renderer/components/report/sections/SubagentSection.tsxsrc/renderer/components/report/sections/TimelineSection.tsxsrc/renderer/components/report/sections/TokenSection.tsxsrc/renderer/components/report/sections/ToolSection.tsxsrc/renderer/index.csssrc/renderer/store/slices/tabSlice.tssrc/renderer/types/sessionReport.tssrc/renderer/types/tabs.tssrc/renderer/utils/displayItemBuilder.tssrc/renderer/utils/reportAssessments.tssrc/renderer/utils/sessionAnalyzer.tssrc/shared/types/api.tssrc/shared/utils/costFormatting.tssrc/shared/utils/pricing.tstest/main/utils/costCalculation.test.tstest/renderer/utils/reportAssessments.test.tstest/renderer/utils/sessionAnalyzer.test.tstest/shared/utils/costFormatting.test.tstest/shared/utils/pricing.test.ts
💤 Files with no reviewable changes (38)
- src/renderer/components/chat/SessionContextPanel/index.tsx
- src/renderer/components/common/TokenUsageDisplay.tsx
- src/renderer/utils/sessionAnalyzer.ts
- src/renderer/components/chat/SessionContextPanel/types.ts
- package.json
- src/renderer/components/report/sections/ToolSection.tsx
- src/renderer/components/report/sections/KeyTakeawaysSection.tsx
- src/shared/utils/costFormatting.ts
- src/main/types/chunks.ts
- src/renderer/store/slices/tabSlice.ts
- src/renderer/components/report/AssessmentBadge.tsx
- src/shared/utils/pricing.ts
- src/renderer/index.css
- src/renderer/components/chat/AIChatGroup.tsx
- src/renderer/components/report/sections/TokenSection.tsx
- src/renderer/components/report/sections/CostSection.tsx
- src/shared/types/api.ts
- src/renderer/components/report/sections/SubagentSection.tsx
- src/renderer/components/report/sections/GitSection.tsx
- src/renderer/components/report/SessionReportTab.tsx
- src/renderer/components/report/sections/OverviewSection.tsx
- src/renderer/components/chat/ChatHistory.tsx
- src/renderer/components/chat/SessionContextPanel/components/SessionContextHeader.tsx
- src/renderer/components/report/ReportSection.tsx
- test/main/utils/costCalculation.test.ts
- src/renderer/components/report/sections/InsightsSection.tsx
- src/renderer/components/report/sections/TimelineSection.tsx
- src/renderer/components/report/sections/ErrorSection.tsx
- src/renderer/components/layout/PaneContent.tsx
- test/renderer/utils/reportAssessments.test.ts
- src/renderer/components/report/sections/FrictionSection.tsx
- test/renderer/utils/sessionAnalyzer.test.ts
- test/shared/utils/costFormatting.test.ts
- test/shared/utils/pricing.test.ts
- src/renderer/types/sessionReport.ts
- scripts/fetch-pricing-data.ts
- src/renderer/utils/reportAssessments.ts
- src/renderer/components/report/sections/QualitySection.tsx
The activeTabId prop was removed from the MoreMenu component as it was not being utilized, streamlining the component's interface.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d655dddb83
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| notifications: Bell, | ||
| settings: Settings, | ||
| session: FileText, | ||
| report: Activity, | ||
| } as const; |
There was a problem hiding this comment.
Handle legacy report tabs in tab icon mapping
After this revert, TAB_ICONS no longer includes report, but legacy report tabs can still be restored from context snapshots: validateSnapshot in src/renderer/store/slices/contextSlice.ts currently keeps all non-session tabs (return true), so a snapshot created before the revert can rehydrate a report tab. In that case const Icon = TAB_ICONS[tab.type] becomes undefined, and rendering <Icon /> throws an invalid element type error when opening/switching to that context.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Code Review
This pull request reverts several large features (session analysis, export dropdown, cost calculation) to refocus the project on its core scope. The changes are extensive, involving the removal of many files and code related to these features. The revert appears to be clean and thorough. I've added one minor suggestion to clean up a leftover variable from the cost calculation removal. The new 'Project Philosophy & Scope' section in CONTRIBUTING.md is a great addition and clearly communicates the project's direction to future contributors.
Removed the unused activeTabId prop from the MoreMenu component in the TabBar, simplifying the component's integration and maintaining a cleaner interface.
Summary
Why
These PRs introduced large custom features (session report tab, pricing JSON blob, cost
calculation engine) that fall outside the core purpose of claude-devtools — providing
visibility into Claude Code's actual session data, not generating derivative analysis on
top of it.
Specifically:
cost formatting utilities. This is a significant maintenance surface for functionality
Claude Code itself doesn't expose. It also introduced issue [BUG] After updating performance is very slow #84.
Going forward, we're prioritizing stability and parity with Claude Code over
scope-expanding features. The updated CONTRIBUTING.md now documents this philosophy
explicitly so future contributors have clear guidance before starting work.
What changed
Test plan
pnpm typecheckpassespnpm testpassespnpm buildsucceedsSummary by CodeRabbit
Features Removed
Tests
Chores
Documentation