TrecJudge counts negative qrels grades (such as -2 for spam) as relevant

TrecJudge (lucene/benchmark, org.apache.lucene.benchmark.quality.trec) reads TREC relevance judgements (qrels) for QualityBenchmark and QueryDriver. Each line is qnum 0 doc-name grade, and the grade is an integer: trec_eval counts a document as relevant only when its grade is at least 1, and TREC Web track qrels (2010-2014) use -2 for spam or useless pages. TrecJudge instead decides relevance with !"0".equals(grade), so every token except the exact text "0" is relevant, -2 and -1 included. The relevant-document count and the P@n, AP, MRR and recall that Lucene prints are then wrong, silently: a junk page graded -2 ranked above the only relevant page gives 2 relevant documents, P@1 1.0, AP 1.0 and MRR 1.0, where trec_eval's rule gives 1, 0, 0.5 and 0.5. Binary and non-negative graded qrels are unaffected, as are the trec_eval submission file, the index and search results. The test dates from LUCENE-836 (2007): every release from 2.3.0 to 10.6.0 is affected.

1. Parsing. The constructor (TrecJudge.java:54-86) splits each line into query id, an ignored column, document name and grade (:66-70):

   String zero = "0";                                    // :57
   ...
   boolean relevant = !zero.equals(st.nextToken());      // :70

A string comparison, not a number test: -2, -1, 00, 0.0, -0 and any word are "not the string 0", so they are relevant. The assert on :71 only checks for extra tokens, so assertions change nothing.

1. Only relevant lines are kept (:72-81), in a per-query set; isRelevant (:89-93) and maxRecall (:153-161, the set size) read it.
2. The metrics. QualityBenchmark creates new QualityStats(judge.maxRecall(qq), ...) (QualityBenchmark.java:126) and calls judge.isRelevant for every hit (:137-138). A relevant hit raises the good-hit count, sets MRR on the first one in the top 5 and adds its precision to the AP sum (QualityStats.java:85-97); AP is that sum divided by maxGoodPoints (:122-124), recall is good hits divided by maxGoodPoints (:100).
3. Worked steps. qrels junk -2, good 1; hits: rank 1 junk, rank 2 good. Stock: both relevant, maxGoodPoints 2; rank 1 precision 1/1 (MRR 1), rank 2 precision 2/2; AP = (1 + 1) / 2 = 1.0, P@1 = 1.0. trec_eval's rule: 1 relevant document; rank 1 is not relevant (P@1 0); rank 2 precision 1/2, reciprocal rank 1/2; AP = 0.5 / 1 = 0.5.
4. Where it shows. The per-query stats that QualityBenchmark logs ("Max Good Points", "Average Precision", "MRR", "Recall", "Precision At n"; QualityBenchmark.java:140-143, QualityStats.java:156-166) and the SUMMARY that QueryDriver prints (QueryDriver.java:101-102).

• qrels d1:1, d2:-2, d3:0, d4:-1: maxRecall 3 instead of 1.
• 00, -0 and +0 are judged relevant too (secondary: no real qrels file uses them); so are words such as x, no and false.
• Controls: grades 1 and 2 relevant, 0 not relevant, as in trec_eval; the in-tree TestQualityRun (binary qrels) passes.

• Not affected: binary 0/1 qrels and graded qrels with grades 0..4, the trec_eval submission file QueryDriver writes, the index and search results.
• Related, not part of this bug: queries whose judgements are all 0 get maxRecall 0 and are left out of QualityStats.average (by a commented design choice), where trec_eval averages them with AP 0.

• Who is exposed. Anyone who evaluates relevance with the lucene/benchmark search-quality package on qrels that use negative grades: the TREC Web track 2010-2014 qrels (-2 for spam), or custom qrels that use -1 for "bad". TrecJudge is the adjudication the package documents for TREC qrels and the only one QueryDriver uses.
• Effect. Silent wrong evaluation numbers: the relevant-document count is too high, and P@n, AP, MRR and recall can all be wrong; in the example P@1 1.0 instead of 0 and AP 1.0 instead of 0.5. A system that ranks spam high can look better than one that does not. validateData only checks that queries and judgements match.
• Not affected. Binary 0/1 and non-negative graded qrels; the trec_eval submission file; indexes and search results.
• Workaround. Score the submission file with trec_eval, as experienced users do, or drop grades below 1 from the qrels first.
• Releases. Every release from 2.3.0 to 10.6.0 and main. Priority low.

