From c1b801c5468672a6020f52abdfe991e4abb3af3f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nora=20Dimitrijevi=C4=87?= <d10c@users.noreply.github.com>
Date: Tue, 7 Jul 2026 22:07:34 +0200
Subject: [PATCH 1/9] Support --learn for inline expectations via learnEdits

Add a `learnEdits` query predicate to `TestPostProcessing::Make` so that
`codeql test run --learn` can update inline `// $ Alert` expectation comments in
source files, instead of only rewriting `.expected`. The test runner consumes
this predicate and applies the edits; here we only compute them.

The predicate emits a deliberately reliable MVP subset, restricted to the plain
`Alert` tag with no value or query-id annotation:

- an actual result with no matching expectation -> append a new `// $ Alert`
  comment on the result's line ("append" operation), and
- a plain `// $ Alert` comment that is the sole expectation on its line and no
  longer matches any result -> remove the comment ("replace" with "").

The comment is appended on, and removed from, the result's *end* line: an
expectation matches a result when the expectation's start line equals the
result's end line (see `onSameLine`). Most results span a single line, but some
extractors include leading trivia in a location (e.g. Rust), so start and end
lines can differ. Removal deletes from the comment marker to the end of the
line ("replace" with an end column of 0, the engine's "to end of line"
convention); this avoids depending on how each extractor reports a line
comment's end column (e.g. Swift reports it as ending at column 1 of the next
line).

Cases needing sub-comment column surgery (promoting `MISSING:`, clearing
`SPURIOUS:`, or editing one tag among several) are left for a follow-up.

To render a new comment, `InputSig` gains `getStartCommentMarker(relativePath)`
and a defaulted `getEndCommentMarker(relativePath)`. These are keyed on the
source file's relative path rather than the analysed language, because one
database can mix languages with different comment syntaxes (e.g. Java + XML);
each per-language Input module returns a marker only for the file types whose
comment syntax it supports. Languages whose extractor can ingest other file
types (e.g. C# also extracts XML, JavaScript also extracts HTML) gate on the
file extension so those files are skipped; extractors that only ingest a single
line-comment language (e.g. Swift) can return a constant. Both predicates are
`bindingset[relativePath]` so they need not be materialised for all files. The
end marker is defaulted to "" so existing modules need only implement the start
marker; this leaves the door open for block-comment (XML/YAML) languages in a
later PR without another signature break.

No committed QL test accompanies this change: the predicate is only observable
through the engine's `--learn` handling (a released engine rejects the reserved
`learnEdits` name outright), so it cannot be pinned via a `.expected` file. It
is instead covered by engine-side end-to-end tests, one per language.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---
 .../utils/test/InlineExpectationsTestQuery.ql |   9 ++
 .../utils/test/InlineExpectationsTestQuery.ql |   8 ++
 .../utils/test/InlineExpectationsTestQuery.ql |   8 ++
 .../utils/test/InlineExpectationsTestQuery.ql |   8 ++
 .../utils/test/InlineExpectationsTestQuery.ql |   8 ++
 .../utils/test/InlineExpectationsTestQuery.ql |   8 ++
 .../utils/test/InlineExpectationsTestQuery.ql |   8 ++
 .../utils/test/InlineExpectationsTestQuery.ql |   8 ++
 .../utils/test/InlineExpectationsTestQuery.ql |   8 ++
 .../util/test/InlineExpectationsTest.qll      | 122 ++++++++++++++++++
 .../utils/test/InlineExpectationsTestQuery.ql |   7 +
 .../utils/test/InlineExpectationsTestQuery.ql |  10 ++
 12 files changed, 212 insertions(+)

diff --git a/cpp/ql/lib/utils/test/InlineExpectationsTestQuery.ql b/cpp/ql/lib/utils/test/InlineExpectationsTestQuery.ql
index 8e6977ba5321..1325e5e3fd12 100644
--- a/cpp/ql/lib/utils/test/InlineExpectationsTestQuery.ql
+++ b/cpp/ql/lib/utils/test/InlineExpectationsTestQuery.ql
@@ -18,4 +18,13 @@ private module Input implements T::TestPostProcessing::InputSig<Impl> {
         f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn
     )
   }
+
+  bindingset[relativePath]
+  string getStartCommentMarker(string relativePath) {
+    // C/C++ databases can also contain XML (e.g. `.xml`, `.props`), whose block-comment
+    // syntax is not yet supported, so we only render for C/C++/Objective-C sources.
+    relativePath
+        .regexpMatch(".*\\.(c|cc|cpp|cxx|cp|c\\+\\+|h|hh|hpp|hxx|h\\+\\+|inl|tcc|ipp|tpp|cu|cuh|m|mm)") and
+    result = "//"
+  }
 }
diff --git a/csharp/ql/lib/utils/test/InlineExpectationsTestQuery.ql b/csharp/ql/lib/utils/test/InlineExpectationsTestQuery.ql
index 35901ee64012..1609ca867e80 100644
--- a/csharp/ql/lib/utils/test/InlineExpectationsTestQuery.ql
+++ b/csharp/ql/lib/utils/test/InlineExpectationsTestQuery.ql
@@ -18,4 +18,12 @@ private module Input implements T::TestPostProcessing::InputSig<Impl> {
         f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn
     )
   }
+
+  bindingset[relativePath]
+  string getStartCommentMarker(string relativePath) {
+    // C# databases can also contain XML (e.g. `.csproj`, `.config`) and Razor markup, whose
+    // comment syntaxes are not yet supported, so we only render for C# sources.
+    relativePath.regexpMatch(".*\\.(cs|csx)") and
+    result = "//"
+  }
 }
diff --git a/go/ql/lib/utils/test/InlineExpectationsTestQuery.ql b/go/ql/lib/utils/test/InlineExpectationsTestQuery.ql
index 1cf2f5ea1d9b..9a180ed8a173 100644
--- a/go/ql/lib/utils/test/InlineExpectationsTestQuery.ql
+++ b/go/ql/lib/utils/test/InlineExpectationsTestQuery.ql
@@ -18,4 +18,12 @@ private module Input implements T::TestPostProcessing::InputSig<Impl> {
         f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn
     )
   }
+
+  bindingset[relativePath]
+  string getStartCommentMarker(string relativePath) {
+    // Go databases can also contain XML, whose block-comment syntax is not yet supported, so
+    // we only render for Go sources.
+    relativePath.matches("%.go") and
+    result = "//"
+  }
 }
diff --git a/java/ql/lib/utils/test/InlineExpectationsTestQuery.ql b/java/ql/lib/utils/test/InlineExpectationsTestQuery.ql
index b0360dfecd8d..ad330a3d1f2c 100644
--- a/java/ql/lib/utils/test/InlineExpectationsTestQuery.ql
+++ b/java/ql/lib/utils/test/InlineExpectationsTestQuery.ql
@@ -18,4 +18,12 @@ private module Input implements T::TestPostProcessing::InputSig<Impl> {
         f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn
     )
   }
+
+  bindingset[relativePath]
+  string getStartCommentMarker(string relativePath) {
+    // Java databases can also contain XML; those files use a different (block) comment
+    // syntax that is not yet supported, so we only render for Java and Kotlin sources.
+    (relativePath.matches("%.java") or relativePath.matches("%.kt")) and
+    result = "//"
+  }
 }
diff --git a/javascript/ql/lib/utils/test/InlineExpectationsTestQuery.ql b/javascript/ql/lib/utils/test/InlineExpectationsTestQuery.ql
index 55892be75d79..c1151457a0a2 100644
--- a/javascript/ql/lib/utils/test/InlineExpectationsTestQuery.ql
+++ b/javascript/ql/lib/utils/test/InlineExpectationsTestQuery.ql
@@ -18,4 +18,12 @@ private module Input implements T::TestPostProcessing::InputSig<Impl> {
         f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn
     )
   }
+
+  bindingset[relativePath]
+  string getStartCommentMarker(string relativePath) {
+    // JavaScript databases can also contain HTML, whose (block) comment syntax is not yet
+    // supported, so we only render for the line-comment source files.
+    relativePath.regexpMatch(".*\\.(js|cjs|mjs|jsx|ts|cts|mts|tsx)") and
+    result = "//"
+  }
 }
diff --git a/python/ql/lib/utils/test/InlineExpectationsTestQuery.ql b/python/ql/lib/utils/test/InlineExpectationsTestQuery.ql
index 9ce5fdf326ca..d849809f6d9d 100644
--- a/python/ql/lib/utils/test/InlineExpectationsTestQuery.ql
+++ b/python/ql/lib/utils/test/InlineExpectationsTestQuery.ql
@@ -18,4 +18,12 @@ private module Input implements T::TestPostProcessing::InputSig<Impl> {
         f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn
     )
   }
+
+  bindingset[relativePath]
+  string getStartCommentMarker(string relativePath) {
+    // Python databases can also contain XML, whose block-comment syntax is not yet supported,
+    // so we only render for Python sources.
+    relativePath.regexpMatch(".*\\.(py|pyi)") and
+    result = "#"
+  }
 }
diff --git a/ql/ql/src/utils/test/InlineExpectationsTestQuery.ql b/ql/ql/src/utils/test/InlineExpectationsTestQuery.ql
index 979839480e1d..89d8a2788e99 100644
--- a/ql/ql/src/utils/test/InlineExpectationsTestQuery.ql
+++ b/ql/ql/src/utils/test/InlineExpectationsTestQuery.ql
@@ -18,4 +18,12 @@ private module Input implements T::TestPostProcessing::InputSig<Impl> {
         f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn
     )
   }
+
+  bindingset[relativePath]
+  string getStartCommentMarker(string relativePath) {
+    // The QL extractor can also extract YAML (e.g. `qlpack.yml`), whose `#` comment syntax
+    // differs, so we only render for QL sources and dbscheme files.
+    relativePath.regexpMatch(".*\\.(ql|qll|dbscheme)") and
+    result = "//"
+  }
 }
diff --git a/ruby/ql/lib/utils/test/InlineExpectationsTestQuery.ql b/ruby/ql/lib/utils/test/InlineExpectationsTestQuery.ql
index 1cbc37a7fe85..08e60a966a15 100644
--- a/ruby/ql/lib/utils/test/InlineExpectationsTestQuery.ql
+++ b/ruby/ql/lib/utils/test/InlineExpectationsTestQuery.ql
@@ -18,4 +18,12 @@ private module Input implements T::TestPostProcessing::InputSig<Impl> {
         f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn
     )
   }
+
+  bindingset[relativePath]
+  string getStartCommentMarker(string relativePath) {
+    // Ruby databases can also contain ERB, whose comment syntax is not yet supported, so we
+    // only render for plain Ruby sources.
+    relativePath.matches("%.rb") and
+    result = "#"
+  }
 }
diff --git a/rust/ql/lib/utils/test/InlineExpectationsTestQuery.ql b/rust/ql/lib/utils/test/InlineExpectationsTestQuery.ql
index e5821ba4f50c..b20a70450dce 100644
--- a/rust/ql/lib/utils/test/InlineExpectationsTestQuery.ql
+++ b/rust/ql/lib/utils/test/InlineExpectationsTestQuery.ql
@@ -18,4 +18,12 @@ private module Input implements T::TestPostProcessing::InputSig<Impl> {
         f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn
     )
   }
+
+  bindingset[relativePath]
+  string getStartCommentMarker(string relativePath) {
+    // Rust databases can also contain YAML, whose `#` comment syntax differs, so we only
+    // render for Rust sources.
+    relativePath.matches("%.rs") and
+    result = "//"
+  }
 }
diff --git a/shared/util/codeql/util/test/InlineExpectationsTest.qll b/shared/util/codeql/util/test/InlineExpectationsTest.qll
index 4e0b2f678449..3834495b7907 100644
--- a/shared/util/codeql/util/test/InlineExpectationsTest.qll
+++ b/shared/util/codeql/util/test/InlineExpectationsTest.qll
@@ -635,6 +635,28 @@ module TestPostProcessing {
 
   signature module InputSig<InlineExpectationsTestSig Input> {
     string getRelativeUrl(Input::Location location);
+
+    /**
+     * Gets the marker that starts a line comment (for example `"//"` or `"#"`) in the source
+     * file with the given `relativePath`, provided that `codeql test run --learn` is able to
+     * render inline expectations for that file. Files for which this has no result are left
+     * untouched by `--learn`.
+     *
+     * This is keyed on the file rather than on the analysed language because a single database
+     * may contain source files in several languages with different comment syntaxes (for
+     * example Java together with XML). `relativePath` is the path reported by `getRelativeUrl`.
+     */
+    bindingset[relativePath]
+    string getStartCommentMarker(string relativePath);
+
+    /**
+     * Gets the marker that ends a comment (for example `"-->"`) in the source file with the
+     * given `relativePath`. Defaults to the empty string, which is correct for languages whose
+     * inline expectations use line comments; block-comment languages can override it so that
+     * `--learn` renders a closing marker.
+     */
+    bindingset[relativePath]
+    default string getEndCommentMarker(string relativePath) { result = "" }
   }
 
   module Make<InlineExpectationsTestSig Input, InputSig<Input> Input2> {
@@ -1011,5 +1033,105 @@ module TestPostProcessing {
       Test::testFailures(_, _) and
       relation = "testFailures"
     }
+
+    /**
+     * Gets the fully rendered inline expectation comment (including the comment markers) that
+     * `--learn` should insert for a new `tag` expectation in the file with the given
+     * `relativePath`, or has no result if that file's comment syntax is not supported.
+     *
+     * The leading space separates the comment from any existing content on the line.
+     */
+    bindingset[relativePath, tag]
+    private string renderExpectationComment(string relativePath, string tag) {
+      exists(string startMarker, string endMarker, string endSuffix |
+        startMarker = Input2::getStartCommentMarker(relativePath) and
+        endMarker = Input2::getEndCommentMarker(relativePath) and
+        (
+          endMarker = "" and endSuffix = ""
+          or
+          endMarker != "" and endSuffix = " " + endMarker
+        ) and
+        result = " " + startMarker + " $ " + tag + endSuffix
+      )
+    }
+
+    /**
+     * Holds if `codeql test run --learn` should edit the source file `file` so that its inline
+     * expectations match the current query results. Each row asks the test runner to change
+     * `line`, where `operation` is either:
+     *
+     * - `"append"`: add `text` (a fully rendered comment) at the end of the line; `startColumn`
+     *   and `endColumn` are both 0.
+     * - `"replace"`: replace the 1-based inclusive column range `[startColumn, endColumn]` with
+     *   `text` (the empty string deletes the range).
+     *
+     * Only a deliberately reliable subset is emitted so far, restricted to the plain `Alert`
+     * tag with no value or query-id annotation:
+     *
+     * - an actual result with no matching expectation gets a new `// $ Alert` comment appended
+     *   (an *unexpected result*), and
+     * - a plain `// $ Alert` comment whose result is now missing, and which is the only
+     *   expectation on that comment, is removed (a *missing result*).
+     *
+     * Cases that need sub-comment column information (promoting `MISSING:`/clearing `SPURIOUS:`,
+     * or editing one tag among several on a line) are intentionally left for a follow-up.
+     */
+    query predicate learnEdits(
+      string file, int line, string operation, int startColumn, int endColumn, string text
+    ) {
+      // Unexpected result: append a new `// $ Alert` comment on the alert's line. The comment
+      // must go on the result's *end* line, because an expectation matches a result when the
+      // expectation's start line equals the result's end line (see `onSameLine`). For most
+      // languages a result spans a single line, but some (e.g. Rust) include leading trivia in
+      // the location, so the start and end lines differ.
+      exists(Test::ActualTestResult actualResult, string relativePath, int el, string comment |
+        actualResult.getTag() = "Alert" and
+        actualResult.getValue() = "" and
+        not actualResult.isOptional() and
+        not exists(
+          Test::getAMatchingExpectation(actualResult.getLocation(), actualResult.toString(),
+            actualResult.getTag(), actualResult.getValue(), false)
+        ) and
+        parseLocationString(actualResult.getLocation().getRelativeUrl(), relativePath, _, _, el, _) and
+        comment = renderExpectationComment(relativePath, "Alert") and
+        file = relativePath and
+        line = el and
+        operation = "append" and
+        startColumn = 0 and
+        endColumn = 0 and
+        text = comment
+      )
+      or
+      // Missing result: remove a plain `// $ Alert` comment that is the comment's sole
+      // expectation and no longer matches any actual result.
+      exists(Test::GoodTestExpectation expectation, string relativePath, int sl, int sc |
+        expectation.getTag() = "Alert" and
+        expectation.getValue() = "" and
+        not expectation = Test::getAMatchingExpectation(_, _, _, _, _) and
+        // the comment carries exactly this one expectation, so removing it wholesale is safe
+        count(Test::FailureLocatable other |
+          other.getLocation() = expectation.getLocation() and
+          (
+            other instanceof Test::GoodTestExpectation or
+            other instanceof Test::FalsePositiveTestExpectation or
+            other instanceof Test::FalseNegativeTestExpectation
+          )
+        ) = 1 and
+        not exists(Test::InvalidTestExpectation invalid |
+          invalid.getLocation() = expectation.getLocation()
+        ) and
+        parseLocationString(expectation.getLocation().getRelativeUrl(), relativePath, sl, sc, _, _) and
+        file = relativePath and
+        line = sl and
+        operation = "replace" and
+        startColumn = sc and
+        // A trailing inline expectation comment always runs to the end of its line, so delete from
+        // the comment marker to the end of the line. `endColumn = 0` is the engine's "to end of
+        // line" convention; using it avoids depending on how each extractor reports a line
+        // comment's end column (e.g. Swift reports it as ending at column 1 of the next line).
+        endColumn = 0 and
+        text = ""
+      )
+    }
   }
 }
diff --git a/swift/ql/lib/utils/test/InlineExpectationsTestQuery.ql b/swift/ql/lib/utils/test/InlineExpectationsTestQuery.ql
index a7c112bc00e0..8f7e397d0afe 100644
--- a/swift/ql/lib/utils/test/InlineExpectationsTestQuery.ql
+++ b/swift/ql/lib/utils/test/InlineExpectationsTestQuery.ql
@@ -18,4 +18,11 @@ private module Input implements T::TestPostProcessing::InputSig<Impl> {
         f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn
     )
   }
+
+  bindingset[relativePath]
+  string getStartCommentMarker(string relativePath) {
+    // The Swift extractor only ingests Swift sources (no XML/YAML/HTML in its dbscheme), so a
+    // constant marker is safe; revisit if Swift ever gains extraction of another file type.
+    exists(relativePath) and result = "//"
+  }
 }
diff --git a/unified/ql/lib/utils/test/InlineExpectationsTestQuery.ql b/unified/ql/lib/utils/test/InlineExpectationsTestQuery.ql
index 039194bc2e38..950898fb12be 100644
--- a/unified/ql/lib/utils/test/InlineExpectationsTestQuery.ql
+++ b/unified/ql/lib/utils/test/InlineExpectationsTestQuery.ql
@@ -18,4 +18,14 @@ private module Input implements T::TestPostProcessing::InputSig<Impl> {
         f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn
     )
   }
+
+  bindingset[relativePath]
+  string getStartCommentMarker(string relativePath) {
+    // The unified extractor is a new tree-sitter-based extractor that currently ingests only
+    // Swift sources (see `file_types` in its `codeql-extractor.yml`), which use `//`. Gating on
+    // the extension keeps this correct if it later gains a language with a different comment
+    // syntax.
+    relativePath.regexpMatch(".*\\.(swift|swiftinterface)") and
+    result = "//"
+  }
 }

From 593415579b639410151f6da2580ef9426343475a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nora=20Dimitrijevi=C4=87?= <d10c@users.noreply.github.com>
Date: Thu, 9 Jul 2026 13:02:03 +0200
Subject: [PATCH 2/9] Address review: drop Objective-C extensions and fix
 spelling

The C/C++ start-comment marker regex listed `.m`/`.mm` (Objective-C and
Objective-C++), but the cpp extractor does not extract Objective-C, so
those files never appear in a database and the entries were dead. Drop
them and reword the comment to say we only render for C/C++ sources, per
review feedback.

Also fix the US spelling "analyzed" in the shared getStartCommentMarker
doc comment, flagged by the misspelling check.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---
 cpp/ql/lib/utils/test/InlineExpectationsTestQuery.ql    | 4 ++--
 shared/util/codeql/util/test/InlineExpectationsTest.qll | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/cpp/ql/lib/utils/test/InlineExpectationsTestQuery.ql b/cpp/ql/lib/utils/test/InlineExpectationsTestQuery.ql
index 1325e5e3fd12..c46f895431c7 100644
--- a/cpp/ql/lib/utils/test/InlineExpectationsTestQuery.ql
+++ b/cpp/ql/lib/utils/test/InlineExpectationsTestQuery.ql
@@ -22,9 +22,9 @@ private module Input implements T::TestPostProcessing::InputSig<Impl> {
   bindingset[relativePath]
   string getStartCommentMarker(string relativePath) {
     // C/C++ databases can also contain XML (e.g. `.xml`, `.props`), whose block-comment
-    // syntax is not yet supported, so we only render for C/C++/Objective-C sources.
+    // syntax is not yet supported, so we only render for C/C++ sources.
     relativePath
-        .regexpMatch(".*\\.(c|cc|cpp|cxx|cp|c\\+\\+|h|hh|hpp|hxx|h\\+\\+|inl|tcc|ipp|tpp|cu|cuh|m|mm)") and
+        .regexpMatch(".*\\.(c|cc|cpp|cxx|cp|c\\+\\+|h|hh|hpp|hxx|h\\+\\+|inl|tcc|ipp|tpp|cu|cuh)") and
     result = "//"
   }
 }
