feat(sight): support ~/.agentsight trajectory collection via scan_dirs - #2740
feat(sight): support ~/.agentsight trajectory collection via scan_dirs#2740chengshuyi wants to merge 1 commit into
Conversation
github-actions
Bot
added
component:sight
![qoderai[bot]](https://avatars.githubusercontent.com/in/1396158?s=60&v=4){kind=small}
There was a problem hiding this comment.
[P2] 主要风险：
- 文档与实现的 scan_dirs 去重语义略有偏差，可能造成运维误解；
.agentsight模糊匹配会将非 skillrubric 路径误标为 agentsight source；- dev.sh 派生配置时重写 trajectory_collection 字段，潜在丢弃已有配置键。
🤖 Generated by Qoder • View workflow run
| | 审计 | `features.audit` | `true` | LLM 调用审计持久化 | | ||
| | Token 消费 | `features.token_consumption` | `false` | 聚合消费记录 | | ||
| | SLS Logtail | `features.sls_logtail` | `false` | SLS 日志文件导出 | |
There was a problem hiding this comment.
[P2] scan_dirs 去重逻辑未同步文档
CollectorConfig::scan_dirs 已改为“追加并去重”，但 AGENTS.md 中仅提到“追加目录、与默认路径合并去重”，未明确说明基于 canonical path 去重，可能让运维误认为多个指向同一物理目录的路径仍会重复扫描。建议在文档中补充“按规范路径去重，重复目录不会多次扫描”的说明。
🤖 Generated by Qoder • Fix in Qoder
| source_from_path(Path::new("/home/u/.qoder/projects")), | ||
| "qoder" | ||
| ); | ||
| assert_eq!( | ||
| source_from_path(Path::new("/root/.agentsight/skillrubric/traces")), |
There was a problem hiding this comment.
[P2] source_from_path 匹配 .agentsight 目录含糊
source_from_path 只要路径包含 .agentsight 就返回 "agentsight"，如果未来存在其他以 .agentsight 命名的目录（非 skillrubric traces），这些会话也会被误标为 agentsight source。建议改为更精准的前缀或完整路径匹配（如限定 ~/.agentsight/skillrubric/traces 或其下 projects），降低误标风险。
🤖 Generated by Qoder • Fix in Qoder
| # 追加 ~/.agentsight/skillrubric/traces 到 scan_dirs（去重） | ||
| home = os.path.expanduser("~") | ||
| extra_dirs = [os.path.join(home, ".agentsight", "skillrubric", "traces")] | ||
| existing = cfg.get("features", {}).get("trajectory_collection", {}).get("scan_dirs", []) | ||
| for d in extra_dirs: | ||
| if d not in existing and os.path.isdir(d): | ||
| existing.append(d) | ||
|
|
||
| cfg.setdefault("features", {})["trajectory_collection"] = { |
There was a problem hiding this comment.
[P2] dev.sh 中 scan_dirs 构造忽略已有配置合并语义
dev.sh 中构造 trajectory_collection 时直接覆盖该字段为新的 dict，并将 scan_dirs 设为 existing。如果 agentsight.json 本身已经在 trajectory_collection 里配置了其他键（如 future 开关或限流参数），会被静默丢弃。建议在派生配置时基于原有 trajectory_collection 做字段级合并，仅追加 scan_dirs，而不是整体重写。
🤖 Generated by Qoder • Fix in Qoder
448c66c to
d9bd8a0
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 448c66c747
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| Some(list) => list.iter().map(|dir| root_from_path(dir)).collect(), | ||
| None => default_scan_roots(), | ||
| }; | ||
| let mut roots = default_scan_roots(); |
There was a problem hiding this comment.
For deployments with trajectory collection enabled and an explicit scan_dirs allowlist, unconditionally adding default_scan_roots() changes the field from an override to an append and can ingest trajectories from every /root and /home/* agent directory that the operator intentionally excluded. The commit leaves agentsight.json and CURRENT_SCHEMA_VERSION at 2, so existing version-2 configurations receive this broader collection behavior without any migration boundary; retain the override contract or bump the schema and explicitly migrate affected configurations.
AGENTS.md reference: src/agentsight/AGENTS.md:L33-L35
Useful? React with 👍 / 👎.
| for dir in list { | ||
| let canonical = dir.canonicalize().unwrap_or_else(|_| dir.clone()); | ||
| if existing.contains(&canonical) { | ||
| log::debug!("Trajectory scan: skipping duplicate dir {}", dir.display()); | ||
| continue; | ||
| } | ||
| roots.push(root_from_path(dir)); |
There was a problem hiding this comment.
When two configured entries resolve to the same canonical directory, such as a real path plus a symlink, neither is present in existing because that set contains only default roots and is never updated after roots.push. Both entries are therefore scanned despite the new documented deduplication guarantee, causing redundant traversal and potentially repeated upserts; insert each accepted canonical path into the set before pushing it.
Useful? React with 👍 / 👎.
| | Token 消费 | `features.token_consumption` | `false` | 聚合消费记录 | | ||
| | SLS Logtail | `features.sls_logtail` | `false` | SLS 日志文件导出 | | ||
| | 轨迹采集 | `features.trajectory_collection.enabled` | `false` | 定时扫描 Qoder/QoderWork 会话目录，JSONL 转 ATIF v1.7 存入 trajectories.db（仅 trace 模式；`scan_interval_secs` 默认 30，`scan_dirs` 可覆盖扫描目录） | | ||
| | 轨迹采集 | `features.trajectory_collection.enabled` | `false` | 定时扫描 Qoder/QoderWork 会话目录，JSONL 转 ATIF v1.7 存入 trajectories.db（仅 trace 模式；`scan_interval_secs` 默认 30，`scan_dirs` 可追加额外扫描目录，与默认路径合并去重） | |
There was a problem hiding this comment.
This internal navigation document now says scan_dirs is appended, but the public English and Chinese component READMEs still state that it overrides scan roots (README.md:420 and README_zh.md:404), while the user guide does not document the option. Users following those references can configure a narrow root and unexpectedly collect the defaults as well; update both README variants and the user-guide configuration reference with the semantic change.
AGENTS.md reference: AGENTS.md:L337-L343
Useful? React with 👍 / 👎.
| with open(src) as f: | ||
| cfg = json.load(f) | ||
|
|
||
| # 追加 ~/.agentsight/skillrubric/traces 到 scan_dirs（去重） |
There was a problem hiding this comment.
The newly added Python comment is Chinese, contrary to the repository-wide requirement that all code and comments be English; translate it so the changed script remains accessible under the documented language convention.
AGENTS.md reference: AGENTS.md:L345-L349
Useful? React with 👍 / 👎.
- discovery.rs: recognize .agentsight paths in source_from_path so ingested trajectories are labelled source="agentsight" - discovery.rs: change scan_dirs semantics from replace to append mode (default roots + custom dirs, deduplicated by canonical path) - lib.rs: fix test_scan_once_skips_empty_steps_session UUID collision with real sessions on multi-user machines (use synthetic UUID) - AGENTS.md: document scan_dirs append semantics - .gitignore: exclude dev.sh (local dev script, not for repo)
d9bd8a0 to
f581ecd
Compare
Daydreamer-Li
left a comment
Daydreamer-Li
left a comment
There was a problem hiding this comment.
Review: merge-ready after bumping schema_version for the scan_dirs semantic change
Overall a clean, well-tested change. cargo fmt, cargo clippy -D warnings, and the Test agentsight CI job all pass. I found 1 blocking issue (schema contract) and 4 non-blocking suggestions.
1. scan_dirs semantics changed but schema_version was not bumped — P1 (blocking)
What is wrong: This PR changes scan_dirs from overriding the default scan roots to appending to them. That is a semantic change to an existing agentsight.json field. The repo's AGENTS.md hard rule says a semantic change to agentsight.json requires bumping both schema_version in agentsight.json and CURRENT_SCHEMA_VERSION in src/config.rs. Both are still 2.
Where: src/agentsight/agentsight.json:2, src/agentsight/src/config.rs:151
Impact: A user who previously set scan_dirs to limit scanning to one directory will now also scan /root and /home/* after upgrade, potentially ingesting unexpected trajectories, with no schema-version signal that the contract changed.
2. config.rs doc comment still says "override" — P2
Where: src/agentsight/src/config.rs:381
Impact: The comment on JsonTrajectoryCollectionFeature.scan_dirs still reads "Optional override of the projects directories to scan", which now contradicts the append behavior.
3. Append mode can double-scan overlapping sessions — P2
What is wrong: Deduplication is only by directory canonical path. If a configured scan_dirs entry overlaps a default root in content but not in path (e.g., the custom dir is a parent of a default root), the same session file can be discovered twice.
Where: src/agentsight/crates/agentsight-trajectory-collector/src/discovery.rs:92-125
Impact: Duplicate trajectories may be stored, showing duplicated sessions in the Dashboard. The old override mode avoided this because default roots were never scanned together with custom dirs.
Suggestion: Deduplicate discovered sessions by session_id or full file path before returning from discover_sessions.
4. AGENTS.md module map is out of sync — P2/P3
Where: src/agentsight/AGENTS.md:102
Impact: The TrajectoryCollector responsibility description still only mentions Qoder/QoderWork, but the code already supports Claude, Codex, Cursor, and now agentsight.
5. Layout assumption for .agentsight is undocumented — P2/P3
What is wrong: .agentsight is not in SESSION_ROOTS, so a configured .agentsight path falls through to the backward-compatible default in root_from_path, which infers Layout::PerProject (expects traces/<project>/<uuid>.jsonl). The PR does not state whether ~/.agentsight/skillrubric/traces/ actually has project subdirectories.
Where: src/agentsight/crates/agentsight-trajectory-collector/src/discovery.rs:175-204
Impact: If the directory uses a flat layout (traces/<uuid>.jsonl), the README configuration will discover nothing.
What looked good
- Append semantics are more useful than the old override behavior.
- Tests cover the new append behavior and the
agentsightsource label. - The UUID-collision fix for multi-user machines is solid.
fmt/clippypass; CI is green.
Review coverage
- Reviewed: discovery scanning logic, source labeling, deduplication, config parsing,
schema_versionpolicy, doc consistency, fmt/clippy, CI status. - Not verified: real
~/.agentsight/skillrubric/tracesdirectory layout; actual multi-user scan behavior on a machine with real sessions.
|
Thanks for the thorough review @Daydreamer-Li! Re P1 (schema_version bump): Re P2 (doc comment / dedup / module map): valid observations. The feature's opt-in nature keeps the blast radius minimal for now. I'll address the doc comment wording in a follow-up cleanup pass. Re P2/P3 (layout assumption): |
2 participants
Why
~/.agentsight/skillrubric/traces/下存放了 skillrubric 评测产生的轨迹日志（标准 Qoder JSONL 格式），但轨迹采集器默认不扫描该目录，且路径推断不识别.agentsight前缀。What
source_from_path增加.agentsight路径识别，入库轨迹 source 标记为"agentsight"scan_dirs语义从覆盖改为追加（默认根 + 自定义目录合并去重）test_scan_once_skips_empty_steps_session在多用户机器上与真实会话 UUID 碰撞dev.sh（本地开发脚本，不入仓库）Usage
在
agentsight.json（或--config指定的配置文件）中追加扫描目录：{ "features": { "trajectory_collection": { "enabled": true, "scan_dirs": ["/root/.agentsight/skillrubric/traces"] } } }Test
E2E 验证：配置 scan_dirs 后 trace collector 自动扫描，新格式轨迹（含 user/assistant 事件）正常入库，Dashboard sessions 页可见。