Conversation
🛡️ Watchflow Governance ChecksStatus: ❌ 2 Violations Found 🟡 Medium Severity (2)Checks PR description (body) and title for a linked issue reference (e.g. #123, Fixes #123, Closes #456). Use when the rule requires issue refs in either field.PR does not reference a linked issue (e.g. #123 or closes #123 in body/title) Validates if the PR description meets minimum length requirementsPR description is empty 💡 Reply with |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (1)**/*.py📄 CodeRabbit inference engine (.cursor/rules/guidelines.mdc)
Files:
🧠 Learnings (1)📓 Common learnings🔇 Additional comments (1)
📝 WalkthroughWalkthroughUpdated GitHub rule validation and examples to use the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/integrations/github/rules_service.py (1)
90-97:⚠️ Potential issue | 🟡 MinorGuard non-dict rule entries before using
.get(...)in the error path.If a list element is not a mapping, Line 97 can raise
AttributeErrorwhile formatting the validation message, which downgrades the response to a generic unexpected-error path.Proposed fix
for i, rule_data in enumerate(rules_data["rules"]): + if not isinstance(rule_data, dict): + return { + "success": False, + "message": ( + f"❌ **Rule #{i + 1} failed validation**\n\n" + "Error: `Each rule must be a YAML mapping/object.`\n\n" + "Please check your rule definition and fix the error above.\n\n" + f"[See rule schema docs.]({DOCS_URL})" + ), + } try: Rule.model_validate(rule_data) except Exception as e: return { "success": False, "message": ( f"❌ **Rule #{i + 1} (`{rule_data.get('description', 'N/A')}`) failed validation**\n\n"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/integrations/github/rules_service.py` around lines 90 - 97, In the loop that calls Rule.model_validate over rules_data["rules"] (the enumerate with variable rule_data and the Rule.model_validate call), guard against non-dict entries before using rule_data.get in the error message: when catching the exception, compute a safe description (e.g., if isinstance(rule_data, dict) use rule_data.get("description","N/A") else use a string fallback like repr(rule_data) or "invalid entry") and use that safe value in the formatted message so AttributeError is avoided for non-mapping list elements.
🧹 Nitpick comments (1)
src/rules/loaders/github_loader.py (1)
64-66: Use structured log fields for disabled-rule skips.At Line 65, the new f-string log is unstructured; this makes filtering/analytics harder versus fielded logs.
Proposed refactor
- if str(rule_data.get("enabled", True)).lower() == "false": - logger.info(f"Skipping disabled rule: {rule_data.get('description', 'unknown')}") + if str(rule_data.get("enabled", True)).strip().lower() == "false": + logger.info( + "Skipping disabled rule", + extra={ + "operation": "load_rules", + "subject_ids": { + "repository": repository, + "rule_description": rule_data.get("description", "unknown"), + }, + "decision": "skip_disabled_rule", + "latency_ms": None, + }, + ) continueAs per coding guidelines, "Use structured logging at boundaries with fields:
operation,subject_ids,decision,latency_ms."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rules/loaders/github_loader.py` around lines 64 - 66, The log call that skips disabled rules uses an unstructured f-string (logger.info(f"Skipping disabled rule: {rule_data.get('description', 'unknown')}")); change it to a structured log call using fields so filters can query easily—call logger.info with a message like "Skipping disabled rule" and pass structured keyword fields including operation (e.g., "rule_skip"), subject_ids (the rule id or description from rule_data), decision ("disabled"), and latency_ms (if available or None); update the logger invocation that references rule_data and ensure you populate subject_ids from rule_data.get("id") or description and include decision="disabled".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/integrations/github/rules_service.py`:
- Around line 90-97: In the loop that calls Rule.model_validate over
rules_data["rules"] (the enumerate with variable rule_data and the
Rule.model_validate call), guard against non-dict entries before using
rule_data.get in the error message: when catching the exception, compute a safe
description (e.g., if isinstance(rule_data, dict) use
rule_data.get("description","N/A") else use a string fallback like
repr(rule_data) or "invalid entry") and use that safe value in the formatted
message so AttributeError is avoided for non-mapping list elements.
---
Nitpick comments:
In `@src/rules/loaders/github_loader.py`:
- Around line 64-66: The log call that skips disabled rules uses an unstructured
f-string (logger.info(f"Skipping disabled rule: {rule_data.get('description',
'unknown')}")); change it to a structured log call using fields so filters can
query easily—call logger.info with a message like "Skipping disabled rule" and
pass structured keyword fields including operation (e.g., "rule_skip"),
subject_ids (the rule id or description from rule_data), decision ("disabled"),
and latency_ms (if available or None); update the logger invocation that
references rule_data and ensure you populate subject_ids from
rule_data.get("id") or description and include decision="disabled".
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/integrations/github/rules_service.pysrc/rules/loaders/github_loader.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/guidelines.mdc)
**/*.py: Use modern typing only:dict[str, Any],list[str],str | None(noDict,List,Optional)
GitHub/HTTP/DB calls must beasync def; avoid blocking calls (time.sleep, sync HTTP) in async paths
All agent outputs and external payloads must use validatedBaseModelfrom Pydantic
Use dataclasses for internal immutable state where appropriate
Use structured logging at boundaries with fields:operation,subject_ids,decision,latency_ms
Implement Agent pattern: single-responsibility agents with typed inputs/outputs
Use Decorator pattern for retries, metrics, caching as cross-cutting concerns
Agent outputs must include:decision,confidence (0..1), shortreasoning,recommendations,strategy_used
Implement confidence policy: reject or route to human-in-the-loop when confidence < 0.5
Use minimal, step-driven prompts; provide Chain-of-Thought only for complexity > 0.7 or ambiguity > 0.6
Strip secrets/PII from agent prompts; scope tools; keep raw reasoning out of logs (store summaries only)
Cache idempotent lookups; lazy-import heavy dependencies; bound fan-out withasyncio.Semaphore
Avoid redundant LLM calls; memoize per event when safe
Use domain errors (e.g.,AgentError) witherror_type,message,context,timestamp,retry_count
Use exponential backoff for transient failures; circuit-break noisy integrations when needed
Fail closed for risky decisions; provide actionable remediation in error paths
Validate all external inputs; verify webhook signatures
Implement prompt-injection hardening; sanitize repository content passed to LLMs
Performance targets: Static validation ~<100ms typical, hybrid decisions sub-second when cache warm, budget LLM paths thoughtfully
Reject old typing syntax (Dict,List,Optional) in code review
Reject blocking calls in async code; reject bareexcept:clauses; reject swallowed errors
Reject LLM calls for trivial/deterministic checks
Reject unvalidated agent outputs and missing confidenc...
Files:
src/integrations/github/rules_service.pysrc/rules/loaders/github_loader.py
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: warestack/watchflow PR: 0
File: .cursor/rules/guidelines.mdc:0-0
Timestamp: 2026-01-31T19:35:22.504Z
Learning: Applies to **/*.yaml : Use YAML for rule configuration with natural-language descriptions; avoid hard-coded if/else in rule files
📚 Learning: 2026-01-31T19:35:22.504Z
Learnt from: CR
Repo: warestack/watchflow PR: 0
File: .cursor/rules/guidelines.mdc:0-0
Timestamp: 2026-01-31T19:35:22.504Z
Learning: Applies to **/*.yaml : Use YAML for rule configuration with natural-language descriptions; avoid hard-coded if/else in rule files
Applied to files:
src/integrations/github/rules_service.py
📚 Learning: 2026-01-31T19:35:22.504Z
Learnt from: CR
Repo: warestack/watchflow PR: 0
File: .cursor/rules/guidelines.mdc:0-0
Timestamp: 2026-01-31T19:35:22.504Z
Learning: Applies to docs/** : Provide runnable examples for new/changed rules; keep cross-links current
Applied to files:
src/integrations/github/rules_service.py
🔇 Additional comments (1)
src/integrations/github/rules_service.py (1)
41-41: Nice alignment to description-first rule examples.These YAML snippets now consistently reflect
description-based rules, which matches the current validation messaging direction.Also applies to: 67-67, 77-77
🛡️ Watchflow Governance ChecksStatus: ❌ 1 Violations Found 🟡 Medium Severity (1)Checks PR description (body) and title for a linked issue reference (e.g. #123, Fixes #123, Closes #456). Use when the rule requires issue refs in either field.PR does not reference a linked issue (e.g. #123 or closes #123 in body/title) 💡 Reply with |
|
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (50.0%) is below the target coverage (80.0%). You can increase the patch coverage or adjust the target coverage. @@ Coverage Diff @@
## main #58 +/- ##
=======================================
- Coverage 68.8% 68.8% -0.1%
=======================================
Files 154 154
Lines 9725 9730 +5
=======================================
+ Hits 6693 6695 +2
- Misses 3032 3035 +3 Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
2 participants
Summary by CodeRabbit
New Features
enabled: falseflag; disabled rules are skipped and logged.Bug Fixes
Documentation
Style