Split OMP from Pi usage - #32
Conversation
Reviewer's GuideThis PR splits Oh My Pi (OMP) out from Pi into a first-class, independent client while reusing Pi’s JSONL format, updating scanning, parsing, CLI/TUI/frontend plumbing, tests, cache schema, and documentation accordingly. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
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.
Code Review
This pull request separates OMP (Oh My Pi) into an independent client distinct from the Pi client, updating the CLI, core library, and frontend to support OMP sessions. Feedback on the pull request suggests adding the Pi logo icon to the OMP entries in the localized README files to maintain visual consistency.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The tests that assert exact positions in
ClientId::ALL(e.g. indices 9..25) are quite brittle and will keep needing updates when clients are added or reordered; consider asserting presence and/or relative ordering instead of hard-coding full index sequences. - Pi and OMP share the same logo and color in multiple places (
client_logo_url,SOURCE_LOGOS,SourceLogoswitch,SOURCE_COLORS); it may be worth centralizing this mapping or adding a brief comment to avoid accidental divergence if one of them is changed later.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The tests that assert exact positions in `ClientId::ALL` (e.g. indices 9..25) are quite brittle and will keep needing updates when clients are added or reordered; consider asserting presence and/or relative ordering instead of hard-coding full index sequences.
- Pi and OMP share the same logo and color in multiple places (`client_logo_url`, `SOURCE_LOGOS`, `SourceLogo` switch, `SOURCE_COLORS`); it may be worth centralizing this mapping or adding a brief comment to avoid accidental divergence if one of them is changed later.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Bot review 处理记录:
|
Closes #31
变更内容
omp注册为独立 client id,扫描~/.omp/agent/sessions,不再并入pipi只扫描~/.pi/agent/sessions,解析结果继续使用 client idpi,展示为Piomp复用 Pi JSONL 解析格式,但解析结果使用 client idomp,CLI 通过--client omp过滤,TUI/frontend/wrapped 展示为OMP.omp写在 Pi 下的描述;不引入oh-my-pi/oh_my_pi/--omp这类 alias.omp被当作pi的结果继续污染统计验证
rtk cargo fmtrtk cargo check -p tokscale-clirtk cargo test -p tokscale-core omprtk cargo test -p tokscale-cli client_filterrtk cargo test -p tokscale-cli wrappedrtk cargo test -p tokscale-cli test_models_client_filter_splits_pi_and_omp_sessionsrtk cargo testrtk bun run --cwd packages/frontend test __tests__/lib/clientRegistry.test.tsrtk bun run --cwd packages/frontend test __tests__/api/submit.test.tsrtk bun run --cwd packages/frontend testrtk bun run --cwd packages/frontend lint(0 errors;仓库现有 9 warnings)rtk bun run build:cliSummary by Sourcery
Register OMP as a first-class client separate from Pi and ensure its sessions, parsing, filtering, and presentation are handled independently across the core, CLI, and frontend.
New Features:
Bug Fixes:
Enhancements:
Summary by CodeRabbit
发布说明
新功能
文档
测试