feat(core): add local source adapters for tracer clients - #60
Conversation
There was a problem hiding this comment.
Sorry @makoMakoGo, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
crates/tokscale-core/src/adapters/zed.rs (1)
32-41: ⚖️ Poor tradeoffmacOS 回退路径硬编码可能导致维护负担。
第 36 行将 macOS Zed 数据库路径硬编码为
Library/Application Support/Zed/threads/threads.db。如果 Zed 未来更改目录结构,此处需要手动更新。虽然 PR 目标中提到"保留 Zed 默认数据库回退顺序"表明这是有意为之,但建议考虑:
- 添加注释说明此路径与 Zed 的哪个版本/配置相对应
- 或将回退路径提取为可配置常量便于未来维护
🤖 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 `@crates/tokscale-core/src/adapters/zed.rs` around lines 32 - 41, The hardcoded macOS Zed database path in the format string within the adapter_discover::push_existing_file call lacks context about which Zed version or configuration it corresponds to, which may cause maintenance issues if the path structure changes. Add a comment above the hardcoded path string explaining which Zed version or configuration this fallback path corresponds to, or consider extracting the path pattern into a named constant at the module level to make it easier to maintain and update in the future if Zed's directory structure changes.crates/tokscale-core/src/lib.rs (1)
1758-1769: ⚡ Quick win给报表主解析路径补一个 adapter-only 回归测试。
新增 driver 测试覆盖了
parse_local_clients,但 graph/model/monthly/hourly 报表走的是这里的parse_all_messages_with_pricing_with_env_strategy。建议加一个小测试:创建 Zed fixture 和 OpenCode decoy,调用parse_all_messages_with_pricing(..., &["zed".to_string()], ...),断言只返回 Zed 消息,避免未来这里回退成 legacy full scan 或漏合并 adapter 产物。🤖 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 `@crates/tokscale-core/src/lib.rs` around lines 1758 - 1769, Add a regression test for the adapter-only filtering behavior used by the `parse_all_messages_with_pricing_with_env_strategy` function (which is used by graph/model/monthly/hourly reports). Create a test that sets up a fixture with multiple adapters (Zed as the selected adapter and OpenCode as a decoy), then call `parse_all_messages_with_pricing` with only the Zed adapter specified in the adapter list, and assert that the returned messages contain only Zed-related messages and no OpenCode messages. This test ensures the function correctly filters by the specified adapters and prevents future regressions where the code might fall back to legacy full scan behavior or fail to merge adapter products correctly.
🤖 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 `@crates/tokscale-core/src/adapters/cache.rs`:
- Around line 69-87: When parsing results in empty messages, the current logic
does not invalidate stale cache entries because the invalidate_cache field only
checks the cacheable condition. When messages.is_empty() is true, set
invalidate_cache to true to ensure old cache entries are removed and not left as
stale entries causing repeated fingerprint misses. Update the invalidate_cache
field in the ParsedUnit struct to use a condition that includes
messages.is_empty() alongside the !cacheable check so that empty parse results
trigger cache invalidation just like non-cacheable results do. This issue also
applies at line range 101-105 where similar logic handles invalidate_cache
assignment and requires the same fix.
In `@crates/tokscale-core/src/lib.rs`:
- Around line 4472-4478: The test fixture setup hardcodes a Linux-specific Zed
default directory path ($HOME/.local/share/zed/threads) in the threads_dir,
threads_db, and wal_path setup without platform conditions, but the Zed adapter
branches by OS for the default path discovery, causing test failures on
macOS/Windows. Fix this at all three affected sites (lines 4472-4478, 7208-7213,
and 7289-7294) by either injecting threads_dir through
scanner_settings.extra_scan_paths, generating the directory conditionally based
on target_os, or adding #[cfg(target_os = "linux")] to gate the tests to Linux
only. Choose one approach and apply it consistently across all occurrences.
In `@docs/plans/2026-06-16-c2-adapter-seam.md`:
- Around line 3-4: Fix the Markdown syntax error where `PR` and `#59` are split
across lines at the start of the document. The current line break causes `#59`
to appear at the beginning of a line, which Markdown may interpret as incomplete
heading syntax. Reformat the Status line so that `PR `#59`` stays together on the
same line, preventing rendering issues and maintaining proper Markdown syntax.
---
Nitpick comments:
In `@crates/tokscale-core/src/adapters/zed.rs`:
- Around line 32-41: The hardcoded macOS Zed database path in the format string
within the adapter_discover::push_existing_file call lacks context about which
Zed version or configuration it corresponds to, which may cause maintenance
issues if the path structure changes. Add a comment above the hardcoded path
string explaining which Zed version or configuration this fallback path
corresponds to, or consider extracting the path pattern into a named constant at
the module level to make it easier to maintain and update in the future if Zed's
directory structure changes.
In `@crates/tokscale-core/src/lib.rs`:
- Around line 1758-1769: Add a regression test for the adapter-only filtering
behavior used by the `parse_all_messages_with_pricing_with_env_strategy`
function (which is used by graph/model/monthly/hourly reports). Create a test
that sets up a fixture with multiple adapters (Zed as the selected adapter and
OpenCode as a decoy), then call `parse_all_messages_with_pricing` with only the
Zed adapter specified in the adapter list, and assert that the returned messages
contain only Zed-related messages and no OpenCode messages. This test ensures
the function correctly filters by the specified adapters and prevents future
regressions where the code might fall back to legacy full scan behavior or fail
to merge adapter products correctly.
🪄 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
Run ID: b82b1441-5be5-447e-97bc-692330dac9a2
📒 Files selected for processing (8)
crates/tokscale-core/src/adapters/cache.rscrates/tokscale-core/src/adapters/discover.rscrates/tokscale-core/src/adapters/mod.rscrates/tokscale-core/src/adapters/omp.rscrates/tokscale-core/src/adapters/pi.rscrates/tokscale-core/src/adapters/zed.rscrates/tokscale-core/src/lib.rsdocs/plans/2026-06-16-c2-adapter-seam.md
Refs #36
Summary
Scope Boundary
Validation
Summary by CodeRabbit
发布说明
新功能
改进