Skip to content

fix(codecommit): apply ignore rules and read .pr_agent.toml from the destination commit - #3576

Merged
IsmaelMartinez merged 1 commit into
The-PR-Agent:mainfrom
PeterDaveHello:fix/codecommit-ignore-rules
Sep 20, 2026
Merged

IsmaelMartinez merged 1 commit into
The-PR-Agent:mainfrom
PeterDaveHello:fix/codecommit-ignore-rules

Conversation

@PeterDaveHello

Copy link
Copy Markdown
Contributor

Summary

  • apply configured glob and regex ignore rules to single-target CodeCommit pull requests before reading file contents
  • load repository settings from the trusted destination commit so pull request changes cannot alter their own filtering rules
  • avoid mutating shared regex settings while translating glob and generated-code patterns
  • cache completed empty diff results while leaving failed or partial attempts retryable
  • preserve existing multi-target behavior instead of applying one target repository's settings to unrelated targets

Tests

  • verify ignored files are excluded before base or head content is fetched
  • verify repeated filtering does not modify the configured regex list
  • verify completed empty results are cached
  • verify a failed content fetch remains retryable
  • verify source-branch repository settings cannot override destination-commit settings
  • verify multi-target pull requests do not inherit the first target's ignore rules

@qodo-free-for-open-source-projects

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Apply CodeCommit ignore rules before content fetches

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Applies ignore rules before fetching CodeCommit contents for single-target pull requests.
• Loads trusted destination settings while preserving existing multi-target behavior.
• Caches completed results without making failed fetch attempts non-retryable.
Diagram

graph TD
  A["CodeCommit provider"] --> B{"Cached result?"} -->|No| C{"Single target?"} -->|Yes| D["Destination settings"] --> E["Ignore filter"] --> F["Content fetch"]
  F -->|Success| G["Cached diff"]
  F -->|Failure| H["Uncached retry"] --> A
  B -->|Yes| G
  C -->|No| F
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Per-target settings and filtering
  • ➕ Applies each repository's ignore rules correctly in multi-target pull requests.
  • ➕ Avoids fetching ignored contents across every target repository.
  • ➖ Requires settings to be loaded and isolated for each target.
  • ➖ Adds configuration-scoping complexity beyond the current request-wide settings model.
  • ➖ Could change established multi-target behavior without broader integration coverage.

Recommendation: The PR's single-target filtering is the safest immediate fix: it prevents unnecessary content fetches without applying one repository's policy to unrelated targets. Per-target settings would be the preferable long-term enhancement once configuration loading can be scoped independently for each target.

Files changed (4) +159 / -9

Bug fix (2) +16 / -8
file_filter.pySupport CodeCommit filtering without mutating shared patterns +3/-4

Support CodeCommit filtering without mutating shared patterns

• Copies configured regex patterns before appending translated glob and generated-code expressions, preventing repeated calls from mutating shared settings. Treats CodeCommit file objects like GitHub objects by filtering against their filename field.

pr_agent/algo/file_filter.py

codecommit_provider.pyFilter CodeCommit files before fetching contents +13/-4

Filter CodeCommit files before fetching contents

• Applies ignore rules before base and head content retrieval for single-target pull requests while bypassing filtering for multi-target requests. Commits completed diff results atomically so empty results are cached and failed attempts remain retryable, and loads repository settings from the trusted destination commit.

pr_agent/git_providers/codecommit_provider.py

Tests (2) +143 / -1
test_codecommit_provider.pyCover CodeCommit filtering, trust, caching, and retry behavior +125/-1

Cover CodeCommit filtering, trust, caching, and retry behavior

• Adds tests proving ignored contents are never fetched, multi-target requests retain prior behavior, empty results are cached, and failed fetches can be retried. Also verifies repository settings come from the destination commit rather than the source branch.

tests/unittest/test_codecommit_provider.py

test_file_filter.pyVerify repeated filtering preserves configured regexes +18/-0

Verify repeated filtering preserves configured regexes

• Adds regression coverage showing translated glob patterns do not accumulate in the shared regex configuration across repeated filter calls.

tests/unittest/test_file_filter.py

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0) 🎨 UX issues (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Four target comments use narrative prose 📘 Rule violation ⚙ Maintainability
Description
get_diff_files adds a four-line comment that describes repository-setting behavior with
declarative and gerund phrasing instead of imperative wording. When maintainers revisit multi-target
filtering, this block reads as narration rather than an instruction to preserve the fallback
behavior.
Code

pr_agent/git_providers/codecommit_provider.py[R157-160]

+        # Repository settings are request-scoped and currently come from one canonical target.
+        # Applying those ignore rules to a multi-target PR could suppress files from unrelated
+        # target repositories, so preserve the pre-existing multi-target behavior until settings
+        # can be scoped per target.
Evidence
Compliance rule 2694688 requires newly added behavioral comments to use imperative phrasing. The
cited block instead begins with the declarative Repository settings are... and gerund
Applying..., then continues as narrative explanation.

Rule 2694688: Docstrings and comments must use imperative phrasing
pr_agent/git_providers/codecommit_provider.py[157-160]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new multi-target filtering comment uses declarative and gerund phrasing rather than the required imperative style.
## Fix Focus Areas
- pr_agent/git_providers/codecommit_provider.py[157-160]
## Recommended Fix
Rewrite the block as an imperative instruction, such as `Preserve the pre-existing multi-target behavior because request-scoped settings currently come from one canonical target and could suppress unrelated files.`

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


  • Author self-review: I have reviewed the code review findings, and addressed the relevant ones.

Grey Divider

Tip of the day
💡 Did you know, you can turn on the rule miner and Qodo learns your standards from review history

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@IsmaelMartinez IsmaelMartinez changed the title fix(codecommit): apply ignore rules before fetching contents fix(codecommit): apply ignore rules and read .pr_agent.toml from the destination commit Sep 20, 2026

@IsmaelMartinez IsmaelMartinez left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for wiring CodeCommit into the shared ignore filter ahead of the content fetch, and for moving .pr_agent.toml to the destination commit, which is the trust boundary the configuration docs describe. Approving.

An ignored path no longer has either side fetched, an empty result is cached, a failed fetch stays retryable, and multi-target requests are untouched. All five provider tests go red against main.

Qodo's note on the comment phrasing is style only and does not block.

@IsmaelMartinez
IsmaelMartinez merged commit c2a37e6 into The-PR-Agent:main Sep 20, 2026
10 checks passed
@PeterDaveHello
PeterDaveHello deleted the fix/codecommit-ignore-rules branch September 24, 2026 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants