Skip to content

Conversation

@aibaars
Copy link
Contributor

@aibaars aibaars commented Jun 16, 2025

This pull request limits the number of error/warning/info diagnostics we store in the CodeQL database to 100 per file. This should avoid SARIF from growing too large.

@github-actions github-actions bot added the Rust Pull requests that update Rust code label Jun 16, 2025
@aibaars aibaars force-pushed the aibaars/limit-diagnostics branch from bd214b4 to c960af8 Compare June 16, 2025 10:11
@aibaars aibaars marked this pull request as ready for review June 18, 2025 08:03
Copilot AI review requested due to automatic review settings June 18, 2025 08:03
@aibaars aibaars requested a review from a team as a code owner June 18, 2025 08:03
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a hard cap of 100 diagnostics per trap file to prevent excessive emissions.

  • Introduced an emit_diagnostic_limit field on Translator and initialized it to 100.
  • Added EMIT_DIAGNOSTIC_LIMIT_PER_FILE constant and applied a check/decrement in the diagnostic emission path.
  • Kept existing behavior for Debug-level diagnostics (they are not counted against the limit).
Comments suppressed due to low confidence (2)

rust/extractor/src/translate/base.rs:126

  • Add a doc comment for the emit_diagnostic_limit field (and for EMIT_DIAGNOSTIC_LIMIT_PER_FILE) to explain its purpose and units, so future maintainers understand its role.
    emit_diagnostic_limit: usize,

rust/extractor/src/translate/base.rs:251

  • No tests were added to verify that diagnostics stop after the limit is reached; consider adding a unit or integration test that emits over 100 diagnostics and asserts that subsequent ones are suppressed.
        if severity > DiagnosticSeverity::Debug && self.emit_diagnostic_limit > 0 {

const UNKNOWN_LOCATION: (LineCol, LineCol) =
(LineCol { line: 0, col: 0 }, LineCol { line: 0, col: 0 });

const EMIT_DIAGNOSTIC_LIMIT_PER_FILE: usize = 100;
Copy link

Copilot AI Jun 18, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider making the diagnostic limit configurable (e.g., via a constructor argument or config file) instead of a fixed constant, which would improve flexibility and allow per-project adjustments.

Copilot uses AI. Check for mistakes.
hvitved
hvitved previously approved these changes Jun 18, 2025
@aibaars aibaars force-pushed the aibaars/limit-diagnostics branch from 05ab438 to a253b31 Compare June 18, 2025 14:05
@aibaars aibaars merged commit 9605eb0 into main Jun 20, 2025
14 checks passed
@aibaars aibaars deleted the aibaars/limit-diagnostics branch June 20, 2025 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants