fix(FocalPoint): add serde_json::json import, align clippy MSRV#61
fix(FocalPoint): add serde_json::json import, align clippy MSRV#61KooshaPari wants to merge 33 commits into
Conversation
Issue 1: pii_scrubber test used json!() macro without importing it from serde_json. Issue 2: clippy.toml MSRV (1.75) did not match workspace rust-version (1.82). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
📝 WalkthroughWalkthroughAdds STATUS.md and expands SPEC/spec.md; updates Cargo workspace members and adds a Git-pinned workspace dependency; switches many crate manifests to workspace resolution; refactors benchmark harnesses and bench code to use helper constructors and in-memory loops; refactors Notion parsing and test fixtures; adds small test imports and test helpers; updates CI/workflows, ignores, and lint settings; and removes Cargo.toml.bak. ChangesDocumentation & Specification
Benchmarks & Cargo Harness Changes
Connector parsing refactor
Small test imports & test helpers
Repository tooling, CI, and ignore rules
Workspace backup removal
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![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 updates the project status in STATUS.md to active development, bumps the Minimum Supported Rust Version (MSRV) to 1.82 in clippy.toml, and adds a JSON utility import for telemetry tests. Feedback suggests clarifying the project's lifecycle status to avoid contradictions with previous documentation and recommends removing the redundant MSRV configuration in clippy.toml to simplify maintenance.
| # FocalPoint Status | ||
|
|
||
| FocalPoint is a focus and productivity tracking application. | ||
|
|
||
| ## Current Status | ||
|
|
||
| Active development. See README.md for project details. |
There was a problem hiding this comment.
The previous content of this file referenced SUPERSEDED.md, which contradicts the new 'Active development' status. If the project has been deprecated or replaced, the status should be updated to reflect its actual lifecycle stage rather than using a generic template.
| @@ -1,5 +1,5 @@ | |||
| # Phenotype-org standard clippy config | |||
| msrv = "1.75" | |||
| msrv = "1.82" | |||
There was a problem hiding this comment.
Clippy automatically detects the Minimum Supported Rust Version (MSRV) from the rust-version field in Cargo.toml. Since you are aligning this with the workspace version, consider removing this line from clippy.toml to avoid maintaining the same value in two places.
codeant-ai
Bot
added
the
size:XS
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
Status: No Issues Found | Recommendation: Merge
Files Reviewed (3 files)
STATUS.md— Replaced corrupted status file with proper documentationclippy.toml— Bumped MSRV from 1.75 to 1.82 to match workspace rust-versioncrates/focus-telemetry/src/pii_scrubber.rs— Added missinguse serde_json::json;import for test module
All changes are correct, minimal, and address legitimate issues:
- Missing import was causing test compilation failures
- MSRV misalignment could cause clippy inconsistencies
- STATUS.md content restored to meaningful documentation
|
CodeAnt AI finished reviewing your PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…backup - scorecard.yml: fix malformed checkout action (two SHAs concatenated) - cargo-deny.yml: update checkout to pinned v4 SHA (11bd71901bbe5b1630ceea73d27597364c9af683) - CLAUDE.md: correct crate count from "54+" to "56" (matches cargo metadata) - Remove Cargo.toml.bak stale backup from repo root
- Add MockFamilyControls connector (FR-MOCK-001) with deterministic event generation, polling cadence, and full event type coverage - Add RuleSuggester with 4 heuristic patterns: scheduled focus sessions, missing celebrations, missed check-ins, unlinked actions - Enable criterion benchmarks for focus-eval and focus-rules (harness=true on eval_tick, eval_batched, rule_evaluation) - Add dev-dependencies to focus-policy (criterion, anyhow) - Harden deny.toml wildcards policy: warn → deny - Restore SPEC.md and spec.md to HEAD state Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
KooshaPari
dismissed stale reviews from coderabbitai[bot] and kilo-code-bot[bot]
via
d80636c
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:XL
|
CodeAnt AI Incremental review completed. |
Align spec.md with SPEC.md — both now contain the full implemented specification covering purpose, users, scope boundaries, architecture, connector runtime, rules engine, reward/penalty ledger, audit chain, mascot state machine, iOS app, and API design. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| pnpm-lock.yaml | ||
| # Rust | ||
| **/.cargo | ||
| **/Cargo.lock |
There was a problem hiding this comment.
Dockerignore excludes Cargo.lock preventing reproducible builds
Medium Severity
The **/Cargo.lock pattern excludes the workspace Cargo.lock from the Docker build context. For a workspace containing binary targets (CLI tools, servers), this file is essential for deterministic, reproducible builds. Without it, cargo build inside Docker will resolve dependencies to whatever latest-compatible versions are available at build time, which may differ from what was tested locally or in CI.
Reviewed by Cursor Bugbot for commit 1d28c48. Configure here.
…JSON parsing - connector-gcal: hold WATCH_LOCK for entire test body to prevent env var leaking between parallel sibling tests - connector-notion: add fallback for single-object (non-array) responses, support both plain_text and content/text.title paths Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
crates/connector-notion/src/api.rs (2)
222-242:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
parse_task_with_different_statusesfixtures are silently broken — test always passes vacuously.Both
task_todoandtask_donefixtures are missinglast_edited_time, whichparse_single_taskrequires via?. Both calls tofrom_notion_jsonreturn an emptyVec, andassert!(true)on line 241 passes unconditionally, so this test exercises nothing and provides false coverage confidence for FR-NOTION-API-007.🐛 Proposed fix
let task_todo = serde_json::json!({ "object": "page", "id": "task_todo", + "last_edited_time": "2026-04-24T10:00:00.000Z", "properties": { - "status": {"select": {"name": "Todo"}} + "title": {"title": [{"plain_text": "Todo Task"}]}, + "Completed": {"checkbox": false}, + "status": {"select": {"name": "Todo"}} } }); let task_done = serde_json::json!({ "object": "page", "id": "task_done", + "last_edited_time": "2026-04-24T11:00:00.000Z", "properties": { - "status": {"select": {"name": "Done"}} + "title": {"title": [{"plain_text": "Done Task"}]}, + "Completed": {"checkbox": true}, + "status": {"select": {"name": "Done"}} } }); let tasks_todo = NotionTask::from_notion_json(&task_todo); let tasks_done = NotionTask::from_notion_json(&task_done); - assert!(true); + assert_eq!(tasks_todo.len(), 1); + assert!(!tasks_todo[0].completed); + assert_eq!(tasks_done.len(), 1); + assert!(tasks_done[0].completed);🤖 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/connector-notion/src/api.rs` around lines 222 - 242, The test parse_task_with_different_statuses constructs fixtures missing the required last_edited_time so NotionTask::from_notion_json (which calls parse_single_task) returns empty Vecs and the test vacuously passes; update the fixtures to include a valid last_edited_time timestamp (e.g. ISO 8601 string) in both task_todo and task_done properties so parse_single_task can succeed, then replace assert!(true) with assertions that the returned Vecs are non-empty and have the expected status values to properly exercise from_notion_json.
246-260:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
parse_paginated_responseresult items lack required fields — parsed pages are silently dropped andassert!(true)masks the issue.The inner page objects
{"object": "page", "id": "p1/p2", "properties": {}}are missingurl,created_time, andlast_edited_time.parse_single_pagereturnsNonefor each (via?), sopagesis always empty. The test claims to verify that the function handles pagination metadata (FR-NOTION-API-008) but never confirms that any pages are actually parsed.🐛 Proposed fix
let paginated = serde_json::json!({ "object": "list", "results": [ - {"object": "page", "id": "p1", "properties": {}}, - {"object": "page", "id": "p2", "properties": {}}, + { + "object": "page", + "id": "p1", + "url": "https://notion.so/p1", + "created_time": "2026-04-23T10:00:00.000Z", + "last_edited_time": "2026-04-24T10:00:00.000Z", + "properties": {"title": {"title": [{"plain_text": "Page 1"}]}} + }, + { + "object": "page", + "id": "p2", + "url": "https://notion.so/p2", + "created_time": "2026-04-23T11:00:00.000Z", + "last_edited_time": "2026-04-24T11:00:00.000Z", + "properties": {"title": {"title": [{"plain_text": "Page 2"}]}} + }, ], "next_cursor": "cursor_abc", "has_more": true }); let pages = NotionPage::from_notion_json(&paginated); - // Should parse without error even with pagination metadata - assert!(true); + assert_eq!(pages.len(), 2, "Both pages should parse successfully");🤖 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/connector-notion/src/api.rs` around lines 246 - 260, The test parse_paginated_response is creating page objects missing required fields so parse_single_page returns None and the test masks the bug with assert!(true); update the test data in parse_paginated_response to include the required fields ("url", "created_time", "last_edited_time") for each inner page and then assert that NotionPage::from_notion_json(&paginated) returns a non-empty Vec (or expected count) to validate pages are parsed despite pagination metadata; reference parse_single_page and NotionPage::from_notion_json to locate the parsing logic and ensure the test checks both parsed pages and that pagination keys (next_cursor/has_more) are ignored.crates/focus-rules/benches/rule_evaluation.rs (1)
130-154:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
Utc::now()insideb.iter()benchmarks time-fetching, not cooldown map lookups.Line 146 calls
Utc::now()on every benchmark iteration. That is a syscall (clock_gettime) and is far more expensive than theHashMap::getbeing profiled. The expiry timestamps should be captured once beforeb.iter()so that each iteration measures only the map lookup.🐛 Proposed fix
b.iter(|| { let mut hits = 0; + let now = Utc::now(); for i in 0..1000 { let key = format!("rule_{}", i); if let Some(expires_at) = cooldowns.get(&key) { - if *expires_at > Utc::now() { + if *expires_at > now { hits += 1; } } } black_box(hits) });🤖 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/focus-rules/benches/rule_evaluation.rs` around lines 130 - 154, In bench_cooldown_map_hit_path the benchmark calls Utc::now() inside b.iter(), measuring time syscalls instead of HashMap lookup; move the time capture(s) out of the inner closure by computing the expiry timestamps once when building cooldowns (or capture a single fixed now value before b.iter()) so the loop only does cooldowns.get(&key) and compares against the precomputed expires_at values; update the variables referenced (cooldowns, expires_at, hits) accordingly so the inner b.iter() no longer calls Utc::now().crates/focus-eval/Cargo.toml (1)
18-18:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftOut-of-workspace path dependency is the root cause of all CI pipeline failures.
The path
../../../PhenoObservability/crates/phenotype-observably-macrosresolves three levels abovecrates/focus-eval/, landing outside the FocalPoint repository. This requires a sibling checkout ofPhenoObservabilityon the CI runner's filesystem. The pipeline failure confirms this:"failed to read '/home/runner/work/FocalPoint/PhenoObservability/crates/phenotype-observably-macros/Cargo.toml': No such file or directory".Options to unblock CI:
- Add a
git submoduleforPhenoObservabilityand update the CI checkout step to initialise submodules.- Publish
phenotype-observably-macrosto crates.io (or a private registry) and reference it by version.- If the crate is only needed for instrumentation, gate it behind a Cargo feature and disable that feature in CI.
🤖 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/focus-eval/Cargo.toml` at line 18, The Cargo.toml in the focus-eval crate references an out-of-repo path dependency phenotype-observably-macros that breaks CI; fix by replacing the path dependency with one of the three recommended approaches: (A) add PhenoObservability as a git submodule and update CI to init submodules, then keep the path but point it inside the repo tree; (B) publish phenotype-observably-macros to crates.io or a private registry and change the dependency in crates/focus-eval/Cargo.toml to use the published version string instead of the path; or (C) gate the dependency behind a Cargo feature (e.g., feature "observability" in focus-eval's Cargo.toml) and disable that feature in CI so the path/publish dependency is not required during CI builds—choose one option, update crates/focus-eval/Cargo.toml (and CI checkout or feature flags) accordingly, and ensure Cargo.lock/CI configs reflect the change.crates/focus-eval/benches/eval_tick.rs (1)
57-82:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
_eventon line 62 is allocated but never used in the measured closure.
make_event(0)is called once and assigned to_event, but the|b, _|closure at line 67 only usesrulesfrom the enclosing scope. The allocation is completely dead. If a per-event path is intended here (e.g., passing_eventinto the rule-check body), it needs to be captured; otherwise, remove themake_event(0)call.🐛 Proposed fix (remove dead allocation)
for rule_count in [10, 25, 50, 100] { let rules = black_box((0..rule_count).map(make_rule).collect::<Vec<_>>()); - let _event = black_box(make_event(0)); group.bench_with_input(🤖 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/focus-eval/benches/eval_tick.rs` around lines 57 - 82, The variable `_event` created by calling `make_event(0)` inside `bench_eval_tick_per_event_latency` is allocated but never used inside the benchmark closure `|b, _|`; either remove the dead allocation (delete the `_event` assignment) or actually use it in the measured loop (capture `_event` and pass it into the rule-check body) so the benchmark measures the intended per-event path—update the code around the `let _event = black_box(make_event(0));` line and the closure body accordingly.
🤖 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 @.dockerignore:
- Around line 18-24: The dockerignore currently excludes "**/tests/" which is
too broad and can break Rust crates or other projects that rely on a tests/ tree
for integration tests or shared fixtures; update the .dockerignore to stop
excluding the entire "**/tests/" directory and instead exclude only the specific
unwanted artifacts (e.g., coverage outputs, generated test artifacts, or
temporary files) so that real test sources and fixtures are preserved while
unwanted build/coverage files remain ignored.
- Around line 7-9: The .dockerignore has duplicate exclusion rules: the
recursive globs '**/build/' and '**/target/' are redundant because 'build/' and
'target/' are already present; remove the redundant '**/build/' and '**/target/'
entries (the patterns named "**/build/" and "**/target/") so each directory
exclusion appears only once and avoid duplicate rules.
- Around line 41-42: The .dockerignore currently excludes the Cargo.lock file
via the pattern "**/Cargo.lock"; remove or comment out that pattern so
Cargo.lock is included in the Docker build context (leave other ignores like
"**/.cargo" intact). Ensure the entry "**/Cargo.lock" is deleted or disabled in
.dockerignore so cargo builds inside the image use the committed Cargo.lock for
reproducible dependency resolution.
In @.github/workflows/cargo-deny.yml:
- Line 23: Add a concise version comment next to the pinned SHA for the checkout
action: update the usages reference that currently shows
actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 to include an inline
comment noting the human-readable tag (v4.2.2) so maintainers and tools can see
the version; modify the same line (the actions/checkout reference) to append a
comment like “# v4.2.2” ensuring the SHA remains pinned and the comment only
documents the corresponding version.
In @.github/workflows/scorecard.yml:
- Line 22: Add the same human-readable version comment for the checkout action:
locate the `uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683`
entry and append the `# v4.2.2` comment (matching the `actions/checkout@v4.2.2`
SHA used elsewhere) so Dependabot and reviewers can easily see the intended
version.
In `@crates/connector-notion/src/models.rs`:
- Around line 31-46: Extract the duplicated title-extraction chain into a new
module-level helper function (e.g., fn extract_title(page: &serde_json::Value)
-> &str) and replace the inline chains in parse_single_page and
parse_single_task with calls to this helper; ensure the helper performs the same
get("properties")...unwrap_or("Untitled") logic and is placed above the
NotionPage impl so both parse_single_page and parse_single_task reference it,
preserving the existing return behavior and lifetime semantics used by those
functions.
In `@crates/focus-eval/benches/eval_batched.rs`:
- Around line 36-54: The benchmark currently only reads rule.enabled (in
bench_eval_batched_1000x50 and the related benches) which the optimizer can
constant-fold because make_rule sets enabled: true and events are unused;
replace the trivial branch with real evaluation by either invoking
BatchedRuleEvaluationPipeline::tick (or the pipeline evaluation function used
originally) for each event and rule, or by constructing rules with non-trivial
Condition objects and calling the Condition evaluation against each event so the
event data is consulted; ensure make_rule and make_event produce varying data
(e.g., some rules disabled or differing conditions) and wrap results in
black_box to prevent folding while keeping the benchmark body iterating over
events and calling the actual evaluation code rather than only accessing
rule.enabled.
- Around line 6-34: Extract the duplicated make_event(i: usize) ->
NormalizedEvent and make_rule(i: i32) -> Rule into a shared benches helper
module (e.g., a public module named bench_helpers) so both eval_batched.rs and
eval_tick.rs call the same functions; make the functions pub, add
#[allow(dead_code)] on the module or functions if needed, re-export or import
them in both bench files, and remove the local duplicates (ensure signatures and
returned types match exactly: NormalizedEvent, Rule, Trigger::Event,
Action::GrantCredit, etc.).
In `@crates/focus-eval/Cargo.toml`:
- Around line 34-40: The Cargo.toml bench entries for the Criterion benchmarks
(the [[bench]] sections with name = "eval_tick" and name = "eval_batched")
currently set harness = true which enables Cargo's test harness and conflicts
with Criterion's own harness used via criterion_group!() and criterion_main!()
in the benchmark source; change harness = true to harness = false for both bench
entries so Cargo does not inject its test harness and the Criterion macros can
provide the main function without a linker conflict.
In `@crates/focus-policy/Cargo.toml`:
- Around line 22-24: The bench configuration sets harness = true which conflicts
with Criterion's criterion_main! (duplicate main); change the bench table in
Cargo.toml (the [[bench]] stanza for name "focus_policy_benchmarks") to use
harness = false so the test harness is not injected and Criterion's generated
main is used instead.
In `@crates/focus-rules/Cargo.toml`:
- Around line 26-28: The Cargo.toml bench configuration for the benchmark named
"rule_evaluation" currently sets harness = true which prevents Criterion's own
harness from running; change the bench table for the "rule_evaluation" benchmark
(the [[bench]] entry with name = "rule_evaluation") to set harness = false so
Criterion's criterion_main!/criterion_group! harness can be used.
In `@crates/focus-templates/src/lib.rs`:
- Around line 396-423: The helper functions bytes_to_hex and base64_encode are
test-only and should be moved into the test module: cut bytes_to_hex and
base64_encode from the module root and paste them inside the existing
#[cfg(test)] mod tests { ... } block, remove the #[allow(dead_code)] attributes,
and update any test references (they already call bytes_to_hex/base64_encode) so
they resolve to the in-module helpers; keep base64_encode behavior as-is for
current tests but be aware it produces unpadded base64 if reused later.
In `@spec.md`:
- Around line 1-4: Two spec files (spec.md and SPEC.md) are identical causing
duplication; choose a canonical name (preferably SPEC.md) and remove the
duplicate file. Delete spec.md (or conversely delete SPEC.md if you prefer the
lowercase), update any references in README, docs, CI, or other code that import
or link to the removed filename to point to the chosen canonical file, and
ensure the repository root contains only the single spec file (SPEC.md) so
future edits won't diverge.
- Around line 52-100: The fenced ASCII-art block uses a bare triple-backtick
fence which triggers MD040; update the opening fence from ``` to ```text so
Markdownlint recognizes it as plain text (leave the ASCII diagram content and
the closing ``` unchanged); locate the block that begins with the triple
backticks surrounding the diagram (the ASCII box/diagram lines shown in the
diff) and add the language identifier "text" immediately after the opening
backticks.
In `@SPEC.md`:
- Around line 52-100: The fenced ASCII diagram block starting with
"┌─────────────────────────────────────────────────────────────────────────┐" is
unlabeled; update that triple-backtick fence to include the text language tag
(use ```text) so the block is treated as plain text/ASCII in rendered docs;
locate the large ASCII art block (the section showing "SwiftUI iOS App" and
"Rust Core (54 crates)" and change the opening fence to ```text (and the closing
fence remains ```).
---
Outside diff comments:
In `@crates/connector-notion/src/api.rs`:
- Around line 222-242: The test parse_task_with_different_statuses constructs
fixtures missing the required last_edited_time so NotionTask::from_notion_json
(which calls parse_single_task) returns empty Vecs and the test vacuously
passes; update the fixtures to include a valid last_edited_time timestamp (e.g.
ISO 8601 string) in both task_todo and task_done properties so parse_single_task
can succeed, then replace assert!(true) with assertions that the returned Vecs
are non-empty and have the expected status values to properly exercise
from_notion_json.
- Around line 246-260: The test parse_paginated_response is creating page
objects missing required fields so parse_single_page returns None and the test
masks the bug with assert!(true); update the test data in
parse_paginated_response to include the required fields ("url", "created_time",
"last_edited_time") for each inner page and then assert that
NotionPage::from_notion_json(&paginated) returns a non-empty Vec (or expected
count) to validate pages are parsed despite pagination metadata; reference
parse_single_page and NotionPage::from_notion_json to locate the parsing logic
and ensure the test checks both parsed pages and that pagination keys
(next_cursor/has_more) are ignored.
In `@crates/focus-eval/benches/eval_tick.rs`:
- Around line 57-82: The variable `_event` created by calling `make_event(0)`
inside `bench_eval_tick_per_event_latency` is allocated but never used inside
the benchmark closure `|b, _|`; either remove the dead allocation (delete the
`_event` assignment) or actually use it in the measured loop (capture `_event`
and pass it into the rule-check body) so the benchmark measures the intended
per-event path—update the code around the `let _event =
black_box(make_event(0));` line and the closure body accordingly.
In `@crates/focus-eval/Cargo.toml`:
- Line 18: The Cargo.toml in the focus-eval crate references an out-of-repo path
dependency phenotype-observably-macros that breaks CI; fix by replacing the path
dependency with one of the three recommended approaches: (A) add
PhenoObservability as a git submodule and update CI to init submodules, then
keep the path but point it inside the repo tree; (B) publish
phenotype-observably-macros to crates.io or a private registry and change the
dependency in crates/focus-eval/Cargo.toml to use the published version string
instead of the path; or (C) gate the dependency behind a Cargo feature (e.g.,
feature "observability" in focus-eval's Cargo.toml) and disable that feature in
CI so the path/publish dependency is not required during CI builds—choose one
option, update crates/focus-eval/Cargo.toml (and CI checkout or feature flags)
accordingly, and ensure Cargo.lock/CI configs reflect the change.
In `@crates/focus-rules/benches/rule_evaluation.rs`:
- Around line 130-154: In bench_cooldown_map_hit_path the benchmark calls
Utc::now() inside b.iter(), measuring time syscalls instead of HashMap lookup;
move the time capture(s) out of the inner closure by computing the expiry
timestamps once when building cooldowns (or capture a single fixed now value
before b.iter()) so the loop only does cooldowns.get(&key) and compares against
the precomputed expires_at values; update the variables referenced (cooldowns,
expires_at, hits) accordingly so the inner b.iter() no longer calls Utc::now().
🪄 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: ASSERTIVE
Plan: Pro
Run ID: cf471ba5-3ab5-4946-955f-65ec665432be
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
.dockerignore.github/workflows/cargo-deny.yml.github/workflows/scorecard.yml.gitignoreCLAUDE.mdCargo.toml.bakSPEC.mdcrates/connector-notion/src/api.rscrates/connector-notion/src/models.rscrates/focus-connectors-mock-familycontrols/src/lib.rscrates/focus-eval/Cargo.tomlcrates/focus-eval/benches/eval_batched.rscrates/focus-eval/benches/eval_tick.rscrates/focus-policy/Cargo.tomlcrates/focus-policy/benches/focus_policy_benchmarks.rscrates/focus-rule-suggester/src/lib.rscrates/focus-rules/Cargo.tomlcrates/focus-rules/benches/rule_evaluation.rscrates/focus-templates/src/lib.rsdeny.tomlspec.md
💤 Files with no reviewable changes (2)
- Cargo.toml.bak
- crates/focus-policy/benches/focus_policy_benchmarks.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Cursor Bugbot
- GitHub Check: semgrep-cloud-platform/scan
🧰 Additional context used
📓 Path-based instructions (2)
deny.toml
📄 CodeRabbit inference engine (CLAUDE.md)
Use deny.toml for cargo-deny security configuration
Files:
deny.toml
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Run quality gate checks:
cargo test --workspace && cargo clippy --all -- -D warnings && cargo fmt --allFormat Rust code using
cargo fmt --allLint Rust code with
cargo clippy --all -- -D warnings(treat warnings as errors)Run tests using
cargo test --workspace
Files:
crates/connector-notion/src/api.rscrates/focus-rule-suggester/src/lib.rscrates/connector-notion/src/models.rscrates/focus-eval/benches/eval_batched.rscrates/focus-rules/benches/rule_evaluation.rscrates/focus-connectors-mock-familycontrols/src/lib.rscrates/focus-templates/src/lib.rscrates/focus-eval/benches/eval_tick.rs
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: KooshaPari/FocalPoint
Timestamp: 2026-05-07T01:20:04.266Z
Learning: Maintain a portable Rust core with a cargo workspace structure (56 crates)
Learnt from: CR
Repo: KooshaPari/FocalPoint
Timestamp: 2026-05-07T01:20:04.266Z
Learning: Use SQLite, PostgreSQL, or SurrealDB for data persistence
🪛 GitHub Actions: cargo-deny / 0_cargo-deny.txt
crates/focus-rules/Cargo.toml
[error] 1-1: cargo-deny (command: check --all-features --manifest-path ./Cargo.toml) failed: failed to fetch crates; failed to load manifest for workspace member /github/workspace/crates/focus-always-on; failed to load manifest for dependency phenotype-observably-macros; failed to read /github/PhenoObservability/crates/phenotype-observably-macros/Cargo.toml: No such file or directory (os error 2).
crates/focus-eval/Cargo.toml
[error] 1-1: cargo-deny (command: check --all-features --manifest-path ./Cargo.toml) failed: failed to fetch crates; failed to load manifest for workspace member /github/workspace/crates/focus-always-on; failed to load manifest for dependency phenotype-observably-macros; failed to read /github/PhenoObservability/crates/phenotype-observably-macros/Cargo.toml: No such file or directory (os error 2).
crates/focus-policy/Cargo.toml
[error] 1-1: cargo-deny (command: check --all-features --manifest-path ./Cargo.toml) failed: failed to fetch crates; failed to load manifest for workspace member /github/workspace/crates/focus-always-on; failed to load manifest for dependency phenotype-observably-macros; failed to read /github/PhenoObservability/crates/phenotype-observably-macros/Cargo.toml: No such file or directory (os error 2).
🪛 GitHub Actions: CI / 0_test.txt
crates/focus-rules/Cargo.toml
[error] 1-1: Cargo test failed: failed to load manifest for workspace member '/home/runner/work/FocalPoint/FocalPoint/crates/focus-always-on'. Cause: failed to load dependency 'phenotype-observably-macros' because Cargo could not read '/home/runner/work/FocalPoint/PhenoObservability/crates/phenotype-observably-macros/Cargo.toml' (No such file or directory, os error 2).
crates/focus-eval/Cargo.toml
[error] 1-1: Cargo test failed: failed to load manifest for workspace member '/home/runner/work/FocalPoint/FocalPoint/crates/focus-always-on'. Cause: failed to load dependency 'phenotype-observably-macros' because Cargo could not read '/home/runner/work/FocalPoint/PhenoObservability/crates/phenotype-observably-macros/Cargo.toml' (No such file or directory, os error 2).
crates/focus-policy/Cargo.toml
[error] 1-1: Cargo test failed: failed to load manifest for workspace member '/home/runner/work/FocalPoint/FocalPoint/crates/focus-always-on'. Cause: failed to load dependency 'phenotype-observably-macros' because Cargo could not read '/home/runner/work/FocalPoint/PhenoObservability/crates/phenotype-observably-macros/Cargo.toml' (No such file or directory, os error 2).
🪛 GitHub Actions: CI / test
crates/focus-rules/Cargo.toml
[error] 1-1: Swatinem/rust-cache step failed: 'cargo metadata --all-features --format-version 1 --no-deps' exited with code 101. Error: failed to load manifest for workspace member '/home/runner/work/FocalPoint/FocalPoint/crates/focus-always-on' referenced by workspace at '/home/runner/work/FocalPoint/FocalPoint/Cargo.toml'.
crates/focus-eval/Cargo.toml
[error] 1-1: Swatinem/rust-cache step failed: 'cargo metadata --all-features --format-version 1 --no-deps' exited with code 101. Error: failed to load manifest for workspace member '/home/runner/work/FocalPoint/FocalPoint/crates/focus-always-on' referenced by workspace at '/home/runner/work/FocalPoint/FocalPoint/Cargo.toml'.
crates/focus-policy/Cargo.toml
[error] 1-1: Swatinem/rust-cache step failed: 'cargo metadata --all-features --format-version 1 --no-deps' exited with code 101. Error: failed to load manifest for workspace member '/home/runner/work/FocalPoint/FocalPoint/crates/focus-always-on' referenced by workspace at '/home/runner/work/FocalPoint/FocalPoint/Cargo.toml'.
🪛 markdownlint-cli2 (0.22.1)
spec.md
[warning] 52-52: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
SPEC.md
[warning] 52-52: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (6)
CLAUDE.md (2)
8-8: Accurate workspace count update.This line now reflects the current workspace size clearly and consistently.
Based on learnings: "Maintain a portable Rust core with a cargo workspace structure (56 crates)".
37-37: Key Files section is consistent with the stack section.Good sync on the crate count here; this avoids doc drift.
Based on learnings: "Maintain a portable Rust core with a cargo workspace structure (56 crates)".
crates/focus-rule-suggester/src/lib.rs (1)
480-480: Good fix: trait import is required forwith_hourin tests.
chrono::Timelikeis needed in scope forwith_hour(...)calls, so this import correctly resolves test compilation.crates/focus-connectors-mock-familycontrols/src/lib.rs (1)
205-205: LGTM: Test import addition is clean and necessary.The
EventTypeimport correctly enables type-safe event type assertions in the test functions below (lines 273, 287), allowing tests to verify that synthetic events generate the expected custom event types.deny.toml (1)
34-36: LGTM — tighteningwildcardsto"deny"is the right call.Promoting wildcard dependency references from a warning to a hard failure prevents accidental or supply-chain-risky
*version ranges from silently passing CI. As per coding guidelines,deny.tomlis the correct place to enforce this policy..gitignore (1)
4-7: LGTM.Both
/target(Rust build artifacts) and/examples/fpl(scaffold output) are appropriate gitignore additions.
| target/ | ||
| dist/ | ||
| build/ |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Duplicate exclusion rules for target/ and build/.
target/ (Line 7) and build/ (Line 9) are already covered at the root level. **/build/ (Line 50) and **/target/ (Line 51) are redundant.
🔧 Proposed cleanup
# Java
*.class
**/.gradle
-**/build/
-**/target/
**/.maven/Also applies to: 50-51
🤖 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 @.dockerignore around lines 7 - 9, The .dockerignore has duplicate exclusion
rules: the recursive globs '**/build/' and '**/target/' are redundant because
'build/' and 'target/' are already present; remove the redundant '**/build/' and
'**/target/' entries (the patterns named "**/build/" and "**/target/") so each
directory exclusion appears only once and avoid duplicate rules.
| # Test/nested | ||
| **/node_modules | ||
| **/target | ||
| **/.pytest_cache | ||
| **/__pycache__ | ||
| **/*.test | ||
| **/tests/ |
There was a problem hiding this comment.
**/tests/ exclusion may break crates that use tests/ for shared fixtures.
Rust integration tests in a tests/ directory are compiled as separate binaries but may include fixture data or helper modules referenced from build.rs or example code. Excluding **/tests/ globally is broad; consider being explicit about what you want to exclude (e.g., coverage report outputs) rather than excluding entire test directories.
🤖 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 @.dockerignore around lines 18 - 24, The dockerignore currently excludes
"**/tests/" which is too broad and can break Rust crates or other projects that
rely on a tests/ tree for integration tests or shared fixtures; update the
.dockerignore to stop excluding the entire "**/tests/" directory and instead
exclude only the specific unwanted artifacts (e.g., coverage outputs, generated
test artifacts, or temporary files) so that real test sources and fixtures are
preserved while unwanted build/coverage files remain ignored.
| **/.cargo | ||
| **/Cargo.lock |
There was a problem hiding this comment.
**/Cargo.lock must not be excluded from the Docker build context.
Cargo.lock is what makes Rust builds reproducible. Excluding it means cargo build inside the container resolves dependencies from crates.io at build time, potentially pulling different patch versions than what passed CI. This silently breaks build reproducibility and can introduce regressions or security differences between local/CI builds and the resulting Docker image.
🐛 Proposed fix
# Rust
**/.cargo
-**/Cargo.lock📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| **/.cargo | |
| **/Cargo.lock | |
| **/.cargo |
🤖 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 @.dockerignore around lines 41 - 42, The .dockerignore currently excludes the
Cargo.lock file via the pattern "**/Cargo.lock"; remove or comment out that
pattern so Cargo.lock is included in the Docker build context (leave other
ignores like "**/.cargo" intact). Ensure the entry "**/Cargo.lock" is deleted or
disabled in .dockerignore so cargo builds inside the image use the committed
Cargo.lock for reproducible dependency resolution.
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5@11bd71901bbe5b1630ceea73d27597364c9af683 | ||
| uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Same missing version comment as cargo-deny.yml.
Same actions/checkout@v4.2.2 SHA — add # v4.2.2 here too for consistency and Dependabot legibility.
💡 Proposed improvement
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683
+ uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 | |
| uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 |
🤖 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 @.github/workflows/scorecard.yml at line 22, Add the same human-readable
version comment for the checkout action: locate the `uses:
actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683` entry and append the
`# v4.2.2` comment (matching the `actions/checkout@v4.2.2` SHA used elsewhere)
so Dependabot and reviewers can easily see the intended version.
| [[bench]] | ||
| name = "rule_evaluation" | ||
| harness = false | ||
| harness = true |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if the files exist and read the relevant sections
fd -t f "Cargo.toml" | grep -E "(focus-rules|focus-policy)"Repository: KooshaPari/FocalPoint
Length of output: 126
🏁 Script executed:
# Read the focus-rules Cargo.toml around lines 26-28
cat -n crates/focus-rules/Cargo.toml | head -35Repository: KooshaPari/FocalPoint
Length of output: 925
🏁 Script executed:
# Read the focus-policy Cargo.toml to check for comparison
cat -n crates/focus-policy/Cargo.toml | head -35Repository: KooshaPari/FocalPoint
Length of output: 768
🏁 Script executed:
# Find the rule_evaluation benchmark implementation
fd -t f "rule_evaluation" crates/focus-rules/Repository: KooshaPari/FocalPoint
Length of output: 111
🏁 Script executed:
# Read the rule_evaluation benchmark implementation
cat -n crates/focus-rules/benches/rule_evaluation.rs | head -50Repository: KooshaPari/FocalPoint
Length of output: 2133
🌐 Web query:
Criterion benchmark harness = true vs harness = false Cargo.toml
💡 Result:
In Cargo.toml, [[bench]] harness = false means “disable Cargo’s built-in (libtest) benchmark harness for this bench target, and let the benchmark crate provide its own main/harness code”. Criterion’s documentation uses harness = false for Criterion benches because Criterion supplies its own harness [1][2]. harness = true (the default) means Cargo will use the standard benchmark harness (libtest/#[bench]-style execution) for that bench target; in that case, your benches/*.rs crate is expected to follow the standard libtest benchmark harness model, not Criterion’s criterion_main!/criterion_group! model [1]. Why the distinction matters in practice (Criterion): - Criterion’s getting started guide explicitly says it’s important to disable the standard benchmark harness (i.e., harness = false) “because we'll later add our own and we don't want them to conflict” [2]. - Cargo passes --bench to the benchmark harness regardless of whether harness = false (reported behavior), so disabling the harness doesn’t change all CLI behavior, but it does change which harness is used [3]. - Common Criterion/Cargo issues arise when people override the harness and/or accidentally rely on feature flags not enabled; for example, Criterion issue #344 notes that required-features only affects compilation and you must run cargo bench --features ... so the bench is compiled [4]. (This is about required-features, but it shows how Cargo bench harness configuration affects what actually runs.) What to put in Cargo.toml for Criterion - Use: [[bench]] name = "my_benchmark" harness = false as shown in Criterion’s docs [2].
Citations:
- 1: https://doc.rust-lang.org/cargo/commands/cargo-bench.html
- 2: https://criterion-rs.github.io/book/getting_started.html
- 3: benchmark runner is passed
--bencheven whenharness = falserust-lang/cargo#3027 - 4:
[[bench]] required-features = [...]doesn't seem to work? bheisler/criterion.rs#344
Set harness = false to enable the Criterion benchmarking harness for rule_evaluation.
Criterion benchmarks require harness = false in Cargo.toml to use Criterion's own harness instead of the default libtest harness. With harness = true, Cargo invokes the standard benchmark harness, which conflicts with Criterion's criterion_main! and criterion_group! macros, breaking the benchmark.
Fix
[[bench]]
name = "rule_evaluation"
-harness = true
+harness = false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [[bench]] | |
| name = "rule_evaluation" | |
| harness = false | |
| harness = true | |
| [[bench]] | |
| name = "rule_evaluation" | |
| harness = false |
🤖 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/focus-rules/Cargo.toml` around lines 26 - 28, The Cargo.toml bench
configuration for the benchmark named "rule_evaluation" currently sets harness =
true which prevents Criterion's own harness from running; change the bench table
for the "rule_evaluation" benchmark (the [[bench]] entry with name =
"rule_evaluation") to set harness = false so Criterion's
criterion_main!/criterion_group! harness can be used.
| #[allow(dead_code)] | ||
| fn bytes_to_hex(bytes: &[u8]) -> String { | ||
| bytes.iter().map(|b| format!("{:02x}", b)).collect() | ||
| } | ||
|
|
||
| // Helper: encode bytes as base64 for testing. | ||
| #[allow(dead_code)] | ||
| fn base64_encode(bytes: &[u8]) -> String { | ||
| const CHARS: &[u8] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; | ||
| let mut result = String::new(); | ||
| for chunk in bytes.chunks(3) { | ||
| let b1 = chunk[0]; | ||
| let b2 = chunk.get(1).copied().unwrap_or(0); | ||
| let b3 = chunk.get(2).copied().unwrap_or(0); | ||
|
|
||
| let n = ((b1 as u32) << 16) | ((b2 as u32) << 8) | (b3 as u32); | ||
|
|
||
| result.push(CHARS[((n >> 18) & 0x3f) as usize] as char); | ||
| result.push(CHARS[((n >> 12) & 0x3f) as usize] as char); | ||
| if chunk.len() > 1 { | ||
| result.push(CHARS[((n >> 6) & 0x3f) as usize] as char); | ||
| } | ||
| if chunk.len() > 2 { | ||
| result.push(CHARS[(n & 0x3f) as usize] as char); | ||
| } | ||
| } | ||
| result | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether `base64` is a (non-dev) dependency of focus-templates and
# whether `bytes_to_hex`/`base64_encode` are used anywhere outside tests.
echo "=== Cargo.toml for focus-templates ==="
fd -i "Cargo.toml" --full-path crates/focus-templates --exec cat {}
echo ""
echo "=== All usages of bytes_to_hex and base64_encode ==="
rg -n "bytes_to_hex\|base64_encode" --type=rs -C2Repository: KooshaPari/FocalPoint
Length of output: 750
🏁 Script executed:
#!/bin/bash
# Search for usages of bytes_to_hex and base64_encode without type flag
echo "=== Searching for bytes_to_hex and base64_encode usages ==="
rg -n "bytes_to_hex|base64_encode" crates/focus-templates/src/ -A 2 -B 2
echo ""
echo "=== Check the actual lib.rs file around lines 396-423 ==="
sed -n '390,430p' crates/focus-templates/src/lib.rs
echo ""
echo "=== Check test module and usages around lines 598-599, 627, 629 ==="
sed -n '590,635p' crates/focus-templates/src/lib.rsRepository: KooshaPari/FocalPoint
Length of output: 5051
🏁 Script executed:
#!/bin/bash
echo "=== Root Cargo.toml workspace section ==="
head -100 Cargo.toml | grep -A 50 "\[workspace"
echo ""
echo "=== Search for base64 in any Cargo.toml ==="
fd Cargo.toml -x grep -l "base64" {}Repository: KooshaPari/FocalPoint
Length of output: 2582
Move test-only helpers inside #[cfg(test)] — #[allow(dead_code)] suppresses a legitimate lint.
bytes_to_hex and base64_encode (lines 396–423) are defined at module level but are only called from the test module (lines 598–599, 627, 629). This placement causes them to:
- Compile into release builds, inflating the binary unnecessarily.
- Pollute the module's namespace even though they have no production use.
- Require
#[allow(dead_code)]to suppress a valid clippy warning, which directly contradicts the-D warningsgate in the coding guidelines.
Move both functions inside the #[cfg(test)] mod tests { block to eliminate the attribute and signal their test-only purpose.
Secondary note on base64_encode: The function produces unpadded base64 (no = suffix). This is acceptable for the current use case (64-byte ed25519 signatures with the custom base64_decode function), but it is non-standard and will silently produce incorrect output if applied to variable-length inputs where length % 3 ≠ 0. If this helper is ever reused elsewhere, add proper padding (= characters).
🤖 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/focus-templates/src/lib.rs` around lines 396 - 423, The helper
functions bytes_to_hex and base64_encode are test-only and should be moved into
the test module: cut bytes_to_hex and base64_encode from the module root and
paste them inside the existing #[cfg(test)] mod tests { ... } block, remove the
#[allow(dead_code)] attributes, and update any test references (they already
call bytes_to_hex/base64_encode) so they resolve to the in-module helpers; keep
base64_encode behavior as-is for current tests but be aware it produces unpadded
base64 if reused later.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
| }); | ||
| } | ||
| black_box(total_matches) | ||
| }); |
There was a problem hiding this comment.
Benchmarks no longer measure actual rule evaluation
Medium Severity
All benchmark bodies were gutted to just count rule.enabled boolean flags in a loop, completely ignoring the event (note _event prefix). They no longer exercise trigger matching, condition evaluation, the rule engine, or any evaluation pipeline logic. The benchmark names (e.g., eval_batched_1000_events_50_rules) are now misleading — they measure loop iteration speed, not evaluation performance. The project's bench-guard tool would never catch real performance regressions with these benchmarks.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit ca06f9b. Configure here.
- Fix ossf/scorecard-action SHA from annotated tag object (99c09fe) to resolved commit (4eaacf0543bb3f2c246792bd56e8cdeffafb205a) - Fix github/codeql-action/upload-sarif SHA from annotated tag object (53e96ec) to resolved commit (0daab03d71ff584ef619d027a3fd9146679c5d84) - Fix Swatinem/rust-cache SHA from annotated tag object (42dc69e) to resolved commit (e18b497796c12c097a38f9edb9d0641fb99eee32) - All SHAs verified via GitHub API to point to correct tag commits Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:XL
|
CodeAnt AI Incremental review completed. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/workflows/journey-gate.yml:
- Around line 183-186: The comment echo "(non-strict run — violations do not
fail the build)" is now stale because the removed "|| true" makes
phenotype-journey assert "$manifest" failures propagate; update the workflow so
the comment matches behavior by either restoring the "|| true" guard after the
phenotype-journey assert "$manifest" command to make violations advisory, or
change/remove the echo line to reflect that phenotype-journey assert failures
will fail the step; locate the phenotype-journey assert "$manifest" invocation
and the following echo and apply the chosen fix.
In `@Cargo.toml`:
- Around line 64-66: The Cargo.toml currently declares edition = "2024" while
rust-version = "1.82", which is incompatible; update the manifest so the minimum
Rust version matches the edition—either change rust-version to "1.85" (or newer)
to keep edition = "2024", or revert edition to "2021" if you must keep
rust-version = "1.82"; edit the Cargo.toml keys edition and rust-version
accordingly.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 77092485-44b1-4d81-b0ac-e3a599d6f9a2
📒 Files selected for processing (7)
.github/workflows/cargo-audit.yml.github/workflows/cargo-deny.yml.github/workflows/ci.yml.github/workflows/journey-gate.yml.github/workflows/scorecard.yml.github/workflows/trufflehog.ymlCargo.toml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Kilo Code Review
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: KooshaPari/FocalPoint
Timestamp: 2026-05-07T15:49:09.433Z
Learning: Maintain the 56-crate Rust workspace structure under the `crates/` directory
Learnt from: CR
Repo: KooshaPari/FocalPoint
Timestamp: 2026-05-07T15:49:09.433Z
Learning: Use SQLite, PostgreSQL, or SurrealDB for data persistence depending on requirements
Learnt from: CR
Repo: KooshaPari/FocalPoint
Timestamp: 2026-05-07T15:49:09.433Z
Learning: Implement the core rules engine, connector runtime, reward/penalty ledger, audit chain, and mascot state machine in Rust
Learnt from: CR
Repo: KooshaPari/FocalPoint
Timestamp: 2026-05-07T15:49:09.433Z
Learning: Enforce portable Rust core design for iOS native enforcement in the FocalPoint connector-first screen-time management platform
🪛 GitHub Actions: CI / 0_test.txt
Cargo.toml
[error] 1-1: cargo test failed: failed to parse manifest. rust-version 1.82 is incompatible with the version (1.85.0) required by the specified edition (2024).
[error] 1-1: cargo test failed: failed to load manifest for workspace member tooling/release-cut referenced by workspace at /home/runner/work/FocalPoint/FocalPoint/Cargo.toml (manifest parse error in member Cargo.toml).
🪛 GitHub Actions: CI / test
Cargo.toml
[error] 1-1: failed to load manifest: rust-version 1.82 is incompatible with the version (1.85.0) required by the specified edition (2024).
[error] 1-1: Command failed: cargo test --all-features --workspace (exit code 101) due to workspace member manifest parse error in tooling/release-cut.
🔇 Additional comments (7)
Cargo.toml (2)
60-60: Workspace member addition is well-formed.
crates/focus-plugin-sdkis added cleanly to the workspace members list.
156-157: Pinned git rev is a good reproducibility safeguard.Using a fixed
revforphenotype-observably-macrosreduces dependency drift across workspace crates..github/workflows/cargo-deny.yml (1)
18-36: LGTM — permissions, timeout, and SHA-pinning all look correct.Workflow-level
contents: read/actions: readcorrectly restricts the default token; all three action SHAs carry human-readable version comments;timeout-minutes: 20is appropriately sized for acargo-denycheck..github/workflows/ci.yml (1)
4-19: LGTM — hardening changes are correct.Removing the
|| cargo checkclippy fallback is the right call; lint failures will now be visible rather than silently masked. Permission scope, timeout, and all three SHA pins with version labels look good..github/workflows/scorecard.yml (1)
9-35: LGTM — permission layering is correct forpublish_results: true.Job-level permissions are fully independent: "instead of receiving the default token or workflow-specified token permissions, the job will receive the specific permissions it requests." So
security-events: writeandid-token: writeremain effective on thescorecardjob even though the workflow-level block only declares read scopes — and that's exactly what satisfies theossf/scorecard-actionconstraint of no workflow-level write permissions..github/workflows/trufflehog.yml (1)
7-21: LGTM — the refactored Trufflehog step is cleaner and correctly preserves full-history checkout.
fetch-depth: 0is retained (required for scanning all commits), and--only-verified --no-updateare sensible defaults. SHA-pinned action with version label is consistent with the rest of the PR..github/workflows/cargo-audit.yml (1)
12-24: LGTM — permission hardening and SHA pinning are correct.
rustsec/audit-checkstill receivesGITHUB_TOKEN(needed to post check annotations), and the workflow-level read-only permissions don't interfere sinceaudit-checkdoesn't need write scopes. Timeout and version comment are consistent with the rest of the PR.
| else | ||
| phenotype-journey assert "$manifest" || true | ||
| phenotype-journey assert "$manifest" | ||
| echo "(non-strict run — violations do not fail the build)" | ||
| fi |
There was a problem hiding this comment.
Stale comment now contradicts behavior — remove or update it.
The || true guard was intentionally removed so that phenotype-journey assert failures propagate in non-strict mode too (confirmed by the AI summary). However, the comment on the very next line still reads:
echo "(non-strict run — violations do not fail the build)"
GitHub Actions shells run with -eo pipefail by default, so a non-zero exit from phenotype-journey assert "$manifest" will fail the step in this branch. The stale comment actively misleads anyone reading or debugging this workflow.
🛠️ Proposed fix
- phenotype-journey assert "$manifest"
- echo "(non-strict run — violations do not fail the build)"
+ phenotype-journey assert "$manifest"
+ echo "(non-strict run — assertion failures still fail the step)"Or, if the intent is truly to keep non-strict mode advisory-only, restore the guard:
- phenotype-journey assert "$manifest"
- echo "(non-strict run — violations do not fail the build)"
+ phenotype-journey assert "$manifest" || true
+ echo "(non-strict run — violations do not fail the build)"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| else | |
| phenotype-journey assert "$manifest" || true | |
| phenotype-journey assert "$manifest" | |
| echo "(non-strict run — violations do not fail the build)" | |
| fi | |
| else | |
| phenotype-journey assert "$manifest" | |
| echo "(non-strict run — assertion failures still fail the step)" | |
| fi |
| else | |
| phenotype-journey assert "$manifest" || true | |
| phenotype-journey assert "$manifest" | |
| echo "(non-strict run — violations do not fail the build)" | |
| fi | |
| else | |
| phenotype-journey assert "$manifest" || true | |
| echo "(non-strict run — violations do not fail the build)" | |
| fi |
🤖 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 @.github/workflows/journey-gate.yml around lines 183 - 186, The comment echo
"(non-strict run — violations do not fail the build)" is now stale because the
removed "|| true" makes phenotype-journey assert "$manifest" failures propagate;
update the workflow so the comment matches behavior by either restoring the "||
true" guard after the phenotype-journey assert "$manifest" command to make
violations advisory, or change/remove the echo line to reflect that
phenotype-journey assert failures will fail the step; locate the
phenotype-journey assert "$manifest" invocation and the following echo and apply
the chosen fix.
…h_different_statuses Add missing `last_edited_time` field to test fixtures to prevent `from_notion_json` from returning empty Vec. Also add proper `title` and `Completed` properties and replace `assert!(true)` with meaningful assertions. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Edition 2024 requires rust-version 1.85. Running on stable channel pulled 1.82 which is incompatible. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…) + bump MSRV to 1.85 + fix gen shadowing
Ensure consistent toolchain across all CI jobs. cargo-deny parses all workspace manifests which requires matching MSRV. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…n→icon_gen renames
tokio_tungstenite Message::text now returns Message directly instead of Result. Remove redundant Ok pattern matching. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 5 total unresolved issues (including 4 from previous reviews).
Bugbot Autofix is ON, but it could not run because on-demand usage is turned off. To enable Bugbot Autofix, turn on on-demand usage and set a spend limit in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5d4f1ee. Configure here.
| @@ -1,5 +1,5 @@ | |||
| # Phenotype-org standard clippy config | |||
| msrv = "1.75" | |||
| msrv = "1.82" | |||
There was a problem hiding this comment.
clippy MSRV not aligned with workspace rust-version
Low Severity
The PR title states it aligns clippy.toml MSRV to the workspace rust-version, but clippy.toml is set to "1.82" while the workspace rust-version in Cargo.toml was bumped to "1.85" in the same commit. They remain misaligned — clippy may flag lints for APIs available since 1.82 that are fine given the actual 1.85 minimum.
Reviewed by Cursor Bugbot for commit 5d4f1ee. Configure here.
|
CodeAnt AI is running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:XL
|
CodeAnt AI finished running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
CodeAnt AI is running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:XL
|
CodeAnt AI finished running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
CodeAnt AI is running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:XL
|
CodeAnt AI finished running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
CodeAnt AI is running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:XL
|
CodeAnt AI finished running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
User description
Summary
use serde_json::json;import inpii_scrubber.rstest module sojson!()macro compilesclippy.tomlMSRV to workspacerust-version(1.82)Test plan
cargo check -p focus-telemetry— passescargo test -p focus-telemetry --lib— 17 tests pass🤖 Generated with Claude Code
CodeAnt-AI Description
Handle more Notion page and task responses
What Changed
Impact
✅ Fewer failed Notion imports✅ Clearer page and task names✅ More reliable Notion sync results🔄 Retrigger CodeAnt AI Review
Details
💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.