fix(agents): address corrected sync review debt - #3033
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
GitHub Actions
Inactive
|
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 (1)
📝 WalkthroughWalkthroughThe changes refine acceptance validation, expand safe verification command matching, improve formatter error handling, synchronize template fingerprints, and add regression coverage. ChangesIssue validation and formatting
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Automated Status SummaryHead SHA: 18c8116
Coverage Overview
Coverage Trend
Top Coverage Hotspots (lowest coverage)
Low Coverage Files (<50.0%)
Updated automatically; will refresh on subsequent CI/Docker completions. Keepalive checklistScopeNo scope information available Tasks
Acceptance criteria
|
![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: 4f7798b614
ℹ️ 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".
| formatted = result.get('formatted_body', '') | ||
| if not formatted: | ||
| print('ERROR: No formatted body returned') | ||
| print(f"ERROR: {result.get('error') or 'No formatted body returned'}") |
There was a problem hiding this comment.
In the Phase 3 run block (checked in both the root and consumer workflow copies), the Python program is passed to python -c inside a shell double-quoted string. This added f"..." terminates that shell string; bash -n reports a syntax error near ( at result.get, so every format-phase run fails before the formatter result is inspected. Use quoting that does not terminate the outer shell argument, or move the snippet to a heredoc.
Useful? React with 👍 / 👎.
| ) | ||
| prose = re.sub(r"(?<!`)`(?!`)[^`\n]*`", "", _without_fenced_code(acceptance)) | ||
| prose = re.sub(r"(?<!`)`(?!`)[^`\n]*`", "", acceptance_prose) | ||
| prose = re.sub(r"(?<!\w)(?:[\w.-]+/)+[\w.-]+", "", prose) |
There was a problem hiding this comment.
When an acceptance criterion uses common slash-separated alternatives—for example, pytest ... passes and the response is fast/performant—this pattern treats fast/performant as a path and deletes it before checking banned adjectives. Both subjective words disappear and validate() returns an OK report, allowing precisely the wording this validator is intended to reject. Restrict the suppression to tokens that are demonstrably paths rather than every slash-separated phrase.
Useful? React with 👍 / 👎.
GitHub Actions
Inactive
![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 @.github/scripts/issue_format.py:
- Line 300: Constrain the path-removal regex used before adjective scanning in
.github/scripts/issue_format.py:300-300 to match only clear filesystem paths,
not arbitrary slash-separated prose such as fast/better, and add a regression
test covering that prose case. Apply the identical constrained filter in
templates/consumer-repo/.github/scripts/issue_format.py:300-300; this sibling
site requires the same direct change.
In `@tests/scripts/test_issue_format.py`:
- Around line 148-166: Parameterize
test_acceptance_gate_inside_fence_does_not_satisfy_validation and
test_acceptance_path_component_is_not_subjective_prose over both validator
script paths: .github/scripts/issue_format.py and
templates/consumer-repo/.github/scripts/issue_format.py. Pass each parameterized
path into the validator setup instead of relying on _validator()’s default,
while preserving the existing assertions and test inputs.
🪄 Autofix
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: b4c96cd4-41e4-4a1a-a0b6-de6590691c47
📒 Files selected for processing (9)
.github/scripts/issue_format.py.github/workflows/agents-issue-optimizer.ymlconfig/template-drift-allowlist.txtscripts/langchain/issue_formatter.pytemplates/consumer-repo/.github/scripts/issue_format.pytemplates/consumer-repo/.github/workflows/agents-issue-optimizer.ymltemplates/consumer-repo/scripts/langchain/issue_formatter.pytests/scripts/test_issue_format.pytests/scripts/test_issue_formatter.py
GitHub Actions
Inactive
GitHub Actions
Inactive
2 participants
Repairs the active source-owned review findings in the corrected Maint 68 sync wave.
pytestverify hints remain valid while bareunittestremains rejectedValidation:
python3.12 -m pytest -q tests/scripts/test_issue_format.py tests/scripts/test_issue_formatter.py tests/workflows/test_agents_issue_optimizer_format_trigger.py(132 passed); template completeness/sync; template drift (0 unallowlisted);git diff --check.This replaces the held consumer sync wave after the source merge and full Maint 68 reconciliation.
Summary by CodeRabbit
Bug Fixes
pytestandpython -m pytestcommands.Tests
pytestverification commands.Documentation