Skip to content

Conversation

@michaelnebel
Copy link
Contributor

No description provided.

Copilot AI review requested due to automatic review settings September 4, 2025 12:16

This comment was marked as outdated.

@michaelnebel michaelnebel force-pushed the csharp/improvedatabasequalitydiagnostics branch 3 times, most recently from 6b0657b to cafe644 Compare September 4, 2025 13:17
Copilot AI review requested due to automatic review settings September 4, 2025 13:17
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 improves the database quality diagnostics message for C# by including specific health metrics in the diagnostic output. The enhancement provides users with concrete percentage values for call targets and expression types to help them better understand database quality issues.

Key changes:

  • Enhanced diagnostic message to include specific database health metrics with percentage values
  • Refactored the diagnostic creation logic to capture and display both call target and expression type statistics
  • Updated expected test outputs to reflect the new message format

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
csharp/ql/src/change-notes/2025-09-04-database-diagnostics.md Documents the enhancement to include database health metrics in diagnostic messages
csharp/ql/src/Telemetry/DatabaseQualityDiagnostics.ql Refactors diagnostic logic to capture specific metrics and updates the diagnostic message format
csharp/ql/integration-tests/all-platforms/standalone/DatabaseQualityDiagnostics.expected Updates expected test output to match the new diagnostic message format

@michaelnebel michaelnebel force-pushed the csharp/improvedatabasequalitydiagnostics branch from cafe644 to 02e2b65 Compare September 4, 2025 13:31
@michaelnebel michaelnebel changed the title C#: Improve database quality diagnostics message. C#: Improve database quality diagnostics query. Sep 5, 2025
@michaelnebel michaelnebel force-pushed the csharp/improvedatabasequalitydiagnostics branch from 02e2b65 to 70b3a46 Compare September 5, 2025 09:17
@michaelnebel michaelnebel marked this pull request as ready for review September 5, 2025 10:40
@michaelnebel michaelnebel requested a review from a team as a code owner September 5, 2025 10:40

private string getDbHealth() {
result =
callMsg + ": " + callTargetOk.floor() + ". " + exprMsg + ": " + exprTypeOk.floor() + ". "
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe

Suggested change
callMsg + ": " + callTargetOk.floor() + ". " + exprMsg + ": " + exprTypeOk.floor() + ". "
callMsg + ": " + callTargetOk.floor() + "%. " + exprMsg + ": " + exprTypeOk.floor() + "%. "

?

|
percentageGood < 95
)
TTheDbQualityDiagnostic(string callMsg, float callTargetOk, string exprMsg, float exprTypeOk) {
Copy link
Contributor

Choose a reason for hiding this comment

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

How about instead:

private predicate diagnostic(string msg, float value, float threshold) {
  CallTargetStatsReport::percentageOfOk(msg, value) and
  threshold = 85
  or
  ExprTypeStatsReport::percentageOfOk(msg, value) and
  threshold = 85
}

private newtype TDbQualityDiagnostic =
  TTheDbQualityDiagnostic() {
    exists(float percentageGood, float threshold |
      diagnostic(_, percentageGood, threshold) and
      percentageGood < threshold
    )
  }

...

private string getDbHealth() {
    result =
      strictconcat(string msg, float value, float threshold |
        diagnostic(msg, value, threshold)
      |
        msg + ": " + value.floor() + " % (threshold " + threshold.floor() + " %)", ". "
      )
  }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, considering the discussion on the posibility of having different thresholds pr metric - then this looks very good.
I will address the review comments for C# and when we have reached consensus, I will update for the other languages as well (in this PR).

hvitved
hvitved previously approved these changes Sep 8, 2025
Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

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

Thanks for fixing.

@michaelnebel michaelnebel marked this pull request as draft September 8, 2025 09:50
@michaelnebel michaelnebel force-pushed the csharp/improvedatabasequalitydiagnostics branch from ae516ee to 29c22e6 Compare September 8, 2025 10:06
@github-actions github-actions bot added Java Rust Pull requests that update Rust code labels Sep 8, 2025
@michaelnebel michaelnebel marked this pull request as ready for review September 8, 2025 10:55
@michaelnebel michaelnebel requested review from a team as code owners September 8, 2025 10:55
@michaelnebel michaelnebel requested a review from hvitved September 8, 2025 10:57
Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

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

Thanks again.

@michaelnebel michaelnebel changed the title C#: Improve database quality diagnostics query. C#/Java/Rust: Improve database quality diagnostics query. Sep 8, 2025
@michaelnebel michaelnebel merged commit 84df8f9 into github:main Sep 8, 2025
39 checks passed
@michaelnebel michaelnebel deleted the csharp/improvedatabasequalitydiagnostics branch September 8, 2025 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C# documentation Java Rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants