feat(automation): suggest starter rules#22
Conversation
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
![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: 47b8e61650
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent 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). (1)
🧰 Additional context used📓 Path-based instructions (1)src/**/*.rs📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔍 Remote MCP GitHub CopilotPerfect! Now I have comprehensive context. Let me compile the additional context findings for you. Summary of Additional Relevant ContextBased on my investigation using GitHub Copilot, gh_grep, and Firecrawl across multiple sources, here are the key facts and concrete information that would help in reviewing PR PR Scope & Safety DesignThis PR adds a new read-only CLI command
Technology Stack ContextThe project uses:
Deterministic Rule ID Generation StrategyThe implementation uses:
Suggestion Eligibility CriteriaThreads must meet ALL criteria:
Tests explicitly validate filtering of new threads, non-INBOX labels, and groups below minimum threshold. Request Validation & Error HandlingFour parameters are validated with specific errors:
All map to Sample Retention StrategyUses bounded sample collection keyed by (internal_date_epoch_ms, thread_id, message_id) tuples. When the sample buffer reaches
Output Rendering (Dual Mode)
Related PR DependenciesPR Documentation UpdatesDocs were updated to clarify that Test Coverage StatusComprehensive unit tests in
End-to-end CLI tests in 🔇 Additional comments (7)
WalkthroughThis PR adds an "automation rules suggest" command that generates disabled starter automation rules from local cached thread candidates. It introduces request/report types and defaults, precedence/header helpers, a deterministic grouping/ranking pipeline that renders single-rule TOML, a suggest_rules service entry (with validation and error variants), CLI subcommand and handler wiring, JSON/plain output rendering, tests for validation/CLI contracts, and documentation updates in README and runbooks. Possibly related PRs
🚥 Pre-merge checks | ✅ 8 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
Pull request overview
Adds a new read-only CLI workflow (automation rules suggest) that generates disabled “starter” automation rules based on recurring patterns found in the local mailbox cache, plus supporting output, error classification, tests, and documentation updates.
Changes:
- Introduces
automation rules suggestCLI subcommand wired through handlers and command metadata. - Implements suggestion generation (grouping, ranking, TOML snippet rendering) and a new JSON/plain-text report surface.
- Extends CLI error classification + adds tests and docs covering the new workflow.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/lib.rs | Adds operation metadata mapping for automation.rules.suggest. |
| src/handlers/automation.rs | Wires the new CLI subcommand to automation::suggest_rules(...). |
| src/cli.rs | Adds the automation rules suggest Clap subcommand and flags/defaults. |
| src/cli_output/tests.rs | Adds error mapping + end-to-end CLI validation tests for suggest. |
| src/cli_output/errors.rs | Classifies new suggestion validation errors (and TOML serialization) into stable error buckets. |
| src/automation/suggestions.rs | Implements candidate grouping, suggestion ranking, rule/TOML snippet generation, and unit tests. |
| src/automation/service.rs | Adds suggest_rules service entrypoint + request validation + new error variants. |
| src/automation/output.rs | Adds plain-text + JSON output routing for AutomationRulesSuggestReport. |
| src/automation/model.rs | Introduces request/report/suggestion model types + default constants for suggest. |
| src/automation/mod.rs | Exposes the new suggest API + constants from the automation module. |
| README.md | Documents the new automation rules suggest command in the quick command list and roadmap text. |
| docs/roadmap/v1-search-triage-draft-queue.md | Notes the new “starter rule suggestions” capability in roadmap material. |
| docs/operations/verification-and-hardening.md | Adds automation rules suggest to the operational runbook. |
| docs/operations/automation-rules-and-bulk-actions.md | Documents usage, constraints, and workflow positioning of rule suggestions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@docs/operations/verification-and-hardening.md`:
- Line 162: The Commands recap omits the new subcommand "automation rules
suggest" causing inconsistency; update the recap block in
verification-and-hardening.md to include the exact command invocation "cargo run
-- automation rules suggest --json" in the list between "cargo run -- sync run
--profile deep-audit --json" and "cargo run -- automation rules validate --json"
so the docs reflect the current binary surface and Phase 5 commands.
In `@src/automation/service.rs`:
- Around line 680-696: The code validates four suggestion parameters in
validate_suggest_request but only the InvalidSuggestionLimit case has a CLI
contract test; add three new contract tests mirroring
automation_rules_suggest_zero_limit_fails_in_json_and_human_modes() to cover
InvalidSuggestionMinThreadCount (pass --min-thread-count 0),
InvalidSuggestionOlderThanDays (pass --older-than-days 0), and
InvalidSuggestionSampleLimit (pass --sample-limit 0). Implement each test in
src/cli_output/tests.rs following the existing pattern: run the CLI in both
human and JSON modes, assert non-zero exit code, and for JSON assert the error
JSON shape contains error.kind matching the corresponding AutomationServiceError
variant (InvalidSuggestionMinThreadCount / InvalidSuggestionOlderThanDays /
InvalidSuggestionSampleLimit). Ensure test names clearly reference the flag and
failure (e.g.,
automation_rules_suggest_zero_min_thread_count_fails_in_json_and_human_modes)
and re-use the same assertions for exit code and JSON structure as the
zero-limit test.
🪄 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: c326a292-aaf4-472d-a3f3-b818262ba5ec
📒 Files selected for processing (14)
README.mddocs/operations/automation-rules-and-bulk-actions.mddocs/operations/verification-and-hardening.mddocs/roadmap/v1-search-triage-draft-queue.mdsrc/automation/mod.rssrc/automation/model.rssrc/automation/output.rssrc/automation/service.rssrc/automation/suggestions.rssrc/cli.rssrc/cli_output/errors.rssrc/cli_output/tests.rssrc/handlers/automation.rssrc/lib.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,ts,tsx,md,txt,json}
📄 CodeRabbit inference engine (Custom checks)
Reject Unicode EM DASH (code point U+2014) in comments and string literals. Use
--instead. Hard FAIL if any newly introduced content includes U+2014.
Files:
docs/roadmap/v1-search-triage-draft-queue.mddocs/operations/verification-and-hardening.mdREADME.mddocs/operations/automation-rules-and-bulk-actions.md
docs/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep docs concrete and aligned with the current binary surface
Files:
docs/roadmap/v1-search-triage-draft-queue.mddocs/operations/verification-and-hardening.mddocs/operations/automation-rules-and-bulk-actions.md
docs/{operations,workflows}/**
📄 CodeRabbit inference engine (AGENTS.md)
docs/{operations,workflows}/**: Put operator procedures indocs/operations/ordocs/workflows/
Update operator-facing docs in the same change whenever CLI error contracts, JSON envelopes, or exit-code behavior change
Files:
docs/operations/verification-and-hardening.mddocs/operations/automation-rules-and-bulk-actions.md
src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.rs: Useanyhowfor command dispatch and top-level context insrc/lib.rs; prefer typedthiserrorerrors in Gmail, workflow, and store layers in Rust
Keep error enums local to the layer that owns the failure semantics; do not introduce a repo-wide catch-all error enum unless it clearly reduces total code and cognitive load
For new CLI JSON contracts, normalize success and failure to one top-level shape:{ "success": true, "data": ... }for success and{ "success": false, "error": { "code": ..., "message": ..., "kind": ..., "operation": ..., "causes": [...] } }for failure
Keeperror.codestable and operator-oriented, keeperror.kindfor deeper subsystem detail, and keeperror.causesto an ordered message chain only in JSON error contracts
Do not include debug or backtrace payloads in the JSON error contract; use stderr and Rust backtrace env vars for deep diagnostics instead
Preserve existing human-facing error text unless it is misleading, ambiguous, or missing required operator action
When adding or changing CLI failures, keep exit codes in a small stable bucket set rather than creating one-off codes per variant
Keep blocking SQLite and filesystem work behindtokio::task::spawn_blocking; do not treat runningspawn_blockingwork as abortable
Add focused error-path tests for every new failure class; if CLI JSON or exit-code behavior changes, add contract tests for the new output and exit mapping in the same pass
Files:
src/cli.rssrc/lib.rssrc/handlers/automation.rssrc/cli_output/errors.rssrc/cli_output/tests.rssrc/automation/output.rssrc/automation/model.rssrc/automation/service.rssrc/automation/mod.rssrc/automation/suggestions.rs
🪛 LanguageTool
docs/operations/automation-rules-and-bulk-actions.md
[style] ~37-~37: Consider using a more formal/concise alternative here.
Context: ...utomation.toml ``` Automation commands other than automation rules suggest require: - ...
(OTHER_THAN)
🔍 Remote MCP Firecrawl, GitHub Copilot
Perfect! Now I have comprehensive context from all sources. Let me compile my findings into a structured summary.
Summary of Additional Context Findings
Based on my investigation using Firecrawl, GitHub Copilot, and gh_grep, here are the relevant facts and context for reviewing this PR:
Automation Subsystem Foundation
PR #11 established the foundational automation subsystem with rule-file driven planning, SQLite persistence for runs/candidates/events, and preview/apply/validate CLI surfaces. This PR extends that foundation by adding rule suggestion capabilities based on mailbox evidence.
Technology Stack and Dependencies
The repository uses:
- Rust 1.89 (MSRV specified in Cargo.toml)
- clap 4.5.39 with derive feature for CLI parsing
- serde 1.0.228 with derive feature for serialization
- toml 0.9.8 for TOML file handling
TOML Serialization Best Practices
Serializing Rust structs into TOML format uses the serde crate alongside the toml library by deriving Serialize and Deserialize traits, then leveraging functions like toml::to_string_pretty to convert struct instances into TOML-formatted strings. The implementation correctly follows this pattern with #[derive(Serialize, Deserialize)] on AutomationRuleSet and uses toml::to_string_pretty() for formatting.
Deterministic Rule ID Generation
The PR implements deterministic rule IDs through:
- Format:
{prefix}-{slugified_value}(e.g.,archive-list-digest-example-com) - Prefixes:
archive-listfor List-ID rules (priority 200),archive-senderfor sender rules (priority 150) - Slug normalization: Converts to lowercase, replaces special characters (
.,_,-,@) with hyphens, limits to 48 characters, trims trailing dashes, defaults to "unknown" if empty - Sample test cases: Validates
<Digest.Example_Com>→digest-example-comand!!!→unknown
This deterministic approach enables consistent rule generation across runs (tests verify this with limits_suggestions_and_samples_deterministically).
Suggestion Eligibility Criteria
Rules are suggested only for threads that:
- Have INBOX label (case-insensitive)
- Are older than configured days (default: 14 days, configurable via
--older-than-days) - Have either:
- List-Unsubscribe header with List-ID header (high confidence), OR
- From address with bulk/list/junk Precedence header (medium confidence)
Non-eligible threads are explicitly ignored with test coverage for new threads and non-INBOX labels.
Error Handling & Validation
Suggestion validation enforces:
limit > 0(InvalidSuggestionLimit)min_thread_count > 0(InvalidSuggestionMinThreadCount)older_than_days > 0(InvalidSuggestionOlderThanDays)sample_limit > 0(InvalidSuggestionSampleLimit)
All validation errors map to ErrorCode::ValidationFailed with kind: "automation.validation". End-to-end test verifies both JSON and human-readable error output for --limit 0.
CLI Integration Pattern
clap with derive macros allows declarative command-line parser definition using struct fields with attributes for validation, default values, and help text. The PR correctly follows this pattern with:
- Default values sourced from module constants
- Help text describing parameters
- Both
--jsonand human output modes
Output Rendering Strategy
The suggestion report supports dual output modes:
- JSON: Uses
crate::cli_output::write_json_success()for serialized output - Plain text: TSV format for suggestions with sanitized fields, followed by TOML snippets, warnings, next steps, and command plan
Related PR Context
PR #11 modified files in the automation module including model.rs, service.rs, output.rs, mod.rs, and cli.rs for the core automation feature. This PR modifies the same files while being backward-compatible and additive-only.
Test Coverage
The PR includes comprehensive unit tests validating:
- List-ID vs. sender rule suggestions
- Filtering logic (ignores new/non-INBOX threads, groups below minimum threshold)
- Deterministic limiting and sampling
- Slug normalization edge cases
- TOML roundtrip validity
- End-to-end CLI integration with error handling
🔇 Additional comments (13)
src/lib.rs (1)
222-227: Suggest command metadata mapping looks correct.The new metadata branch cleanly propagates
jsonand the expected operation id for error/report envelopes.docs/roadmap/v1-search-triage-draft-queue.md (1)
64-64: Roadmap update is clear and aligned with the new command behavior.src/handlers/automation.rs (1)
18-37: Suggest command dispatch wiring is solid.The request construction and output path are consistent with the rest of the automation handlers.
README.md (1)
76-76: README updates are aligned with the new suggest workflow and command surface.Also applies to: 223-223
src/cli.rs (1)
356-373: The newautomation rules suggestCLI shape looks good.Flags/defaults are well-scoped and consistent with existing command design.
src/cli_output/errors.rs (1)
121-124: Error classification additions are correctly bucketed.The new suggestion validation variants and TOML serialization mapping preserve the established error contract behavior.
Also applies to: 141-143
src/cli_output/tests.rs (1)
182-196: Great coverage for the new suggest validation failure path.These tests appropriately lock JSON/human output behavior and exit-code mapping for the new command.
Also applies to: 198-269
src/automation/output.rs (1)
55-122: LGTM –AutomationRulesSuggestReportoutput rendering is correct.TSV column order matches the header,
sanitize()covers tab/control characters in all interpolated values, JSON path correctly delegates towrite_json_success, and thewrite/docs/operations/automation-rules-and-bulk-actions.md (1)
37-68: LGTM – prerequisite differentiation and example commands are accurate.The distinction that
automation rules suggestdoes not require an existing rules file is correctly stated and consistent with thesuggest_rulesimplementation inservice.rs. The example flag invocations match the CLI defaults exported frommodel.rs.src/automation/mod.rs (1)
5-17: LGTM – additive module surface expansion is clean.src/automation/suggestions.rs (2)
73-140: LGTM – suggestion pipeline is correct and deterministic.BTreeMap keying, the two-level sort (rank descending, then rule_id ascending as tiebreak), and
sample_limit-bounded sample collection all produce stable output across runs. The testlimits_suggestions_and_samples_deterministicallyexplicitly validates this contract.
340-372: LGTM –slugifylength bound is correct.The break fires after pushing (so the pre-trim slug is at most
RULE_ID_SOURCE_LIMIT = 48characters), andtrim_matches('-')handles any trailing dash left by the break. The two test vectors ("<Digest.Example_Com>"→"digest-example-com","!!!"→"unknown") validate the main paths.src/automation/model.rs (1)
33-104: LGTM – new request/response types are consistent with existing model conventions.
AutomationRulesSuggestRequestcorrectly omitsSerialize/Deserialize(input-only), all report types deriveSerialize, andDEFAULT_AUTOMATION_SUGGESTION_OLDER_THAN_DAYSis typedu32to match the field it backs.
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 `@src/automation/suggestions.rs`:
- Around line 184-186: The current push-then-truncate behavior
(group.samples.push(sample) while group.samples.len() < sample_limit) makes
selection order-dependent; change this to a bounded-memory selection that always
retains the most-recent samples regardless of input order — e.g., maintain a
min-heap (or sorted buffer) of at most request.sample_limit keyed by each
sample's timestamp and on each new sample compare/replace the min entry, or
implement reservoir sampling if true uniform sampling is required; update the
logic around group.samples, sample_limit and sample so that the later ranking
code (the block that ranks by latest sampled timestamp at the code referenced
around lines 323–333) operates on a correctly chosen representative set.
- Around line 369-375: In normalized_precedence, matching uses substring
contains which wrongly accepts values like "notbulk"; change to token-based
matching instead: after trimming and to_ascii_lowercase, split the header into
tokens on non-alphanumeric characters (e.g. header.split(|c: char|
!c.is_alphanumeric()).filter(|t| !t.is_empty())) and check if any token exactly
equals one of the expected values ["bulk","list","junk"]; return that exact
token as before. This keeps the function name normalized_precedence and its
input handling but replaces contains with exact-token equality to avoid false
positives.
🪄 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: 8223448f-060e-4c2e-b23a-bd226c51c984
📒 Files selected for processing (1)
src/automation/suggestions.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.rs: Useanyhowfor command dispatch and top-level context insrc/lib.rs; prefer typedthiserrorerrors in Gmail, workflow, and store layers in Rust
Keep error enums local to the layer that owns the failure semantics; do not introduce a repo-wide catch-all error enum unless it clearly reduces total code and cognitive load
For new CLI JSON contracts, normalize success and failure to one top-level shape:{ "success": true, "data": ... }for success and{ "success": false, "error": { "code": ..., "message": ..., "kind": ..., "operation": ..., "causes": [...] } }for failure
Keeperror.codestable and operator-oriented, keeperror.kindfor deeper subsystem detail, and keeperror.causesto an ordered message chain only in JSON error contracts
Do not include debug or backtrace payloads in the JSON error contract; use stderr and Rust backtrace env vars for deep diagnostics instead
Preserve existing human-facing error text unless it is misleading, ambiguous, or missing required operator action
When adding or changing CLI failures, keep exit codes in a small stable bucket set rather than creating one-off codes per variant
Keep blocking SQLite and filesystem work behindtokio::task::spawn_blocking; do not treat runningspawn_blockingwork as abortable
Add focused error-path tests for every new failure class; if CLI JSON or exit-code behavior changes, add contract tests for the new output and exit mapping in the same pass
Files:
src/automation/suggestions.rs
🔍 Remote MCP Firecrawl
Additional context gathered (concise, review-focused)
-
PR page (full conversation & bot reviews) confirms the feature: adds read-only CLI
automation rules suggestthat generates disabled starter rules from recurring local mailbox evidence; includes docs, models, service, output, CLI wiring, and tests. Safety constraints: suggestions limited to older INBOX list/bulk patterns; generated rules are disabled and archive-only; no Gmail mutations nor writes to.mailroom/automation.toml. -
Verification steps listed by the author (formatting/lint/test checks and some runtime smoke commands) are present on the PR description.
-
Automated review bots (Copilot / CodeRabbit / coderabbitai / qodo-code-review) ran and produced:
- Positive automated coverage notes and file-by-file review highlights (models, service, suggestions pipeline, output rendering, CLI wiring all noted as additive and plausible).
- CodeRabbit flagged a doc inconsistency: recommendation to add
cargo run -- automation rules suggest --jsoninto the "Commands recap" in docs/operations/verification-and-hardening.md. - coderabbitai requested changes: specifically, add three missing CLI contract tests covering validation errors for --min-thread-count 0, --older-than-days 0, and --sample-limit 0 (only --limit 0 currently has a contract test). The bot included precise test naming and assertion guidance mirroring the existing zero-limit test in src/cli_output/tests.rs.
-
Files and areas the automated reviewers exercised/flagged for reviewers:
- src/automation/suggestions.rs — deterministic grouping, slug rules, sampling, ranking; tests present and flagged as thorough.
- src/automation/service.rs — validate_suggest_request enforces four validation errors; reviewers note only one of those currently has an end-to-end CLI contract test.
- docs/operations/verification-and-hardening.md and docs/operations/automation-rules-and-bulk-actions.md — updated to include the suggest flow; reviewer recommends minor consistency patch to the commands recap.
Sources used
- PR page and all review bot comments (scraped): #22 — Firecrawl scrape
🔇 Additional comments (1)
src/automation/suggestions.rs (1)
295-314: Rule-id collision handling looks solid.The suffixing logic plus TOML re-render on mutated IDs is correct and keeps serialized output consistent with
suggestion.rule.id.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf7ab2c7b8
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8fd8f650ed
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Summary
automation rules suggestto generate disabled starter rules from recurring local mailbox evidence.mailroom/automation.tomlwritesSafety
Verification
rtk test cargo fmt --checkrtk err cargo clippy --all-targets --all-features -- -D warningsrtk test cargo testrtk test cargo run -- paths --jsonrtk test cargo run -- doctor --jsonrtk git diff --check