diff --git a/shared/util/codeql/util/test/InlineExpectationsTest.qll b/shared/util/codeql/util/test/InlineExpectationsTest.qll
index 3834495b7907..bae62cd8f2ea 100644
--- a/shared/util/codeql/util/test/InlineExpectationsTest.qll
+++ b/shared/util/codeql/util/test/InlineExpectationsTest.qll
@@ -642,7 +642,7 @@ module TestPostProcessing {
      * render inline expectations for that file. Files for which this has no result are left
      * untouched by `--learn`.
      *
-     * This is keyed on the file rather than on the analysed language because a single database
+     * This is keyed on the file rather than on the analyzed language because a single database
      * may contain source files in several languages with different comment syntaxes (for
      * example Java together with XML). `relativePath` is the path reported by `getRelativeUrl`.
      */

From 28dd3a74efe49daf22fb4eea5d59690f999d311f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nora=20Dimitrijevi=C4=87?= <d10c@users.noreply.github.com>
Date: Thu, 9 Jul 2026 15:53:22 +0200
Subject: [PATCH 3/9] Support MISSING promotion and SPURIOUS clearing in
 learnEdits

Extend the `learnEdits` predicate used by `codeql test run --learn` to
cover two more inline-expectation cases beyond the initial add/remove
subset, still restricted to the plain `Alert` tag on comments that carry
a single expectation:

- `// $ MISSING: Alert` whose result now appears is promoted to a plain
  `// $ Alert` (a *fixed missing result*), by rewriting the whole comment
  in place from its marker to the end of the line.
- `// $ SPURIOUS: Alert` whose result no longer fires is removed (a
  *fixed spurious result*), reusing the same whole-comment deletion path
  as a now-missing plain `Alert`.

Both promotion and removal operate on the comment as a whole rather than
editing individual columns within it, which is reliable for a
single-expectation comment; the `isSoleExpectationOnComment` guard (also
excluding comments with an unparseable expectation) keeps multi-tag
comments out of scope for a follow-up. The renderer is split into
`renderInlineComment` (no leading space, for in-place rewrites starting
at the comment marker) and `renderExpectationComment` (adds the leading
separator space, for appending after code).

The removal path relies on the engine's `endColumn = 0` "delete to end of
line" convention, which now also trims the whitespace gap the removed
trailing comment leaves behind.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---
 .../util/test/InlineExpectationsTest.qll      | 112 +++++++++++++-----
 1 file changed, 83 insertions(+), 29 deletions(-)

diff --git a/shared/util/codeql/util/test/InlineExpectationsTest.qll b/shared/util/codeql/util/test/InlineExpectationsTest.qll
index bae62cd8f2ea..d4d66efe80b3 100644
--- a/shared/util/codeql/util/test/InlineExpectationsTest.qll
+++ b/shared/util/codeql/util/test/InlineExpectationsTest.qll
@@ -1035,14 +1035,17 @@ module TestPostProcessing {
     }
 
     /**
-     * Gets the fully rendered inline expectation comment (including the comment markers) that
-     * `--learn` should insert for a new `tag` expectation in the file with the given
-     * `relativePath`, or has no result if that file's comment syntax is not supported.
+     * Gets the inline expectation comment (including the comment markers, but with no leading
+     * whitespace) that renders a plain `tag` expectation in the file with the given `relativePath`,
+     * or has no result if that file's comment syntax is not supported. For example `// $ Alert` for
+     * a language whose line comments start with `//`.
      *
-     * The leading space separates the comment from any existing content on the line.
+     * This form is used when rewriting an existing comment in place, starting at its comment
+     * marker, so it must not carry the leading separator space that `renderExpectationComment`
+     * adds for appending after code.
      */
     bindingset[relativePath, tag]
-    private string renderExpectationComment(string relativePath, string tag) {
+    private string renderInlineComment(string relativePath, string tag) {
       exists(string startMarker, string endMarker, string endSuffix |
         startMarker = Input2::getStartCommentMarker(relativePath) and
         endMarker = Input2::getEndCommentMarker(relativePath) and
@@ -1051,7 +1054,39 @@ module TestPostProcessing {
           or
           endMarker != "" and endSuffix = " " + endMarker
         ) and
-        result = " " + startMarker + " $ " + tag + endSuffix
+        result = startMarker + " $ " + tag + endSuffix
+      )
+    }
+
+    /**
+     * Gets the fully rendered inline expectation comment (including the comment markers) that
+     * `--learn` should append for a new `tag` expectation in the file with the given
+     * `relativePath`, or has no result if that file's comment syntax is not supported.
+     *
+     * The leading space separates the comment from any existing content on the line.
+     */
+    bindingset[relativePath, tag]
+    private string renderExpectationComment(string relativePath, string tag) {
+      result = " " + renderInlineComment(relativePath, tag)
+    }
+
+    /**
+     * Holds if `expectation` is the only inline expectation carried by its comment, so `--learn`
+     * can rewrite or delete the comment as a whole without disturbing a sibling expectation.
+     * Comments that additionally carry an invalid (unparseable) expectation are excluded, to avoid
+     * corrupting text the library could not fully understand.
+     */
+    private predicate isSoleExpectationOnComment(Test::FailureLocatable expectation) {
+      count(Test::FailureLocatable other |
+        other.getLocation() = expectation.getLocation() and
+        (
+          other instanceof Test::GoodTestExpectation or
+          other instanceof Test::FalsePositiveTestExpectation or
+          other instanceof Test::FalseNegativeTestExpectation
+        )
+      ) = 1 and
+      not exists(Test::InvalidTestExpectation invalid |
+        invalid.getLocation() = expectation.getLocation()
       )
     }
 
@@ -1066,15 +1101,18 @@ module TestPostProcessing {
      *   `text` (the empty string deletes the range).
      *
      * Only a deliberately reliable subset is emitted so far, restricted to the plain `Alert`
-     * tag with no value or query-id annotation:
+     * tag with no value or query-id annotation, on comments that carry a single expectation:
      *
      * - an actual result with no matching expectation gets a new `// $ Alert` comment appended
-     *   (an *unexpected result*), and
-     * - a plain `// $ Alert` comment whose result is now missing, and which is the only
-     *   expectation on that comment, is removed (a *missing result*).
+     *   (an *unexpected result*);
+     * - a comment whose sole expectation is a plain `// $ Alert` whose result is now missing, or a
+     *   `// $ SPURIOUS: Alert` whose result no longer fires, is removed (a *missing result* or a
+     *   *fixed spurious result*); and
+     * - a comment whose sole expectation is `// $ MISSING: Alert` whose result now appears is
+     *   promoted to a plain `// $ Alert` (a *fixed missing result*).
      *
-     * Cases that need sub-comment column information (promoting `MISSING:`/clearing `SPURIOUS:`,
-     * or editing one tag among several on a line) are intentionally left for a follow-up.
+     * Cases that need sub-comment column information (editing one tag among several on a comment
+     * that carries multiple expectations) are intentionally left for a follow-up.
      */
     query predicate learnEdits(
       string file, int line, string operation, int startColumn, int endColumn, string text
@@ -1102,24 +1140,19 @@ module TestPostProcessing {
         text = comment
       )
       or
-      // Missing result: remove a plain `// $ Alert` comment that is the comment's sole
-      // expectation and no longer matches any actual result.
-      exists(Test::GoodTestExpectation expectation, string relativePath, int sl, int sc |
+      // Obsolete expectation: remove a comment whose sole expectation is either a plain
+      // `// $ Alert` that no longer matches any result (a *missing result*), or a
+      // `// $ SPURIOUS: Alert` whose result no longer fires, so the known false positive is fixed
+      // (a *fixed spurious result*). In both cases the whole comment is deleted.
+      exists(Test::FailureLocatable expectation, string relativePath, int sl, int sc |
+        (
+          expectation instanceof Test::GoodTestExpectation or
+          expectation instanceof Test::FalsePositiveTestExpectation
+        ) and
         expectation.getTag() = "Alert" and
         expectation.getValue() = "" and
         not expectation = Test::getAMatchingExpectation(_, _, _, _, _) and
-        // the comment carries exactly this one expectation, so removing it wholesale is safe
-        count(Test::FailureLocatable other |
-          other.getLocation() = expectation.getLocation() and
-          (
-            other instanceof Test::GoodTestExpectation or
-            other instanceof Test::FalsePositiveTestExpectation or
-            other instanceof Test::FalseNegativeTestExpectation
-          )
-        ) = 1 and
-        not exists(Test::InvalidTestExpectation invalid |
-          invalid.getLocation() = expectation.getLocation()
-        ) and
+        isSoleExpectationOnComment(expectation) and
         parseLocationString(expectation.getLocation().getRelativeUrl(), relativePath, sl, sc, _, _) and
         file = relativePath and
         line = sl and
@@ -1127,11 +1160,32 @@ module TestPostProcessing {
         startColumn = sc and
         // A trailing inline expectation comment always runs to the end of its line, so delete from
         // the comment marker to the end of the line. `endColumn = 0` is the engine's "to end of
-        // line" convention; using it avoids depending on how each extractor reports a line
-        // comment's end column (e.g. Swift reports it as ending at column 1 of the next line).
+        // line" convention (it also trims the whitespace gap the removed comment leaves behind);
+        // using it avoids depending on how each extractor reports a line comment's end column
+        // (e.g. Swift reports it as ending at column 1 of the next line).
         endColumn = 0 and
         text = ""
       )
+      or
+      // Fixed missing result: a comment whose sole expectation is `// $ MISSING: Alert` now has a
+      // matching result, so promote it to a plain `// $ Alert`. The whole comment is rewritten in
+      // place, from its comment marker to the end of the line, with a freshly rendered plain
+      // comment. Rewriting wholesale is reliable for a single-expectation comment and avoids
+      // editing individual columns within the comment. The replacement text is non-empty, so
+      // unlike the removal case the whitespace before the comment is preserved.
+      exists(Test::FalseNegativeTestExpectation expectation, string relativePath, int sl, int sc |
+        expectation.getTag() = "Alert" and
+        expectation.getValue() = "" and
+        expectation = Test::getAMatchingExpectation(_, _, _, _, _) and
+        isSoleExpectationOnComment(expectation) and
+        parseLocationString(expectation.getLocation().getRelativeUrl(), relativePath, sl, sc, _, _) and
+        file = relativePath and
+        line = sl and
+        operation = "replace" and
+        startColumn = sc and
+        endColumn = 0 and
+        text = renderInlineComment(relativePath, "Alert")
+      )
     }
   }
 }

From d42007f3c32185a3515ded70c8cef194c74f59cd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nora=20Dimitrijevi=C4=87?= <d10c@users.noreply.github.com>
Date: Wed, 15 Jul 2026 15:31:22 +0200
Subject: [PATCH 4/9] Rewrite fully-owned expectation comments as a whole in
 learnEdits

`codeql test run --learn` previously handled inline expectations one
expectation at a time: it could append a new `// $ Alert`, delete a
comment whose *sole* expectation no longer matched, and promote a
comment whose sole expectation was `// $ MISSING: Alert`. Comments that
carried several expectations were left untouched, because the old code
had no way to edit one expectation among several without disturbing its
siblings.

Generalise the rewrite to operate on a comment as a whole. For each
comment the postprocess computes the set of expectations that survive
learning -- a default or `SPURIOUS:` expectation is kept only while it
still matches a result, a `MISSING:` expectation is promoted to the
default column once its result fires (and kept otherwise) -- and
re-renders the comment from that surviving set, grouped into the
default / `SPURIOUS:` / `MISSING:` columns with a deterministic layout.
If nothing survives, the comment is deleted (reusing the existing
`endColumn = 0` delete-to-end-of-line convention, which also trims the
whitespace the removed comment leaves behind). This subsumes the old
single-expectation removal and MISSING-promotion disjuncts.

Because a single query's postprocess only sees its own tags, the
rewrite is gated on `isFullyOwnedComment`: the comment must carry at
least one expectation this test understands, no expectation ignored via
a foreign query ID, and no unparseable expectation. This prevents a
`--learn` run for one query from silently dropping an expectation such
as `// $ Alert[other-query]` that belongs to a different query sharing
the same source file. Surgically editing such shared comments, and
merging a freshly learned tag into an existing comment rather than
appending a separate one, are handled in follow-up commits.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---
 .../util/test/InlineExpectationsTest.qll      | 234 +++++++++++++-----
 1 file changed, 170 insertions(+), 64 deletions(-)

diff --git a/shared/util/codeql/util/test/InlineExpectationsTest.qll b/shared/util/codeql/util/test/InlineExpectationsTest.qll
index d4d66efe80b3..cf8b06aea781 100644
--- a/shared/util/codeql/util/test/InlineExpectationsTest.qll
+++ b/shared/util/codeql/util/test/InlineExpectationsTest.qll
@@ -373,6 +373,27 @@ module Make<InlineExpectationsTestSig Impl> {
       )
     }
 
+    /**
+     * Holds if `location` is the location of an inline expectation comment that this test fully
+     * owns: it carries at least one expectation, none of its expectations is ignored by this test
+     * (for example via a query ID that does not match the current query), and none is unparseable.
+     *
+     * `codeql test run --learn` may rewrite such a comment as a whole. Comments that also carry an
+     * expectation this test cannot see must be left untouched, because rewriting them from only the
+     * expectations this test understands would silently drop the expectations belonging to another
+     * query that shares the same source file (for example `// $ Alert[query1] Alert[query2]`).
+     */
+    predicate isFullyOwnedComment(Impl::Location location) {
+      exists(Impl::ExpectationComment comment | comment.getLocation() = location |
+        exists(ValidTestExpectation owned | owned.getLocation() = location) and
+        not exists(string tags |
+          getAnExpectation(comment, _, _, tags, _) and
+          TestImpl::tagIsIgnored(tags.splitAt(","))
+        ) and
+        not exists(InvalidTestExpectation invalid | invalid.getLocation() = location)
+      )
+    }
+
     query predicate testFailures(FailureLocatable element, string message) {
       hasFailureMessage(element, message)
     }
@@ -1071,22 +1092,124 @@ module TestPostProcessing {
     }
 
     /**
-     * Holds if `expectation` is the only inline expectation carried by its comment, so `--learn`
-     * can rewrite or delete the comment as a whole without disturbing a sibling expectation.
-     * Comments that additionally carry an invalid (unparseable) expectation are excluded, to avoid
-     * corrupting text the library could not fully understand.
+     * Holds if, after `--learn`, the inline expectation comment at `commentLoc` should carry the
+     * expectation `text` in `column` (`""` for the default column, or `"SPURIOUS"` / `"MISSING"`).
+     *
+     * These are the expectations that survive learning: a default or `SPURIOUS:` expectation is
+     * kept only while it still matches a result, a `MISSING:` expectation whose result now fires is
+     * promoted to the default column, and a `MISSING:` expectation whose result is still absent is
+     * kept unchanged.
+     */
+    private predicate learnedExpectation(TestLocation commentLoc, string column, string text) {
+      exists(Test::FailureLocatable e |
+        e.getLocation() = commentLoc and text = e.getExpectationText()
+      |
+        // a default expectation that still matches a result is kept
+        e instanceof Test::GoodTestExpectation and
+        e = Test::getAMatchingExpectation(_, _, _, _, _) and
+        column = ""
+        or
+        // a `SPURIOUS:` expectation whose result still fires is kept
+        e instanceof Test::FalsePositiveTestExpectation and
+        e = Test::getAMatchingExpectation(_, _, _, _, _) and
+        column = "SPURIOUS"
+        or
+        // a `MISSING:` expectation whose result now fires is promoted to the default column
+        e instanceof Test::FalseNegativeTestExpectation and
+        e = Test::getAMatchingExpectation(_, _, _, _, _) and
+        column = ""
+        or
+        // a `MISSING:` expectation whose result is still absent is kept
+        e instanceof Test::FalseNegativeTestExpectation and
+        not e = Test::getAMatchingExpectation(_, _, _, _, _) and
+        column = "MISSING"
+      )
+    }
+
+    /**
+     * Holds if `column` (`""` for the default column, or `"SPURIOUS"` / `"MISSING"`) currently
+     * carries the expectation `text` on the comment at `commentLoc`.
      */
-    private predicate isSoleExpectationOnComment(Test::FailureLocatable expectation) {
-      count(Test::FailureLocatable other |
-        other.getLocation() = expectation.getLocation() and
+    private predicate currentExpectation(TestLocation commentLoc, string column, string text) {
+      exists(Test::FailureLocatable e |
+        e.getLocation() = commentLoc and text = e.getExpectationText()
+      |
+        e instanceof Test::GoodTestExpectation and column = ""
+        or
+        e instanceof Test::FalsePositiveTestExpectation and column = "SPURIOUS"
+        or
+        e instanceof Test::FalseNegativeTestExpectation and column = "MISSING"
+      )
+    }
+
+    /** Holds if `--learn` should change the set of expectations carried by the comment at `commentLoc`. */
+    private predicate commentNeedsRewrite(TestLocation commentLoc) {
+      exists(string column, string text |
+        learnedExpectation(commentLoc, column, text) and
+        not currentExpectation(commentLoc, column, text)
+      )
+      or
+      exists(string column, string text |
+        currentExpectation(commentLoc, column, text) and
+        not learnedExpectation(commentLoc, column, text)
+      )
+    }
+
+    /** Gets the rank that orders the default, `SPURIOUS:`, and `MISSING:` columns within a comment. */
+    private int getColumnRank(string column) {
+      column = "" and result = 0
+      or
+      column = "SPURIOUS" and result = 1
+      or
+      column = "MISSING" and result = 2
+    }
+
+    /**
+     * Gets the rendered text of `column` on the learned comment at `commentLoc`, for example
+     * `Alert Alert[foo]` for the default column or `MISSING: Alert` for the `MISSING:` column, or
+     * has no result if that column carries no expectation after learning. Expectations within a
+     * column are ordered lexically, so a rewritten comment has a deterministic layout.
+     */
+    private string renderLearnedColumn(TestLocation commentLoc, string column) {
+      exists(string text | learnedExpectation(commentLoc, column, text)) and
+      exists(string joined |
+        joined =
+          concat(string text |
+            learnedExpectation(commentLoc, column, text)
+          |
+            text, " " order by text
+          )
+      |
+        column = "" and result = joined
+        or
+        column != "" and result = column + ": " + joined
+      )
+    }
+
+    /**
+     * Gets the fully rendered inline expectation comment (including the comment markers, but with no
+     * leading whitespace) that `--learn` should leave in place of the comment at `commentLoc` in the
+     * file with the given `relativePath`. Has no result if no expectation survives learning (in
+     * which case the caller deletes the comment instead) or the file's comment syntax is
+     * unsupported.
+     */
+    bindingset[relativePath]
+    private string renderLearnedComment(string relativePath, TestLocation commentLoc) {
+      exists(string startMarker, string endMarker, string endSuffix, string body |
+        startMarker = Input2::getStartCommentMarker(relativePath) and
+        endMarker = Input2::getEndCommentMarker(relativePath) and
         (
-          other instanceof Test::GoodTestExpectation or
-          other instanceof Test::FalsePositiveTestExpectation or
-          other instanceof Test::FalseNegativeTestExpectation
-        )
-      ) = 1 and
-      not exists(Test::InvalidTestExpectation invalid |
-        invalid.getLocation() = expectation.getLocation()
+          endMarker = "" and endSuffix = ""
+          or
+          endMarker != "" and endSuffix = " " + endMarker
+        ) and
+        body =
+          concat(string column |
+            exists(renderLearnedColumn(commentLoc, column))
+          |
+            renderLearnedColumn(commentLoc, column), " " order by getColumnRank(column)
+          ) and
+        result = startMarker + " $ " + body + endSuffix
       )
     }
 
@@ -1100,19 +1223,22 @@ module TestPostProcessing {
      * - `"replace"`: replace the 1-based inclusive column range `[startColumn, endColumn]` with
      *   `text` (the empty string deletes the range).
      *
-     * Only a deliberately reliable subset is emitted so far, restricted to the plain `Alert`
-     * tag with no value or query-id annotation, on comments that carry a single expectation:
+     * The following edits are emitted:
      *
      * - an actual result with no matching expectation gets a new `// $ Alert` comment appended
-     *   (an *unexpected result*);
-     * - a comment whose sole expectation is a plain `// $ Alert` whose result is now missing, or a
-     *   `// $ SPURIOUS: Alert` whose result no longer fires, is removed (a *missing result* or a
-     *   *fixed spurious result*); and
-     * - a comment whose sole expectation is `// $ MISSING: Alert` whose result now appears is
-     *   promoted to a plain `// $ Alert` (a *fixed missing result*).
+     *   (an *unexpected result*); and
+     * - an existing expectation comment that this test fully owns is rewritten as a whole so that
+     *   it matches the current results: obsolete default and `// $ SPURIOUS:` expectations are
+     *   dropped (a *missing result* or a *fixed spurious result*), a `// $ MISSING:` expectation
+     *   whose result now fires is promoted to the default column (a *fixed missing result*), and
+     *   the surviving expectations are re-rendered. If nothing survives, the comment is deleted.
      *
-     * Cases that need sub-comment column information (editing one tag among several on a comment
-     * that carries multiple expectations) are intentionally left for a follow-up.
+     * The rewrite handles comments that carry several expectations across the default,
+     * `SPURIOUS:`, and `MISSING:` columns, but only when the test *fully owns* the comment (see
+     * `isFullyOwnedComment`), so it never discards an expectation belonging to a different query
+     * that shares the same source file. Appending a new tag by merging it into an existing comment,
+     * and surgically editing a comment that also carries a foreign query's expectation, are left
+     * for a follow-up.
      */
     query predicate learnEdits(
       string file, int line, string operation, int startColumn, int endColumn, string text
@@ -1140,51 +1266,31 @@ module TestPostProcessing {
         text = comment
       )
       or
-      // Obsolete expectation: remove a comment whose sole expectation is either a plain
-      // `// $ Alert` that no longer matches any result (a *missing result*), or a
-      // `// $ SPURIOUS: Alert` whose result no longer fires, so the known false positive is fixed
-      // (a *fixed spurious result*). In both cases the whole comment is deleted.
-      exists(Test::FailureLocatable expectation, string relativePath, int sl, int sc |
-        (
-          expectation instanceof Test::GoodTestExpectation or
-          expectation instanceof Test::FalsePositiveTestExpectation
-        ) and
-        expectation.getTag() = "Alert" and
-        expectation.getValue() = "" and
-        not expectation = Test::getAMatchingExpectation(_, _, _, _, _) and
-        isSoleExpectationOnComment(expectation) and
-        parseLocationString(expectation.getLocation().getRelativeUrl(), relativePath, sl, sc, _, _) and
+      // Rewrite an existing expectation comment as a whole so that it matches the current results.
+      // This subsumes the single-expectation removal and MISSING-promotion cases and additionally
+      // handles comments that carry several expectations across the default, `SPURIOUS:`, and
+      // `MISSING:` columns. The comment is replaced from its marker to the end of the line: with
+      // the re-rendered surviving expectations, or with the empty string when nothing survives (in
+      // which case `endColumn = 0` also trims the whitespace gap the removed comment leaves
+      // behind). `endColumn = 0` is the engine's "to end of line" convention, which avoids
+      // depending on how each extractor reports a line comment's end column (e.g. Swift reports it
+      // as ending at column 1 of the next line).
+      exists(TestLocation commentLoc, string relativePath, int sl, int sc |
+        Test::isFullyOwnedComment(commentLoc) and
+        commentNeedsRewrite(commentLoc) and
+        parseLocationString(commentLoc.getRelativeUrl(), relativePath, sl, sc, _, _) and
         file = relativePath and
         line = sl and
         operation = "replace" and
         startColumn = sc and
-        // A trailing inline expectation comment always runs to the end of its line, so delete from
-        // the comment marker to the end of the line. `endColumn = 0` is the engine's "to end of
-        // line" convention (it also trims the whitespace gap the removed comment leaves behind);
-        // using it avoids depending on how each extractor reports a line comment's end column
-        // (e.g. Swift reports it as ending at column 1 of the next line).
         endColumn = 0 and
-        text = ""
-      )
-      or
-      // Fixed missing result: a comment whose sole expectation is `// $ MISSING: Alert` now has a
-      // matching result, so promote it to a plain `// $ Alert`. The whole comment is rewritten in
-      // place, from its comment marker to the end of the line, with a freshly rendered plain
-      // comment. Rewriting wholesale is reliable for a single-expectation comment and avoids
-      // editing individual columns within the comment. The replacement text is non-empty, so
-      // unlike the removal case the whitespace before the comment is preserved.
-      exists(Test::FalseNegativeTestExpectation expectation, string relativePath, int sl, int sc |
-        expectation.getTag() = "Alert" and
-        expectation.getValue() = "" and
-        expectation = Test::getAMatchingExpectation(_, _, _, _, _) and
-        isSoleExpectationOnComment(expectation) and
-        parseLocationString(expectation.getLocation().getRelativeUrl(), relativePath, sl, sc, _, _) and
-        file = relativePath and
-        line = sl and
-        operation = "replace" and
-        startColumn = sc and
-        endColumn = 0 and
-        text = renderInlineComment(relativePath, "Alert")
+        (
+          exists(string column, string t | learnedExpectation(commentLoc, column, t)) and
+          text = renderLearnedComment(relativePath, commentLoc)
+          or
+          not exists(string column, string t | learnedExpectation(commentLoc, column, t)) and
+          text = ""
+        )
       )
     }
   }

From dc89e2d8e50629262521151d385844a89fea17d4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nora=20Dimitrijevi=C4=87?= <d10c@users.noreply.github.com>
Date: Wed, 15 Jul 2026 15:41:26 +0200
Subject: [PATCH 5/9] Preserve foreign expectations when --learn rewrites a
 comment

The whole-comment rewrite previously refused to touch any comment that
also carried an expectation this test ignores -- for example
`// $ Alert[other-query] Alert`, where `Alert[other-query]` is annotated
with a different query's ID and so is invisible to the current query's
postprocess. Rewriting such a comment from only the expectations this
test understands would have silently dropped the foreign one, so the
guard (`isFullyOwnedComment`) excluded it entirely, leaving even the
owned, stale part unfixable.

Make these comments surgically editable instead. `getAForeignExpectation`
exposes each ignored expectation's verbatim text and column, and the
rewrite now renders the union of the surviving owned expectations and
the preserved foreign ones. So `// $ Alert[other-query] Alert` on a line
that no longer fires becomes `// $ Alert[other-query]`: the owned `Alert`
is dropped while the other query's expectation is kept untouched.

The guard is renamed to `isRewritableComment` and relaxed accordingly:
a comment is rewritable when it has at least one parseable expectation,
none unparseable, and no comma-separated group that mixes an owned tag
with an ignored one (such a group, e.g. `Alert,Source[other-query]`,
would need to be split apart and is left untouched). Genuinely
concurrent edits from two different queries' postprocess runs targeting
the same comment remain out of scope; that needs the engine to
reconcile edits and is not addressed here.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---
 .../util/test/InlineExpectationsTest.qll      | 103 +++++++++++++-----
 1 file changed, 75 insertions(+), 28 deletions(-)

diff --git a/shared/util/codeql/util/test/InlineExpectationsTest.qll b/shared/util/codeql/util/test/InlineExpectationsTest.qll
index cf8b06aea781..0c85c6d82714 100644
--- a/shared/util/codeql/util/test/InlineExpectationsTest.qll
+++ b/shared/util/codeql/util/test/InlineExpectationsTest.qll
@@ -374,23 +374,51 @@ module Make<InlineExpectationsTestSig Impl> {
     }
 
     /**
-     * Holds if `location` is the location of an inline expectation comment that this test fully
-     * owns: it carries at least one expectation, none of its expectations is ignored by this test
-     * (for example via a query ID that does not match the current query), and none is unparseable.
+     * Holds if `location` is the location of an inline expectation comment that
+     * `codeql test run --learn` may rewrite as a whole.
      *
-     * `codeql test run --learn` may rewrite such a comment as a whole. Comments that also carry an
-     * expectation this test cannot see must be left untouched, because rewriting them from only the
-     * expectations this test understands would silently drop the expectations belonging to another
-     * query that shares the same source file (for example `// $ Alert[query1] Alert[query2]`).
+     * A comment is rewritable when it carries at least one parseable expectation, none of its
+     * expectations is unparseable, and none mixes (in a single comma-separated group) a tag this
+     * test understands with a tag it ignores (for example a query ID that does not match the
+     * current query). The last condition keeps the rewrite from having to take apart a group such
+     * as `Alert,Source[other-query]` whose parts this test treats differently; such comments are
+     * left untouched.
+     *
+     * A rewritable comment may still carry whole expectations this test ignores (for example
+     * `// $ Alert[other-query]`). Those are preserved verbatim by `hasForeignExpectation` when the
+     * comment is rewritten, so that a `--learn` run for one query never drops an expectation that
+     * belongs to a different query sharing the same source file.
      */
-    predicate isFullyOwnedComment(Impl::Location location) {
+    predicate isRewritableComment(Impl::Location location) {
       exists(Impl::ExpectationComment comment | comment.getLocation() = location |
-        exists(ValidTestExpectation owned | owned.getLocation() = location) and
-        not exists(string tags |
+        getAnExpectation(comment, _, _, _, _) and
+        not exists(InvalidTestExpectation invalid | invalid.getLocation() = location) and
+        not exists(string tags, string owned, string ignored |
           getAnExpectation(comment, _, _, tags, _) and
-          TestImpl::tagIsIgnored(tags.splitAt(","))
-        ) and
-        not exists(InvalidTestExpectation invalid | invalid.getLocation() = location)
+          owned = tags.splitAt(",") and
+          not TestImpl::tagIsIgnored(owned) and
+          ignored = tags.splitAt(",") and
+          TestImpl::tagIsIgnored(ignored)
+        )
+      )
+    }
+
+    /**
+     * Holds if the comment at `location` carries a whole expectation this test ignores (for
+     * example one annotated with a query ID that does not match the current query), whose verbatim
+     * text is `text` and which sits in `column` (`""` for the default column, or a named column
+     * such as `"SPURIOUS"` / `"MISSING"`).
+     *
+     * `codeql test run --learn` preserves such expectations unchanged when it rewrites the comment,
+     * because they belong to a different query that shares the same source file and this test
+     * cannot tell whether they still hold.
+     */
+    predicate hasForeignExpectation(Impl::Location location, string column, string text) {
+      exists(Impl::ExpectationComment comment, TColumn col, string tags |
+        comment.getLocation() = location and
+        getAnExpectation(comment, col, text, tags, _) and
+        column = getColumnString(col) and
+        forall(string tag | tag = tags.splitAt(",") | TestImpl::tagIsIgnored(tag))
       )
     }
 
@@ -1127,8 +1155,25 @@ module TestPostProcessing {
     }
 
     /**
-     * Holds if `column` (`""` for the default column, or `"SPURIOUS"` / `"MISSING"`) currently
-     * carries the expectation `text` on the comment at `commentLoc`.
+     * Holds if, after `--learn`, the inline expectation comment at `commentLoc` should carry the
+     * expectation `text` in `column` (`""` for the default column, or a named column such as
+     * `"SPURIOUS"` / `"MISSING"`).
+     *
+     * This combines the surviving expectations this test understands (see `learnedExpectation`)
+     * with the expectations it ignores (see `Test::hasForeignExpectation`), which are preserved
+     * verbatim so that rewriting a comment for one query never drops another query's expectation on
+     * the same line.
+     */
+    private predicate desiredExpectation(TestLocation commentLoc, string column, string text) {
+      learnedExpectation(commentLoc, column, text)
+      or
+      Test::hasForeignExpectation(commentLoc, column, text)
+    }
+
+    /**
+     * Holds if `column` (`""` for the default column, or a named column such as `"SPURIOUS"` /
+     * `"MISSING"`) currently carries the expectation `text` on the comment at `commentLoc`. This
+     * includes expectations this test ignores, so it can be compared against `desiredExpectation`.
      */
     private predicate currentExpectation(TestLocation commentLoc, string column, string text) {
       exists(Test::FailureLocatable e |
@@ -1140,18 +1185,20 @@ module TestPostProcessing {
         or
         e instanceof Test::FalseNegativeTestExpectation and column = "MISSING"
       )
+      or
+      Test::hasForeignExpectation(commentLoc, column, text)
     }
 
     /** Holds if `--learn` should change the set of expectations carried by the comment at `commentLoc`. */
     private predicate commentNeedsRewrite(TestLocation commentLoc) {
       exists(string column, string text |
-        learnedExpectation(commentLoc, column, text) and
+        desiredExpectation(commentLoc, column, text) and
         not currentExpectation(commentLoc, column, text)
       )
       or
       exists(string column, string text |
         currentExpectation(commentLoc, column, text) and
-        not learnedExpectation(commentLoc, column, text)
+        not desiredExpectation(commentLoc, column, text)
       )
     }
 
@@ -1171,11 +1218,11 @@ module TestPostProcessing {
      * column are ordered lexically, so a rewritten comment has a deterministic layout.
      */
     private string renderLearnedColumn(TestLocation commentLoc, string column) {
-      exists(string text | learnedExpectation(commentLoc, column, text)) and
+      desiredExpectation(commentLoc, column, _) and
       exists(string joined |
         joined =
           concat(string text |
-            learnedExpectation(commentLoc, column, text)
+            desiredExpectation(commentLoc, column, text)
           |
             text, " " order by text
           )
@@ -1234,11 +1281,11 @@ module TestPostProcessing {
      *   the surviving expectations are re-rendered. If nothing survives, the comment is deleted.
      *
      * The rewrite handles comments that carry several expectations across the default,
-     * `SPURIOUS:`, and `MISSING:` columns, but only when the test *fully owns* the comment (see
-     * `isFullyOwnedComment`), so it never discards an expectation belonging to a different query
-     * that shares the same source file. Appending a new tag by merging it into an existing comment,
-     * and surgically editing a comment that also carries a foreign query's expectation, are left
-     * for a follow-up.
+     * `SPURIOUS:`, and `MISSING:` columns. Expectations this test ignores (for example a tag
+     * annotated with a different query's ID) are preserved verbatim, so the comment keeps any
+     * expectation belonging to a different query that shares the same source file; see
+     * `isRewritableComment` and `Test::hasForeignExpectation`. Appending a new tag by merging it
+     * into an existing comment rather than adding a separate one is left for a follow-up.
      */
     query predicate learnEdits(
       string file, int line, string operation, int startColumn, int endColumn, string text
@@ -1270,13 +1317,13 @@ module TestPostProcessing {
       // This subsumes the single-expectation removal and MISSING-promotion cases and additionally
       // handles comments that carry several expectations across the default, `SPURIOUS:`, and
       // `MISSING:` columns. The comment is replaced from its marker to the end of the line: with
-      // the re-rendered surviving expectations, or with the empty string when nothing survives (in
+      // the re-rendered desired expectations, or with the empty string when none remains (in
       // which case `endColumn = 0` also trims the whitespace gap the removed comment leaves
       // behind). `endColumn = 0` is the engine's "to end of line" convention, which avoids
       // depending on how each extractor reports a line comment's end column (e.g. Swift reports it
       // as ending at column 1 of the next line).
       exists(TestLocation commentLoc, string relativePath, int sl, int sc |
-        Test::isFullyOwnedComment(commentLoc) and
+        Test::isRewritableComment(commentLoc) and
         commentNeedsRewrite(commentLoc) and
         parseLocationString(commentLoc.getRelativeUrl(), relativePath, sl, sc, _, _) and
         file = relativePath and
@@ -1285,10 +1332,10 @@ module TestPostProcessing {
         startColumn = sc and
         endColumn = 0 and
         (
-          exists(string column, string t | learnedExpectation(commentLoc, column, t)) and
+          desiredExpectation(commentLoc, _, _) and
           text = renderLearnedComment(relativePath, commentLoc)
           or
-          not exists(string column, string t | learnedExpectation(commentLoc, column, t)) and
+          not desiredExpectation(commentLoc, _, _) and
           text = ""
         )
       )

From cd975f5c231210bd9cd844f463a242f925feb93d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nora=20Dimitrijevi=C4=87?= <d10c@users.noreply.github.com>
Date: Wed, 15 Jul 2026 15:46:49 +0200
Subject: [PATCH 6/9] Merge a learned tag into an existing comment on the same
 line

When `--learn` discovers an unexpected `Alert` result on a line that already
carries a rewritable expectation comment, fold the new tag into that comment
instead of appending a second `// $ ...` comment beside it. This keeps one
comment per line and, in particular, lets a freshly learned tag join an
expectation that belongs to a different query sharing the source file (e.g.
`// $ Alert[q/other]` becomes `// $ Alert Alert[q/other]`).

`mergedNewTag` feeds the learned default-column tag into `desiredExpectation`,
so the existing whole-comment rewrite renders it alongside the surviving and
foreign expectations. The append disjunct in `learnEdits` is now gated on there
being no rewritable comment on the result's line, so append and merge are
mutually exclusive: a line with a rewritable comment is always merged, never
double-commented.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---
 .../util/test/InlineExpectationsTest.qll      | 73 +++++++++++++++----
 1 file changed, 58 insertions(+), 15 deletions(-)

diff --git a/shared/util/codeql/util/test/InlineExpectationsTest.qll b/shared/util/codeql/util/test/InlineExpectationsTest.qll
index 0c85c6d82714..46b09f7682e7 100644
--- a/shared/util/codeql/util/test/InlineExpectationsTest.qll
+++ b/shared/util/codeql/util/test/InlineExpectationsTest.qll
@@ -1154,6 +1154,40 @@ module TestPostProcessing {
       )
     }
 
+    /**
+     * Holds if some non-optional `Alert` result with no matching expectation fires on `line` of
+     * `relativePath` -- an *unexpected result* that `--learn` should record with a new `Alert`
+     * expectation, either by appending a fresh comment or by merging into an existing one.
+     */
+    private predicate hasUnexpectedAlertOnLine(string relativePath, int line) {
+      exists(Test::ActualTestResult actualResult |
+        actualResult.getTag() = "Alert" and
+        actualResult.getValue() = "" and
+        not actualResult.isOptional() and
+        not exists(
+          Test::getAMatchingExpectation(actualResult.getLocation(), actualResult.toString(),
+            actualResult.getTag(), actualResult.getValue(), false)
+        ) and
+        parseLocationString(actualResult.getLocation().getRelativeUrl(), relativePath, _, _, line, _)
+      )
+    }
+
+    /**
+     * Holds if `--learn` should merge a freshly learned `Alert` expectation into the existing,
+     * rewritable comment at `commentLoc` (in `column` `""`, the default), because an unexpected
+     * `Alert` result fires on that comment's line. Merging keeps the new tag alongside the
+     * comment's existing expectations rather than appending a second comment to the line.
+     */
+    private predicate mergedNewTag(TestLocation commentLoc, string column, string text) {
+      exists(string relativePath, int line |
+        Test::isRewritableComment(commentLoc) and
+        parseLocationString(commentLoc.getRelativeUrl(), relativePath, line, _, _, _) and
+        hasUnexpectedAlertOnLine(relativePath, line) and
+        column = "" and
+        text = "Alert"
+      )
+    }
+
     /**
      * Holds if, after `--learn`, the inline expectation comment at `commentLoc` should carry the
      * expectation `text` in `column` (`""` for the default column, or a named column such as
@@ -1162,12 +1196,14 @@ module TestPostProcessing {
      * This combines the surviving expectations this test understands (see `learnedExpectation`)
      * with the expectations it ignores (see `Test::hasForeignExpectation`), which are preserved
      * verbatim so that rewriting a comment for one query never drops another query's expectation on
-     * the same line.
+     * the same line, and with any freshly learned tag merged into the comment (see `mergedNewTag`).
      */
     private predicate desiredExpectation(TestLocation commentLoc, string column, string text) {
       learnedExpectation(commentLoc, column, text)
       or
       Test::hasForeignExpectation(commentLoc, column, text)
+      or
+      mergedNewTag(commentLoc, column, text)
     }
 
     /**
@@ -1272,29 +1308,32 @@ module TestPostProcessing {
      *
      * The following edits are emitted:
      *
-     * - an actual result with no matching expectation gets a new `// $ Alert` comment appended
-     *   (an *unexpected result*); and
-     * - an existing expectation comment that this test fully owns is rewritten as a whole so that
-     *   it matches the current results: obsolete default and `// $ SPURIOUS:` expectations are
-     *   dropped (a *missing result* or a *fixed spurious result*), a `// $ MISSING:` expectation
-     *   whose result now fires is promoted to the default column (a *fixed missing result*), and
-     *   the surviving expectations are re-rendered. If nothing survives, the comment is deleted.
+     * - an actual result with no matching expectation records a new `Alert` expectation (an
+     *   *unexpected result*): if the result's line already has a rewritable comment the tag is
+     *   merged into it (see below), otherwise a fresh `// $ Alert` comment is appended; and
+     * - an existing rewritable expectation comment is rewritten as a whole so that it matches the
+     *   current results: obsolete default and `// $ SPURIOUS:` expectations are dropped (a *missing
+     *   result* or a *fixed spurious result*), a `// $ MISSING:` expectation whose result now fires
+     *   is promoted to the default column (a *fixed missing result*), a freshly learned tag on the
+     *   line is merged in, and the resulting expectations are re-rendered. If nothing remains, the
+     *   comment is deleted.
      *
      * The rewrite handles comments that carry several expectations across the default,
      * `SPURIOUS:`, and `MISSING:` columns. Expectations this test ignores (for example a tag
      * annotated with a different query's ID) are preserved verbatim, so the comment keeps any
      * expectation belonging to a different query that shares the same source file; see
-     * `isRewritableComment` and `Test::hasForeignExpectation`. Appending a new tag by merging it
-     * into an existing comment rather than adding a separate one is left for a follow-up.
+     * `isRewritableComment` and `Test::hasForeignExpectation`.
      */
     query predicate learnEdits(
       string file, int line, string operation, int startColumn, int endColumn, string text
     ) {
-      // Unexpected result: append a new `// $ Alert` comment on the alert's line. The comment
-      // must go on the result's *end* line, because an expectation matches a result when the
-      // expectation's start line equals the result's end line (see `onSameLine`). For most
-      // languages a result spans a single line, but some (e.g. Rust) include leading trivia in
-      // the location, so the start and end lines differ.
+      // Unexpected result with no comment to merge into: append a new `// $ Alert` comment on the
+      // alert's line. The comment must go on the result's *end* line, because an expectation
+      // matches a result when the expectation's start line equals the result's end line (see
+      // `onSameLine`). For most languages a result spans a single line, but some (e.g. Rust)
+      // include leading trivia in the location, so the start and end lines differ. If the line
+      // already has a rewritable comment, the tag is merged into it by the rewrite disjunct below
+      // (see `mergedNewTag`) rather than appended as a separate comment.
       exists(Test::ActualTestResult actualResult, string relativePath, int el, string comment |
         actualResult.getTag() = "Alert" and
         actualResult.getValue() = "" and
@@ -1304,6 +1343,10 @@ module TestPostProcessing {
             actualResult.getTag(), actualResult.getValue(), false)
         ) and
         parseLocationString(actualResult.getLocation().getRelativeUrl(), relativePath, _, _, el, _) and
+        not exists(TestLocation existing |
+          Test::isRewritableComment(existing) and
+          parseLocationString(existing.getRelativeUrl(), relativePath, el, _, _, _)
+        ) and
         comment = renderExpectationComment(relativePath, "Alert") and
         file = relativePath and
         line = el and

From 053f023d506ed9b15bf009578441278667b05a96 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nora=20Dimitrijevi=C4=87?= <d10c@users.noreply.github.com>
Date: Wed, 15 Jul 2026 16:53:32 +0200
Subject: [PATCH 7/9] Preserve a trailing regular comment when --learn rewrites
 a comment

An inline expectation comment may end with a trailing regular (non-interpreted)
comment after the expectations, e.g. `// $ Alert // explanatory note`. The
parser (see `expectationCommentPattern`) ends the expectation region at the
first `//`, so everything from there is an ordinary comment. Until now `--learn`
replaced an expectation comment from its marker to the end of the line, which
discarded that trailing note whenever the comment was rewritten or deleted.

Add `Test::getTrailingComment`, which extracts the trailing `// ...` using the
same `(?:[^/]|/[^/])*` boundary the parser uses, and thread it through the
rewrite: `renderLearnedComment` re-appends it after the re-rendered
expectations, and the "nothing survives" branch of `learnEdits` replaces the
comment with just the trailing note (rather than the empty string) so the note
is kept in place. Only `//` starts such a trailing comment -- `#` never ends the
expectation region -- so in practice this benefits `//`-comment languages.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---
 .../util/test/InlineExpectationsTest.qll      | 58 ++++++++++++++++---
 1 file changed, 49 insertions(+), 9 deletions(-)

diff --git a/shared/util/codeql/util/test/InlineExpectationsTest.qll b/shared/util/codeql/util/test/InlineExpectationsTest.qll
index 46b09f7682e7..e7e983bfa819 100644
--- a/shared/util/codeql/util/test/InlineExpectationsTest.qll
+++ b/shared/util/codeql/util/test/InlineExpectationsTest.qll
@@ -422,6 +422,25 @@ module Make<InlineExpectationsTestSig Impl> {
       )
     }
 
+    /**
+     * Holds if the expectation comment at `location` ends with a trailing regular (non-interpreted)
+     * comment `text`, that is a `//` sequence after the expectations whose remainder the framework
+     * treats as an ordinary comment (see `expectationCommentPattern`). `text` includes the trailing
+     * comment's own `//` marker, for example `// note`.
+     *
+     * `codeql test run --learn` keeps this trailing comment when it rewrites or deletes the
+     * expectation comment, so an explanatory note written next to an expectation is not lost. Only
+     * `//` starts such a trailing comment, mirroring `expectationCommentPattern`'s
+     * `(?:[^/]|/[^/])*` expectation region, which ends only at `//`; a `#` never does, so this is in
+     * practice a feature of `//`-comment languages.
+     */
+    predicate getTrailingComment(Impl::Location location, string text) {
+      exists(Impl::ExpectationComment comment |
+        comment.getLocation() = location and
+        text = comment.getContents().regexpCapture("\\s*\\$ (?:[^/]|/[^/])*(//.*)", 1)
+      )
+    }
+
     query predicate testFailures(FailureLocatable element, string message) {
       hasFailureMessage(element, message)
     }
@@ -1278,7 +1297,9 @@ module TestPostProcessing {
      */
     bindingset[relativePath]
     private string renderLearnedComment(string relativePath, TestLocation commentLoc) {
-      exists(string startMarker, string endMarker, string endSuffix, string body |
+      exists(
+        string startMarker, string endMarker, string endSuffix, string body, string trailingSuffix
+      |
         startMarker = Input2::getStartCommentMarker(relativePath) and
         endMarker = Input2::getEndCommentMarker(relativePath) and
         (
@@ -1286,13 +1307,22 @@ module TestPostProcessing {
           or
           endMarker != "" and endSuffix = " " + endMarker
         ) and
+        // Preserve a trailing regular comment (e.g. `// $ Alert // note`) that sits after the
+        // expectations, so rewriting the expectations never drops an explanatory note.
+        (
+          exists(string trailing |
+            Test::getTrailingComment(commentLoc, trailing) and trailingSuffix = " " + trailing
+          )
+          or
+          not Test::getTrailingComment(commentLoc, _) and trailingSuffix = ""
+        ) and
         body =
           concat(string column |
             exists(renderLearnedColumn(commentLoc, column))
           |
             renderLearnedColumn(commentLoc, column), " " order by getColumnRank(column)
           ) and
-        result = startMarker + " $ " + body + endSuffix
+        result = startMarker + " $ " + body + trailingSuffix + endSuffix
       )
     }
 
@@ -1322,7 +1352,10 @@ module TestPostProcessing {
      * `SPURIOUS:`, and `MISSING:` columns. Expectations this test ignores (for example a tag
      * annotated with a different query's ID) are preserved verbatim, so the comment keeps any
      * expectation belonging to a different query that shares the same source file; see
-     * `isRewritableComment` and `Test::hasForeignExpectation`.
+     * `isRewritableComment` and `Test::hasForeignExpectation`. A trailing regular comment after
+     * the expectations (for example the ` // note` in `// $ Alert // note`) is likewise preserved;
+     * see `Test::getTrailingComment`. Only `//` starts such a trailing comment, so this is in
+     * practice a feature of `//`-comment languages.
      */
     query predicate learnEdits(
       string file, int line, string operation, int startColumn, int endColumn, string text
@@ -1360,11 +1393,12 @@ module TestPostProcessing {
       // This subsumes the single-expectation removal and MISSING-promotion cases and additionally
       // handles comments that carry several expectations across the default, `SPURIOUS:`, and
       // `MISSING:` columns. The comment is replaced from its marker to the end of the line: with
-      // the re-rendered desired expectations, or with the empty string when none remains (in
-      // which case `endColumn = 0` also trims the whitespace gap the removed comment leaves
-      // behind). `endColumn = 0` is the engine's "to end of line" convention, which avoids
-      // depending on how each extractor reports a line comment's end column (e.g. Swift reports it
-      // as ending at column 1 of the next line).
+      // the re-rendered desired expectations (keeping any trailing regular comment), with just the
+      // trailing regular comment when no expectation remains but a note like `// $ Alert // note`
+      // does, or with the empty string when nothing remains (in which case `endColumn = 0` also
+      // trims the whitespace gap the removed comment leaves behind). `endColumn = 0` is the
+      // engine's "to end of line" convention, which avoids depending on how each extractor reports
+      // a line comment's end column (e.g. Swift reports it as ending at column 1 of the next line).
       exists(TestLocation commentLoc, string relativePath, int sl, int sc |
         Test::isRewritableComment(commentLoc) and
         commentNeedsRewrite(commentLoc) and
@@ -1379,7 +1413,13 @@ module TestPostProcessing {
           text = renderLearnedComment(relativePath, commentLoc)
           or
           not desiredExpectation(commentLoc, _, _) and
-          text = ""
+          // No expectation survives, so drop the `$ ...` comment. If it carried a trailing regular
+          // comment, keep that in place of the whole comment; otherwise delete to the end of line.
+          (
+            Test::getTrailingComment(commentLoc, text)
+            or
+            not Test::getTrailingComment(commentLoc, _) and text = ""
+          )
         )
       )
     }

From 76357e8e6e4466b81e446dda2710f9de74c065f1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nora=20Dimitrijevi=C4=87?= <d10c@users.noreply.github.com>
Date: Wed, 15 Jul 2026 17:20:03 +0200
Subject: [PATCH 8/9] Mix learned tags with plain comments and re-wrap trailing
 notes by marker

`codeql test run --learn` could already keep a trailing regular note when
rewriting an expectation comment (`// $ Alert // note`), but only for
`//`-comment languages: the note was carried verbatim *including* its `//`
marker, so deleting the last expectation from a `#`-comment file would have
produced an invalid `// note` line rather than `# note`, and a plain comment
carrying no expectation at all was never a merge target.

Generalise trailing-note handling so it is marker-aware and works for every
line-comment language:

- Rename `getTrailingComment` to `getTrailingNote` and return the note's
  *content*, with its own comment marker and surrounding whitespace stripped.
  It now also matches a plain comment that carries no expectation of its own
  (`// note`, `# note`), whose whole content is the note.
- When re-rendering a comment, the note is re-wrapped with the inner `//`
  delimiter the framework recognises regardless of the outer marker, so a
  `#`-comment file renders `# $ Alert // note`.
- When the last expectation is removed but a note survives, re-wrap the note in
  the file's own start marker (`# note`, not the invalid `// note`).
- Broaden `isRewritableComment` to accept a plain comment carrying only a note,
  so an unexpected result on that line merges `Alert` into the comment
  (`// note` -> `// $ Alert // note`) instead of appending a second comment.

`isRewritableComment` is only consulted on the `--learn` path, so ordinary
`codeql test run` output is unchanged.
---
 .../util/test/InlineExpectationsTest.qll      | 90 ++++++++++++-------
 1 file changed, 58 insertions(+), 32 deletions(-)

diff --git a/shared/util/codeql/util/test/InlineExpectationsTest.qll b/shared/util/codeql/util/test/InlineExpectationsTest.qll
index e7e983bfa819..44fd49c82915 100644
--- a/shared/util/codeql/util/test/InlineExpectationsTest.qll
+++ b/shared/util/codeql/util/test/InlineExpectationsTest.qll
@@ -374,12 +374,14 @@ module Make<InlineExpectationsTestSig Impl> {
     }
 
     /**
-     * Holds if `location` is the location of an inline expectation comment that
+     * Holds if `location` is the location of a comment that
      * `codeql test run --learn` may rewrite as a whole.
      *
-     * A comment is rewritable when it carries at least one parseable expectation, none of its
-     * expectations is unparseable, and none mixes (in a single comma-separated group) a tag this
-     * test understands with a tag it ignores (for example a query ID that does not match the
+     * A comment is rewritable when it either carries at least one parseable expectation, or is a
+     * plain comment carrying only a trailing note (see `getTrailingNote`) that a freshly learned
+     * tag can be merged into (for example `// note` -> `// $ Alert // note`); and in addition none
+     * of its expectations is unparseable, and none mixes (in a single comma-separated group) a tag
+     * this test understands with a tag it ignores (for example a query ID that does not match the
      * current query). The last condition keeps the rewrite from having to take apart a group such
      * as `Alert,Source[other-query]` whose parts this test treats differently; such comments are
      * left untouched.
@@ -391,7 +393,14 @@ module Make<InlineExpectationsTestSig Impl> {
      */
     predicate isRewritableComment(Impl::Location location) {
       exists(Impl::ExpectationComment comment | comment.getLocation() = location |
-        getAnExpectation(comment, _, _, _, _) and
+        (
+          getAnExpectation(comment, _, _, _, _)
+          or
+          // A plain comment carrying only a note (no expectation of its own) is rewritable too, so
+          // that a freshly learned tag can be merged into it rather than appended as a second
+          // comment (for example `// note` -> `// $ Alert // note`).
+          exists(getTrailingNote(location)) and not getAnExpectation(comment, _, _, _, _)
+        ) and
         not exists(InvalidTestExpectation invalid | invalid.getLocation() = location) and
         not exists(string tags, string owned, string ignored |
           getAnExpectation(comment, _, _, tags, _) and
@@ -423,22 +432,32 @@ module Make<InlineExpectationsTestSig Impl> {
     }
 
     /**
-     * Holds if the expectation comment at `location` ends with a trailing regular (non-interpreted)
-     * comment `text`, that is a `//` sequence after the expectations whose remainder the framework
-     * treats as an ordinary comment (see `expectationCommentPattern`). `text` includes the trailing
-     * comment's own `//` marker, for example `// note`.
+     * Holds if the comment at `location` carries a trailing regular (non-interpreted) note `note`,
+     * with the note's own comment marker and surrounding whitespace stripped. This is either:
      *
-     * `codeql test run --learn` keeps this trailing comment when it rewrites or deletes the
-     * expectation comment, so an explanatory note written next to an expectation is not lost. Only
-     * `//` starts such a trailing comment, mirroring `expectationCommentPattern`'s
-     * `(?:[^/]|/[^/])*` expectation region, which ends only at `//`; a `#` never does, so this is in
-     * practice a feature of `//`-comment languages.
+     * - the text after a `//` that follows the expectations in an expectation comment (for example
+     *   `note` in `// $ Alert // note` or `# $ Alert // note`), which the framework treats as an
+     *   ordinary comment (see `expectationCommentPattern`); or
+     * - the whole content of a plain comment that carries no expectation at all (for example `note`
+     *   in `// note` or `# note`), into which `--learn` may merge a freshly learned tag.
+     *
+     * `codeql test run --learn` keeps this note when it rewrites, deletes, or merges into the
+     * comment, re-wrapping it with the appropriate markers (`<marker> $ ... // note` when
+     * expectations remain, or `<marker> note` when none do), so an explanatory note written next to
+     * code is never lost. Only `//` delimits such a note within an expectation comment, mirroring
+     * `expectationCommentPattern`'s `(?:[^/]|/[^/])*` expectation region, which ends only at `//`; a
+     * `#` never does, so `# $ Alert # note` reads `note` as a tag rather than a note.
      */
-    predicate getTrailingComment(Impl::Location location, string text) {
-      exists(Impl::ExpectationComment comment |
-        comment.getLocation() = location and
-        text = comment.getContents().regexpCapture("\\s*\\$ (?:[^/]|/[^/])*(//.*)", 1)
-      )
+    string getTrailingNote(Impl::Location location) {
+      exists(Impl::ExpectationComment comment | comment.getLocation() = location |
+        result = comment.getContents().regexpCapture("\\s*\\$ (?:[^/]|/[^/])*//(.*)", 1).trim()
+        or
+        // A plain comment with no expectation of its own: its whole content is the note.
+        not getAnExpectation(comment, _, _, _, _) and
+        not exists(InvalidTestExpectation invalid | invalid.getLocation() = location) and
+        result = comment.getContents().trim()
+      ) and
+      result != ""
     }
 
     query predicate testFailures(FailureLocatable element, string message) {
@@ -1307,14 +1326,16 @@ module TestPostProcessing {
           or
           endMarker != "" and endSuffix = " " + endMarker
         ) and
-        // Preserve a trailing regular comment (e.g. `// $ Alert // note`) that sits after the
-        // expectations, so rewriting the expectations never drops an explanatory note.
+        // Preserve a trailing regular note (e.g. the `note` in `// $ Alert // note`) that sits
+        // after the expectations, so rewriting the expectations never drops an explanatory note.
+        // The inner delimiter is always `//`, which the framework recognises regardless of the
+        // outer comment marker (so a `#`-comment file renders `# $ Alert // note`).
         (
-          exists(string trailing |
-            Test::getTrailingComment(commentLoc, trailing) and trailingSuffix = " " + trailing
+          exists(string note |
+            note = Test::getTrailingNote(commentLoc) and trailingSuffix = " // " + note
           )
           or
-          not Test::getTrailingComment(commentLoc, _) and trailingSuffix = ""
+          not exists(Test::getTrailingNote(commentLoc)) and trailingSuffix = ""
         ) and
         body =
           concat(string column |
@@ -1352,10 +1373,10 @@ module TestPostProcessing {
      * `SPURIOUS:`, and `MISSING:` columns. Expectations this test ignores (for example a tag
      * annotated with a different query's ID) are preserved verbatim, so the comment keeps any
      * expectation belonging to a different query that shares the same source file; see
-     * `isRewritableComment` and `Test::hasForeignExpectation`. A trailing regular comment after
-     * the expectations (for example the ` // note` in `// $ Alert // note`) is likewise preserved;
-     * see `Test::getTrailingComment`. Only `//` starts such a trailing comment, so this is in
-     * practice a feature of `//`-comment languages.
+     * `isRewritableComment` and `Test::hasForeignExpectation`. A trailing regular note after the
+     * expectations (for example the `note` in `// $ Alert // note` or `# $ Alert // note`) is
+     * likewise preserved, and a freshly learned tag can be merged into a plain comment that carries
+     * only such a note (`// note` -> `// $ Alert // note`); see `Test::getTrailingNote`.
      */
     query predicate learnEdits(
       string file, int line, string operation, int startColumn, int endColumn, string text
@@ -1413,12 +1434,17 @@ module TestPostProcessing {
           text = renderLearnedComment(relativePath, commentLoc)
           or
           not desiredExpectation(commentLoc, _, _) and
-          // No expectation survives, so drop the `$ ...` comment. If it carried a trailing regular
-          // comment, keep that in place of the whole comment; otherwise delete to the end of line.
+          // No expectation survives, so drop the `$ ...` part of the comment. If it carried a
+          // trailing regular note, keep that note re-wrapped in the file's own comment marker (so a
+          // `#`-comment file yields `# note`, not an invalid `// note`); otherwise delete to the
+          // end of line.
           (
-            Test::getTrailingComment(commentLoc, text)
+            exists(string note |
+              note = Test::getTrailingNote(commentLoc) and
+              text = Input2::getStartCommentMarker(relativePath) + " " + note
+            )
             or
-            not Test::getTrailingComment(commentLoc, _) and text = ""
+            not exists(Test::getTrailingNote(commentLoc)) and text = ""
           )
         )
       )

From fdb5a2e62dcfe905c69431116fc9f9d0809ca46d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nora=20Dimitrijevi=C4=87?= <d10c@users.noreply.github.com>
Date: Wed, 15 Jul 2026 18:52:18 +0200
Subject: [PATCH 9/9] Make --learn record any unexpected result, not just Alert

`codeql test run --learn` learns inline expectations from a
`@kind test-postprocess` query, but until now the *append* path -- the
one that introduces a brand-new expectation on a line that has none, or
merges one into an existing comment -- only ever recorded the built-in
`Alert` tag. Column surgery on an existing comment (dropping a stale
tag, promoting `MISSING:`, re-rendering siblings) was already
tag-generic, but a result carrying any other tag could never be learned
from scratch. That left the headline path-problem case unhandled: a
path-problem reports its source with a `Source` tag on the source line
(distinct from the alert line), so learning could never add the
`// $ Source` a passing test needs.

Replace the `Alert`-only `hasUnexpectedAlertOnLine` with a generic
`learnedNewExpectation(relativePath, endLine, text)`: for every
non-optional actual result with no matching expectation it records the
result's fully rendered expectation text (`getExpectationText`, so a
value is carried as `tag=value`), keyed on the result's end line (an
expectation matches a result when its start line equals the result's
end line; see `onSameLine`). `RelatedLocation` results are excluded --
they are only reported when an expectation on the line already
references them, so they are never a genuinely new result to learn.

Both callers become generic. The merge path (`mergedNewTag` ->
`mergedNewExpectation`) feeds `desiredExpectation`, so several new tags
landing on a line that already has a rewritable comment are merged and
re-rendered together via `renderLearnedColumn`. The fresh-append
disjunct of `learnEdits` now aggregates all expectations learned for a
line into a single comment (`renderNewComment`, ordered lexically to
match `renderLearnedColumn`) rather than emitting one comment per
result, so a line on which two results fire grows one deterministic
`// $ ...` comment instead of two.

`Alert` behaviour is unchanged: an `Alert` result still renders exactly
`// $ Alert` and is still merged/appended identically.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---
 .../util/test/InlineExpectationsTest.qll      | 113 +++++++++++-------
 1 file changed, 73 insertions(+), 40 deletions(-)

diff --git a/shared/util/codeql/util/test/InlineExpectationsTest.qll b/shared/util/codeql/util/test/InlineExpectationsTest.qll
index 44fd49c82915..38a52250a9e6 100644
--- a/shared/util/codeql/util/test/InlineExpectationsTest.qll
+++ b/shared/util/codeql/util/test/InlineExpectationsTest.qll
@@ -1157,6 +1157,29 @@ module TestPostProcessing {
       result = " " + renderInlineComment(relativePath, tag)
     }
 
+    /**
+     * Gets the fully rendered inline expectation comment (including comment markers) that `--learn`
+     * should append to `line` of `relativePath`, carrying *every* expectation freshly learned for
+     * that line (see `learnedNewExpectation`) in a single comment, or has no result if that file's
+     * comment syntax is not supported.
+     *
+     * The expectations are ordered lexically, matching `renderLearnedColumn`, so that a line on
+     * which several results fire (for example a path-problem source and sink that coincide) grows
+     * one deterministic comment such as `// $ Sink Source` rather than several separate comments.
+     */
+    bindingset[relativePath, line]
+    private string renderNewComment(string relativePath, int line) {
+      exists(string body |
+        body =
+          concat(string text |
+            learnedNewExpectation(relativePath, line, text)
+          |
+            text, " " order by text
+          ) and
+        result = renderExpectationComment(relativePath, body)
+      )
+    }
+
     /**
      * Holds if, after `--learn`, the inline expectation comment at `commentLoc` should carry the
      * expectation `text` in `column` (`""` for the default column, or `"SPURIOUS"` / `"MISSING"`).
@@ -1193,36 +1216,48 @@ module TestPostProcessing {
     }
 
     /**
-     * Holds if some non-optional `Alert` result with no matching expectation fires on `line` of
-     * `relativePath` -- an *unexpected result* that `--learn` should record with a new `Alert`
-     * expectation, either by appending a fresh comment or by merging into an existing one.
+     * Holds if `--learn` should record a new expectation `text` on `endLine` of `relativePath`,
+     * because a non-optional actual result there has no matching expectation (an *unexpected
+     * result*). `text` is the fully rendered expectation, so it carries a value where the result
+     * has one (`Alert`, `Source`, or a custom `tag=value`).
+     *
+     * The expectation is keyed on the result's *end* line, because an expectation matches a
+     * result when the expectation's start line equals the result's end line (see `onSameLine`).
+     * Whether the new expectation is appended as a fresh comment or merged into an existing one is
+     * decided by the callers (see the append disjunct of `learnEdits` and `mergedNewExpectation`).
+     *
+     * `RelatedLocation` results are excluded: they are only reported when an expectation on the
+     * line already references them (see `hasRelatedLocation`/`shouldReportRelatedLocations`), so
+     * they never constitute a genuinely new result that learning should introduce on its own.
      */
-    private predicate hasUnexpectedAlertOnLine(string relativePath, int line) {
+    private predicate learnedNewExpectation(string relativePath, int endLine, string text) {
       exists(Test::ActualTestResult actualResult |
-        actualResult.getTag() = "Alert" and
-        actualResult.getValue() = "" and
         not actualResult.isOptional() and
+        actualResult.getTag() != "RelatedLocation" and
         not exists(
           Test::getAMatchingExpectation(actualResult.getLocation(), actualResult.toString(),
             actualResult.getTag(), actualResult.getValue(), false)
         ) and
-        parseLocationString(actualResult.getLocation().getRelativeUrl(), relativePath, _, _, line, _)
+        text = actualResult.getExpectationText() and
+        parseLocationString(actualResult.getLocation().getRelativeUrl(), relativePath, _, _,
+          endLine, _)
       )
     }
 
     /**
-     * Holds if `--learn` should merge a freshly learned `Alert` expectation into the existing,
+     * Holds if `--learn` should merge a freshly learned expectation `text` into the existing,
      * rewritable comment at `commentLoc` (in `column` `""`, the default), because an unexpected
-     * `Alert` result fires on that comment's line. Merging keeps the new tag alongside the
-     * comment's existing expectations rather than appending a second comment to the line.
+     * result fires on that comment's line. Merging keeps the new tag alongside the comment's
+     * existing expectations rather than appending a second comment to the line; when several new
+     * expectations land on the same line they are all merged and re-rendered together (see
+     * `renderLearnedColumn`).
      */
-    private predicate mergedNewTag(TestLocation commentLoc, string column, string text) {
+    private predicate mergedNewExpectation(TestLocation commentLoc, string column, string text) {
       exists(string relativePath, int line |
         Test::isRewritableComment(commentLoc) and
         parseLocationString(commentLoc.getRelativeUrl(), relativePath, line, _, _, _) and
-        hasUnexpectedAlertOnLine(relativePath, line) and
-        column = "" and
-        text = "Alert"
+        learnedNewExpectation(relativePath, line, text) and
+        column = ""
       )
     }
 
@@ -1234,14 +1269,15 @@ module TestPostProcessing {
      * This combines the surviving expectations this test understands (see `learnedExpectation`)
      * with the expectations it ignores (see `Test::hasForeignExpectation`), which are preserved
      * verbatim so that rewriting a comment for one query never drops another query's expectation on
-     * the same line, and with any freshly learned tag merged into the comment (see `mergedNewTag`).
+     * the same line, and with any freshly learned expectations merged into the comment (see
+     * `mergedNewExpectation`).
      */
     private predicate desiredExpectation(TestLocation commentLoc, string column, string text) {
       learnedExpectation(commentLoc, column, text)
       or
       Test::hasForeignExpectation(commentLoc, column, text)
       or
-      mergedNewTag(commentLoc, column, text)
+      mergedNewExpectation(commentLoc, column, text)
     }
 
     /**
@@ -1359,15 +1395,18 @@ module TestPostProcessing {
      *
      * The following edits are emitted:
      *
-     * - an actual result with no matching expectation records a new `Alert` expectation (an
-     *   *unexpected result*): if the result's line already has a rewritable comment the tag is
-     *   merged into it (see below), otherwise a fresh `// $ Alert` comment is appended; and
+     * - an actual result with no matching expectation records a new expectation (an *unexpected
+     *   result*): the expectation is the result's tag, carrying a value where the result has one
+     *   (`Alert`, a path-problem `Source`/`Sink`, or a custom `tag=value`). If the result's line
+     *   already has a rewritable comment the expectation is merged into it (see below), otherwise a
+     *   fresh comment carrying every expectation learned for the line is appended (for example
+     *   `// $ Alert`, or `// $ Sink Source` when several results fire on one line); and
      * - an existing rewritable expectation comment is rewritten as a whole so that it matches the
      *   current results: obsolete default and `// $ SPURIOUS:` expectations are dropped (a *missing
      *   result* or a *fixed spurious result*), a `// $ MISSING:` expectation whose result now fires
-     *   is promoted to the default column (a *fixed missing result*), a freshly learned tag on the
-     *   line is merged in, and the resulting expectations are re-rendered. If nothing remains, the
-     *   comment is deleted.
+     *   is promoted to the default column (a *fixed missing result*), any freshly learned tags on
+     *   the line are merged in, and the resulting expectations are re-rendered. If nothing remains,
+     *   the comment is deleted.
      *
      * The rewrite handles comments that carry several expectations across the default,
      * `SPURIOUS:`, and `MISSING:` columns. Expectations this test ignores (for example a tag
@@ -1375,33 +1414,27 @@ module TestPostProcessing {
      * expectation belonging to a different query that shares the same source file; see
      * `isRewritableComment` and `Test::hasForeignExpectation`. A trailing regular note after the
      * expectations (for example the `note` in `// $ Alert // note` or `# $ Alert // note`) is
-     * likewise preserved, and a freshly learned tag can be merged into a plain comment that carries
+     * likewise preserved, and freshly learned tags can be merged into a plain comment that carries
      * only such a note (`// note` -> `// $ Alert // note`); see `Test::getTrailingNote`.
      */
     query predicate learnEdits(
       string file, int line, string operation, int startColumn, int endColumn, string text
     ) {
-      // Unexpected result with no comment to merge into: append a new `// $ Alert` comment on the
-      // alert's line. The comment must go on the result's *end* line, because an expectation
-      // matches a result when the expectation's start line equals the result's end line (see
-      // `onSameLine`). For most languages a result spans a single line, but some (e.g. Rust)
-      // include leading trivia in the location, so the start and end lines differ. If the line
-      // already has a rewritable comment, the tag is merged into it by the rewrite disjunct below
-      // (see `mergedNewTag`) rather than appended as a separate comment.
-      exists(Test::ActualTestResult actualResult, string relativePath, int el, string comment |
-        actualResult.getTag() = "Alert" and
-        actualResult.getValue() = "" and
-        not actualResult.isOptional() and
-        not exists(
-          Test::getAMatchingExpectation(actualResult.getLocation(), actualResult.toString(),
-            actualResult.getTag(), actualResult.getValue(), false)
-        ) and
-        parseLocationString(actualResult.getLocation().getRelativeUrl(), relativePath, _, _, el, _) and
+      // Unexpected result with no comment to merge into: append a fresh comment carrying every
+      // expectation learned for the result's line (see `learnedNewExpectation`). The comment must
+      // go on the result's *end* line, because an expectation matches a result when the
+      // expectation's start line equals the result's end line (see `onSameLine`). For most
+      // languages a result spans a single line, but some (e.g. Rust) include leading trivia in the
+      // location, so the start and end lines differ. If the line already has a rewritable comment,
+      // the new expectations are merged into it by the rewrite disjunct below (see
+      // `mergedNewExpectation`) rather than appended as a separate comment.
+      exists(string relativePath, int el, string comment |
+        learnedNewExpectation(relativePath, el, _) and
         not exists(TestLocation existing |
           Test::isRewritableComment(existing) and
           parseLocationString(existing.getRelativeUrl(), relativePath, el, _, _, _)
         ) and
-        comment = renderExpectationComment(relativePath, "Alert") and
+        comment = renderNewComment(relativePath, el) and
         file = relativePath and
         line = el and
         operation = "append" and