Skip to content

Review agent should flag test file references to removed methods as high-severity #218

Description

@fullsend-ai-retro

What happened

PR #114 removed three methods from gcsweb.py (_extract_operator_version, _fetch_build_log_text, _DEFAULT_OPERATOR_VERSION_PATTERNS) but did not update the test file test_build_health.py, which has 14 call sites referencing _extract_operator_version. The review agent was dispatched (run 29514681265) but could not review because the PR was already merged (9-second merge window). If the review had run, the correctness sub-agent should have detected that methods deleted in the PR are still referenced in test files — this is a mechanical check (deleted symbol referenced elsewhere) that is high-confidence and high-impact.

What could go better

The review agent's correctness sub-agent should include a check for deleted functions/methods/classes that are still referenced in test files (or any other files in the repo). This is a straightforward static analysis pattern: when a PR deletes a function definition, search for call sites in unchanged files. If found, flag as high-severity (test breakage). This check would have caught the bug in PR #114 had the review agent been able to run. It would also complement the proposal to add test CI — reviews run even when CI is absent. Confidence: medium — this check is mechanically sound but the review agent's ability to perform cross-file reference analysis depends on its tool access and prompting.

Proposed change

Add guidance to the review agent's correctness sub-agent skill (or the review agent definition in fullsend-ai/agents) to check for 'orphaned references' when a PR deletes function/method definitions. Specifically: when a PR removes a def method_name or class ClassName, the sub-agent should search test files and other unchanged files for references to the deleted symbol. If found, flag as HIGH severity with the finding: 'Method X was removed in file A but is still referenced in file B at lines N'. This check applies to Python, JavaScript, TypeScript, and other languages where function names are used as identifiers.

Validation criteria

When a PR deletes a function and test files still reference it, the review agent flags it as a high-severity correctness finding before the PR can be merged.


Generated by retro agent from redhat-community-ai-tools/ci-failure-tracker#114

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions