refactor: migrate conventional-title config from string to dict#1028
Merged
refactor: migrate conventional-title config from string to dict#1028
Conversation
Change ai-features.conventional-title from string enum (true/false/fix)
to dict with enabled, mode (suggest/fix), and timeout-minutes.
Config:
ai-features:
conventional-title:
enabled: true
mode: suggest # suggest | fix
timeout-minutes: 10
Signed-off-by: Meni Yakove <myakove@gmail.com>
Collaborator
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Container Operations
Cherry-pick Operations
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR migrates the Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub (PR)
participant Runner as Runner Handler
participant WT as Worktree (checkout)
participant AI as AI CLI
participant GHAPI as GitHub API
GH->>Runner: New/updated PR triggers check
Runner->>WT: checkout worktree for PR (cwd)
Runner->>AI: run AI suggestion (cwd=WT, timeout=timeout-minutes, prompt)
AI-->>Runner: suggested_title or error
alt mode == "fix" and valid
Runner->>GHAPI: update PR title (apply auto-fix)
GHAPI-->>Runner: success
else mode == "suggest" and suggested
Runner-->>GH: add suggestion message to check output
end
Runner-->>GH: post check result (pass/fail with AI messages)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan Review Summary by QodoMigrate conventional-title config from string to dict format
WalkthroughsDescription• Migrate conventional-title config from string enum to dict format • Add enabled, mode (suggest/fix), and timeout-minutes properties • Update config schema, handler logic, and all example configurations • Pass timeout parameter to AI CLI for conventional title suggestions Diagramflowchart LR
A["String enum config<br/>true/false/fix"] -->|"Refactor"| B["Dict config<br/>enabled/mode/timeout"]
B -->|"Update schema"| C["schema.yaml"]
B -->|"Update handler"| D["runner_handler.py"]
B -->|"Update tests"| E["test_runner_handler.py"]
B -->|"Update examples"| F["config.yaml files"]
B -->|"Update docs"| G["README.md"]
File Changes1. webhook_server/config/schema.yaml
|
Code Review by Qodo
1. Schema drops legacy string values
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
webhook_server/tests/test_runner_handler.py (1)
703-740:⚠️ Potential issue | 🟡 MinorLOW: Lock the new
timeout_minutesbehavior in this test.This is the only updated test that inspects
call_ai_clikwargs, but it only checkscwd. If the newtimeout_minutesargument gets dropped or renamed, the suite still passes. Please assert the default10here and add one non-default case.As per coding guidelines, `webhook_server/tests/*.py`: Test files must achieve 90% code coverage - new code without tests fails CI.Minimal assertion for the default path
mock_ai_cli.assert_awaited_once() call_kwargs = mock_ai_cli.call_args[1] assert call_kwargs["cwd"] == "/tmp/test-clone" + assert call_kwargs["timeout_minutes"] == 10🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webhook_server/tests/test_runner_handler.py` around lines 703 - 740, The test test_conventional_title_failure_with_ai_suggestion currently only asserts the cwd passed to the mocked call_ai_cli; add an assertion that call_ai_cli received timeout_minutes==10 (the default) by checking mock_ai_cli.call_args[1]["timeout_minutes"] == 10, and add a second small test (or extend this test) that configures runner_handler.github_webhook.ai_features to include a non-default timeout (e.g., "timeout_minutes": 5) and asserts that mock_ai_cli was awaited with timeout_minutes==5; reference the existing mock object for call_ai_cli and the run_conventional_title_check invocation to locate where to add these assertions.webhook_server/libs/handlers/runner_handler.py (1)
577-591:⚠️ Potential issue | 🟠 MajorHIGH: Legacy string configs are silently disabled here.
Even if the schema stays backward-compatible, this helper returns
Nonefor any non-dict value, so old"true"and"fix"settings never reach suggest/fix mode. Please translate the legacy strings here or normalize them before this method runs, otherwise upgrades change behavior without warning.Compatibility shim example
ct_config = ai_config.get("conventional-title") + if isinstance(ct_config, str): + return {"true": "suggest", "fix": "fix", "false": None}.get(ct_config) if not isinstance(ct_config, dict) or not ct_config.get("enabled"): return None - return ct_config.get("mode", "suggest") + return ct_config.get("mode") or "suggest"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webhook_server/libs/handlers/runner_handler.py` around lines 577 - 591, The helper _get_ai_conventional_title_mode currently returns None for non-dict ct_config, silently dropping legacy string/bool configs; update it to accept and normalize legacy values from self.github_webhook.ai_features: if ct_config is a bool True or string "true"/"suggest"/"fix", treat them as {"enabled": True, "mode": "suggest" or "fix"} (map plain "fix" to mode "fix", any other truthy string to "suggest"); keep existing dict handling for modern config, and only return None when enabled is false or missing. Locate ct_config handling in _get_ai_conventional_title_mode and perform this normalization before the enabled/mode checks so legacy settings continue to work.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@webhook_server/config/schema.yaml`:
- Around line 21-47: The ai-features.conventional-title node must preserve
legacy string values; update the schema or add a pre-validation migration:
either change the conventional-title definition to accept a union (oneOf) of the
legacy string enum ("true","false","fix") and the new object shape (with
properties enabled, mode, timeout-minutes, default values and
additionalProperties:false), or implement a migration step that rewrites
incoming config values by mapping "true" → {enabled: true, mode: suggest},
"false" → {enabled: false}, and "fix" → {enabled: true, mode: fix} before schema
validation, and document this mapping in the config docs; reference the
ai-features.conventional-title schema and the properties enabled, mode,
timeout-minutes when making the change.
---
Outside diff comments:
In `@webhook_server/libs/handlers/runner_handler.py`:
- Around line 577-591: The helper _get_ai_conventional_title_mode currently
returns None for non-dict ct_config, silently dropping legacy string/bool
configs; update it to accept and normalize legacy values from
self.github_webhook.ai_features: if ct_config is a bool True or string
"true"/"suggest"/"fix", treat them as {"enabled": True, "mode": "suggest" or
"fix"} (map plain "fix" to mode "fix", any other truthy string to "suggest");
keep existing dict handling for modern config, and only return None when enabled
is false or missing. Locate ct_config handling in
_get_ai_conventional_title_mode and perform this normalization before the
enabled/mode checks so legacy settings continue to work.
In `@webhook_server/tests/test_runner_handler.py`:
- Around line 703-740: The test
test_conventional_title_failure_with_ai_suggestion currently only asserts the
cwd passed to the mocked call_ai_cli; add an assertion that call_ai_cli received
timeout_minutes==10 (the default) by checking
mock_ai_cli.call_args[1]["timeout_minutes"] == 10, and add a second small test
(or extend this test) that configures runner_handler.github_webhook.ai_features
to include a non-default timeout (e.g., "timeout_minutes": 5) and asserts that
mock_ai_cli was awaited with timeout_minutes==5; reference the existing mock
object for call_ai_cli and the run_conventional_title_check invocation to locate
where to add these assertions.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 99b27c18-dab4-4e1b-b2d3-eb3f02125c89
📒 Files selected for processing (6)
README.mdexamples/.github-webhook-server.yamlexamples/config.yamlwebhook_server/config/schema.yamlwebhook_server/libs/handlers/runner_handler.pywebhook_server/tests/test_runner_handler.py
- Validate conventional-title mode against (suggest, fix) with fallback - Set check to success after valid AI title auto-fix - Run AI title suggestion in PR worktree for correct context - Assert timeout_minutes in test Signed-off-by: Meni Yakove <myakove@gmail.com>
myakove-bot
added
lgtm-coderabbitai[bot]
and removed
changes-requested-coderabbitai[bot]
labels
Collaborator
Author
|
/retest all |
Collaborator
|
New container for ghcr.io/myk-org/github-webhook-server:latest published |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrate
ai-features.conventional-titlefrom string enum ("true"/"false"/"fix") to dict format withenabled,mode, andtimeout-minutes. Consistent withresolve-cherry-pick-conflicts-with-aidict pattern.Closes #1027
Changes
Old format
New format
Files changed
webhook_server/config/schema.yamlconventional-titlefrom string enum to objectwebhook_server/libs/handlers/runner_handler.pywebhook_server/tests/test_runner_handler.pyexamples/config.yamlexamples/.github-webhook-server.yamlREADME.mdTests
All 1353 tests pass, 90.10% coverage.
Summary by CodeRabbit
Documentation
enabled,mode(suggest/fix), andtimeout-minutes; docs and examples updated.New Features