The trec test directory does not exist on main (the module's quality tests sit one package up), hence the mkdir. The seed only satisfies the test framework. As a regression test, reword the comment so it states only the expectation, for example "TREC qrels grade -2 (junk) is not relevant: trec_eval counts a document only if grade >= 1". The end-to-end effect on P@n, AP and MRR follows from isRelevant and maxRecall. The numbers in the steps come from the hunt's end-to-end test (two documents, QualityBenchmark: relevant documents, P@1, AP) and, for MRR, from the diagram's probe (QualityBenchmark and QueryDriver on the same two documents), both run on the same commit. The expected numbers apply trec_eval's relevance rule, read from its source, by hand and through Lucene's own QualityStats; the trec_eval binary was not run.

mkdir -p $LUCENE/lucene/benchmark/src/test/org/apache/lucene/benchmark/quality/trec
cp $REPRO/TestTrecJudgeNegativeGrade.java $LUCENE/lucene/benchmark/src/test/org/apache/lucene/benchmark/quality/trec/
cd $LUCENE && ./gradlew :lucene:benchmark:test --tests org.apache.lucene.benchmark.quality.trec.TestTrecJudgeNegativeGrade -Ptests.seed=DEADBEEF -Ptests.asserts=true
# also with -Ptests.asserts=false

TestTrecJudgeNegativeGrade > testNegativeGradeIsNotRelevant FAILED
    java.lang.AssertionError: expected isRelevant=false for qrels grade -2, got true
        at __randomizedtesting.SeedInfo.seed([DEADBEEF:3055D722D8068F5F]:0)
        ...
        at org.apache.lucene.benchmark.quality.trec.TestTrecJudgeNegativeGrade.testNegativeGradeIsNotRelevant(TestTrecJudgeNegativeGrade.java:30)
:lucene:benchmark:test (FAILURE): 1 test, 1  failure

# the same with -Ptests.asserts=false, on the standalone runner (-ea, -da) and on the 10.6.0 release jars (-ea, -da)
# with the recommended fix: :lucene:benchmark:test (SUCCESS): 1 test

# end to end: QueryDriver on the two-document example (junk -2 ranked 1st, good 1 ranked 2nd), same commit, -da;
# after each '#': the same query through QualityBenchmark with a judge that applies the grade >= 1 rule
SUMMARY
  Num Points:             2.000      # 2.000
  Num Good Points:        2.000      # 1.000
  Max Good Points:        2.000      # 1.000
  Average Precision:      1.000      # 0.500
  MRR:                    1.000      # 0.500
  Recall:                 1.000      # 1.000
  Precision At 1:         1.000      # 0.000

• Contract. TrecJudge "Judge[s] if given document is relevant to given quality query, based on Trec format for judgements" (TrecJudge.java:28-30). The package: "For benchmarking TREC collections with TREC QRels, take a look at the trec package" (package-info.java:33-34); the trec package is "Utilities for Trec related quality benchmarking, feeding from Trec Topics and QRels inputs" (trec/package-info.java:18).
• Graded example. package-info.java:36-69 runs "the TREC 2006 queries 701-850 on the .Gov2 collection" through TrecJudge; those qrels (the TREC 2004-2006 Terabyte ad hoc qrels for topics 701-850) use only grades 0, 1 and 2: 108,435 / 22,566 / 4,351 lines over the three files.
• Reference scorer. QueryDriver writes an "output submission file for trec_eval" (QueryDriver.java:48). trec_eval reads the grade as "an integer between -127 and 127" (get_qrels.c:42-43) through a checked strtol wrapper that rejects out-of-range or non-numeric grades (get_qrels.c:18-35, :217-221), counts relevant only rel >= relevance_level (form_res_rels.c:213, :228; default 1) and maps negative values to "In pool, but unjudged" (form_res_rels.c:178-180).
• Real data. The TREC 2012 Web track eval-README: "-2: spam or otherwise seems useless for any information need, 0: not relevant, 1: relevant, 2: highly relevant, 3: key, 4: nav"; "Adhoc evaluation results are computed using trec_eval".
• History. The string test came in with the class (LUCENE-836, #1911, 2007); neither the commit nor the issue discusses graded or negative grades, and no test, comment or later commit says a negative grade means relevant.

Strongest case (the intended-behaviour check and the cross-model reviews, rated moderate): the constructor javadoc names the 4th column is-relevant and shows only 1 and 0 (TrecJudge.java:41, :47-48, unchanged since 2007); the in-tree qrels fixture is binary; NIST's classic ad hoc qrels page calls the field "a binary code of 0 for not relevant and 1 for relevant"; and the package says "To run on different formats of queries and judgements provide your own Judge" (package-info.java:75-77). So negative grades might be outside the declared input.

Why it fails: the package offers TrecJudge for TREC qrels in general and demonstrates it on graded qrels; grades 2 to 4 already work, so the code is not binary-only in practice. Nothing treats a negative grade as relevant, and official qrels with -2 are scored with trec_eval. Even under the binary reading, the parser silently gives an out-of-format value the opposite meaning instead of rejecting it. Verdict: not intended, a defect. A committer may still call the change a robustness fix.

Option A was built and run in a scratch worktree of 645d4fe14a20. Option B, the hunt's sketch, was checked by shadowing the patched class over the main jars. A javadoc-only change was considered and not built.

Replace the string test with Integer.parseInt(grade) > 0 and document the column as an integer grade with trec_eval's default rule (grade 1 or more is relevant; 0 and negative grades are not).

    * <pre>
-   *     qnum  0   doc-name     is-relevant
+   *     qnum  0   doc-name     relevance-grade
    * </pre>
    *
-   * Two sample lines:
+   * The relevance grade is an integer. As in trec_eval, a document is relevant only if its grade is
+   * 1 or more; 0 and negative grades (such as -2 for spam pages in some TREC tracks) are not
+   * relevant.
+   *
+   * <p>Two sample lines:
 ...
-    String zero = "0";
 ...
-        boolean relevant = !zero.equals(st.nextToken());
+        boolean relevant = Integer.parseInt(st.nextToken()) > 0;

Trade-offs

• 0/1 and 0..4 qrels give exactly the same results as today (TestQualityRun passes).
• Odd spellings of zero change meaning: 00, -0 and +0 become non-relevant (relevant today); +1 is accepted as 1.
• A grade token that is not a plain Java int now throws NumberFormatException instead of silently counting as relevant: 0.0, words such as x, values beyond the int range, and any number with other characters attached. trec_eval also rejects x and out-of-range values (it stops loading the qrels) but reads 0.0 as 0 with a warning. Wrap it in an IllegalArgumentException naming the line if a clearer message matters.
• A query whose qrels lines are all negative (or all 0) keeps no relevant document: maxRecall 0, a "queries have no judgments" warning from validateData, and QualityStats.average leaves it out of the SUMMARY. All-0 queries are handled this way today; pre-filtering the qrels to grades >= 1 has the same effect.
• No relevance-level option like trec_eval's -l is added (out of scope).

What the checks validated

• Stock + test: fails with assertions on and off (gradle); + fix: passes, on and off.
• + fix: all lucene/benchmark tests, 91 tests in 19 suites, 0 skipped, 0 failures (assertions on); TestQualityRun and the new test with assertions off.
• + fix: :lucene:benchmark:check -x test and renderJavadoc pass; the check tasks that ran were google-java-format, ecjLint and forbiddenApis (eclint is not configured in this checkout and errorProne was skipped).
• With the patched class shadowed: the hunt's end-to-end test (P@1 0, AP 0.5), the independent reproduction's three tests (MRR 0.5; 00, -0, +0) and the minimal test through the standalone runner (-ea, -da) pass. Not run: the full Lucene test suite.
• An independent fact check repeated the gradle runs in its own scratch worktree, with the same results.

A private parseGrade reads an optional sign and the leading digits (0.0 -> 0, -2 -> -2) and throws only when there is no digit; relevant iff > 0 (+17/-3).

Trade-offs

Closer to trec_eval's syntax for odd tokens (0.0 -> 0, without trec_eval's warning), but more code, and not an exact trec_eval parser. It throws on a grade beyond the long range, as trec_eval also rejects one: its str_to_long wrapper treats strtol's out-of-range result as an error and stops loading the qrels.

What the checks validated

Shadowed over the 645d4fe14a20 jars: the minimal test passes with -ea and -da, and so do a cross-model reviewer's test and the upstream TestQualityRun; in the hunt's harness the token mismatches drop from 10 to 0. Not run through gradle.

Document that the 4th column is a 0/1 flag and that any value other than 0 is relevant. Not built.

Trade-offs

Smallest change, but TREC Web track qrels stay silently wrong, and it contradicts the package's "TREC QRels" claim and its graded example.

Method: bounded-exhaustive differential check (semi-formal), a hunt unit (benchmark quality metrics versus trec_eval). A planning probe first saw maxRecall 4 instead of 2 and grades -2 and -1 judged relevant. The finder's harness QualityRefCheck.java then ran, on the stock jar, 26 single-token checks over a 13-token grade alphabet (10 mismatches: -2, -1, 00, 0.0, -0) and all 625 four-document assignments of grades -2..2 (544 judged wrongly, exactly those with a negative grade; this count reflects the alphabet, not real-world frequency), against an independent reference of trec_eval's rule taken from its source. With the fix sketch shadowed, both counts were 0. The finder then wrote a public-API JUnit (a four-line qrels and an end-to-end QualityBenchmark run); this doc's test is its one-line form.