docs(claude): improve CLAUDE.md with architecture details#150
Conversation
Add service interfaces documentation, daemon architecture section, CCOtel feature documentation, and streamline existing content for better Claude Code guidance. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @AnnatarHe, 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 enhances the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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
|
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly improves the CLAUDE.md documentation by restructuring the architecture section, adding details on service interfaces, and documenting new features like CCOtel. The changes make the documentation more streamlined and informative. My review focuses on ensuring the accuracy of this new information. I've found a few inconsistencies between the documentation and the codebase, particularly regarding default port numbers and dependency injection descriptions. I've also included a suggestion to improve the clarity of a build command example. Addressing these points will make the documentation even more reliable for developers.
| - Main config: `$HOME/.shelltime/config.toml` | ||
| - Local overrides: `$HOME/.shelltime/config.local.toml` (merged, gitignored) | ||
| - Daemon socket: `/tmp/shelltime.sock` (configurable via `socketPath`) | ||
| - CCOtel gRPC port: configurable via `ccotel.grpcPort` (default: 4317) |
There was a problem hiding this comment.
There is an inconsistency in the default gRPC port for CCOtel. This documentation states the default is 4317, which is the standard for OTLP gRPC. However, the code consistently uses 54027 as the default (see model/config.go:139 and model/ccotel_env.go). Please update the documentation to reflect the correct default port used in the code.
| - CCOtel gRPC port: configurable via `ccotel.grpcPort` (default: 4317) | |
| - CCOtel gRPC port: configurable via `ccotel.grpcPort` (default: 54027) |
| - Daemon is optional but recommended (<8ms latency vs ~100ms+ direct) | ||
| - Encryption requires daemon mode and a token with encryption capability | ||
| - Shell hooks are platform-specific (bash, zsh, fish) - test on target shells | ||
| - CCOtel feature enables Claude Code metrics/logs passthrough via gRPC (port 4317) No newline at end of file |
There was a problem hiding this comment.
The CCOtel gRPC port mentioned here is inconsistent with the codebase. The documentation states port 4317, but the code's default is 54027. To avoid confusion, please update this line to match the actual default port.
| - CCOtel feature enables Claude Code metrics/logs passthrough via gRPC (port 4317) | |
| - CCOtel feature enables Claude Code metrics/logs passthrough via gRPC (port 54027) |
| # Build with version information | ||
| go build -ldflags "-X main.version=v0.1.0 -X main.commit=$(git rev-parse HEAD) -X main.date=$(date -u +%Y-%m-%d)" -o shelltime ./cmd/cli/main.go | ||
| # Build with all ldflags (version, commit, AI service config) | ||
| go build -ldflags "-X main.version=v0.1.0 -X main.commit=$(git rev-parse HEAD) -X main.date=$(date -u +%Y-%m-%d) -X main.ppEndpoint=<endpoint> -X main.ppToken=<token>" -o shelltime ./cmd/cli/main.go |
There was a problem hiding this comment.
For better clarity and to avoid ambiguity with other potential tokens or endpoints, it would be beneficial to use more specific placeholder names for the AI service configuration. Since the documentation later identifies the service as PromptPal, using names that reflect this would be more explicit.
| go build -ldflags "-X main.version=v0.1.0 -X main.commit=$(git rev-parse HEAD) -X main.date=$(date -u +%Y-%m-%d) -X main.ppEndpoint=<endpoint> -X main.ppToken=<token>" -o shelltime ./cmd/cli/main.go | |
| go build -ldflags "-X main.version=v0.1.0 -X main.commit=$(git rev-parse HEAD) -X main.date=$(date -u +%Y-%m-%d) -X main.ppEndpoint=<promptpal_endpoint> -X main.ppToken=<promptpal_token>" -o shelltime ./cmd/cli/main.go |
| - `AIService`: PromptPal integration for AI-powered command suggestions (`shelltime q`) | ||
| - `CommandService`: Executable lookup with fallback paths (handles daemon's limited PATH) | ||
|
|
||
| Injection happens in `cmd/*/main.go` via `commands.InjectVar()` and `commands.InjectAIService()`. |
There was a problem hiding this comment.
This statement about dependency injection is slightly inaccurate. The injection of AIService only happens in cmd/cli/main.go, and cmd/daemon/main.go uses model.InjectVar instead of commands.InjectVar. To improve accuracy, it would be better to specify that this particular injection mechanism applies to the CLI.
| Injection happens in `cmd/*/main.go` via `commands.InjectVar()` and `commands.InjectAIService()`. | |
| Injection for the CLI happens in `cmd/cli/main.go` via `commands.InjectVar()` and `commands.InjectAIService()`. |
| ### Daemon Architecture | ||
| 1. **SocketHandler**: Unix domain socket server accepting JSON messages from CLI | ||
| 2. **GoChannel**: Watermill pub/sub for decoupled message processing | ||
| 3. **SocketTopicProcessor**: Consumes messages and routes to appropriate handlers |
There was a problem hiding this comment.
There's a minor inconsistency between this documentation and the code. The name here is SocketTopicProcessor, but the function call in cmd/daemon/main.go:82 is daemon.SocketTopicProccessor (with a typo, an extra 'c'). While the spelling in this document is correct, it would be best to fix the typo in the Go code to ensure consistency across the project.
1 participant
Summary
Test plan
🤖 Generated with Claude Code