Classification-only metrics: drop rank metrics, richer aggregation, F-beta everywhere (0.3.0) by dfuchss · Pull Request #56 · ardoco/metrics

dfuchss · 2026-08-26T11:57:18Z

Reshapes the metrics library into a classification-only 0.3.0-SNAPSHOT: drops the rank metrics, makes aggregations keep their data and be addressable by name, and supports F-beta scores through every interface.

Breaking, deliberately and without deprecated shims. Downstream ArDoCo code needs updating; the ArDoCo parent still pins metrics.version=0.2.1, so that needs a bump once this releases.

1. Rank metrics removed

RankMetricsCalculator, the calculation functions, the three result types, the rank/aggRnk CLI subcommands, the /rank-metrics endpoints and the wiki page are gone.

They were not merely untested — they were wrong. RankMetricsCalculatorImpl.calculateWeightedAverage never divided auc by sumOfWeights, so the aggregated AUC was a weighted sum, not an average. The same loop branched on all { it.auc == null } while summing per element, so a list where only some results had an AUC produced a partial sum over a full denominator. Repairing math with zero test coverage and no consumers seemed worse than deleting it.

2. Aggregations keep their data

The old calculateMicroAverage summed TP/FP/FN/TN and then threw the counts away, by stuffing them into a SingleClassificationResult<Nothing> with empty element sets. AggregatedClassificationResult had no confusion matrix at all, weights was null for micro but not otherwise, and all three aggregations each carried their own copy of the inputs — so a /average response repeated its inputs three times.

Now the data structures are split by who owns what:

ClassificationResult carries a ConfusionMatrix. For a single result and for the micro average the metrics are exactly the metrics of that matrix; for the macro and the weighted average, which are means over the single results, it is the pooled matrix and describes the underlying data rather than the origin of the values. The contract says so, and a test pins which of the two holds where.
AggregatedClassificationResult is reduced to the aggregated values plus the pooled matrix.
The new ClassificationAggregationResult holds the single results and the weights once, and derives the rest on demand: the pooled confusionMatrix, the element unions (truePositives() / falsePositives() / falseNegatives()) and the distribution of a metric across the inputs (spread(metric), fbetaSpread(beta)).

asList() and the union/spread accessors are functions rather than properties on purpose, so they do not end up duplicated in the serialized JSON.

3. `calculateAverages` returns an object

// before — order and cardinality were an undocumented contract
val macro = calculator.calculateAverages(results).first { it.type == MACRO_AVERAGE }

// after
val aggregation = calculator.calculateAverages(results)
aggregation.macroAverage.f1
aggregation[AggregationType.MICRO_AVERAGE].recall

The REST /average response is keyed by macroAverage / weightedAverage / microAverage instead of being a classificationResults array.

4. F-beta everywhere

Every result carries fbetaScores: Map<Double, Double> next to f1, and the betas are selectable per call — library overloads, CLI -b 0.5,2, REST "betas": [...]. Beta 1.0 is always included, duplicates are dropped and the keys are ascending. calculateF1 now delegates to the new calculateFBeta, removing the formula that SingleClassificationResult.fBeta duplicated.

The aggregation rule is explicit and pinned by a test: macro and weighted F-beta are the (weighted) mean of the per-result scores, while micro is recalculated from the pooled matrix. Neither is the F-beta of the averaged precision and recall — on the test fixture that wrong computation gives 0.62 where the correct macro F1 is 0.49.

Bugs fixed along the way

A weights list of the wrong length walked off the end with IndexOutOfBoundsException, reachable straight from the REST /average endpoint with user-supplied weights. Now a require, answered with 400.
A confusionMatrixSum smaller than the number of classified and expected elements silently produced a negative true-negative count and nonsense accuracy/phi. Now rejected.
HomeController registered a @Primary bare ObjectMapper, discarding Spring Boot's configured one. Under Spring Boot 4 jackson-module-kotlin is auto-registered, so the bean was redundant and harmful.
Handler mapped everything non-NPE to 500, swallowing Spring's own exceptions: an unknown path answered 500 instead of 404, an unsupported method 500 instead of 405, a malformed body 500 instead of 400. It now extends ResponseEntityExceptionHandler, so those keep their proper status, and IllegalArgumentException maps to 400 so invalid input is a client error.
The CLI always exited 0, whatever happened. main() discarded the status execute() returned, so a missing input file, an invalid beta or an unreadable result file printed a message and still reported success — unusable for a pipeline that branches on the exit status. It now exits with the command's status, pinned by a test that runs the CLI in its own process, because calling execute() directly cannot catch this.
aggCl crashed with a raw stack trace on any file in -d that is not a classification result — a stray note, a .DS_Store, or its own -o output written back into the input directory on a re-run. Hidden files are now skipped and anything else that fails to parse is reported by name with exit status 1, rather than being silently dropped from the aggregate. An IllegalArgumentException out of calculateAverages (mixed true negatives, say) is handled the same way instead of escaping.
calculateAccuracy(0, 0, 0, 0) returned NaN, which Jackson writes as the JSON string "NaN" — violating the number type the REST schema declares for accuracy, so a generated client fails to parse it. Reachable from /classification-metrics with an empty classification, an empty ground truth and confusionMatrixSum: 0. It now returns the same 1.0 sentinel as its siblings, and a test pins that no metric of an empty confusion matrix is non-finite.

Jackson traps worth knowing about

Three findings that shaped the API and are guarded by tests, since calculator deliberately depends on neither Jackson nor Swagger and so cannot carry annotations:

The property is fbetaScores, not fScores. Jackson's legacy bean naming mangles getFScores() to fscores while jackson-module-kotlin reports the constructor parameter as fScores. They never match, so the mismatch is treated as a read-only property and silently dropped — every non-F1 score would vanish on read. A test asserts the literal JSON key.
Getter-only properties serialize but do not deserialize. confusionMatrix and ConfusionMatrix.total started out that way, which meant a consumer with a default ObjectMapper could not read back this library's own output. confusionMatrix became a creator property (defaulted from the elements, checked against them in init) and total() became a function. A round-trip test with FAIL_ON_UNKNOWN_PROPERTIES enabled guards it.
Pre-0.3.0 result files stay readable by aggCl: they have f1 but no fbetaScores, which the Kotlin constructor default covers.

Tests

The suite covered six free functions with a handful of values each; calculatePhiCoefficientMax and calculatePhiOverPhiMax had zero coverage, as did the calculator impl, the whole aggregation path, the CLI and the REST layer.

	Instr	Branch	Line
calculator	97.4%	97.9%	95.5%
cli	85.7%	91.7%	92.0%
rest	89.4%	80.8%	93.0%
total	94.7%	95.4%	94.5%

Every surviving metric is pinned by its documented sentinel, its boundaries, reference values from the textbook definitions, and cross-cutting invariants (0 <= p, r, F_beta <= 1; -1 <= phi <= 1; min(p,r) <= F1 <= max(p,r); phiMax >= |phi|). Every metric of every aggregation type is checked against hand-computed values, together with structural invariants — a one-element input aggregates to itself, N identical inputs aggregate to one of them, macro equals weighted for equal weights.

JavaInteropTest pins the API the wiki documents for Java callers, since Kotlin default arguments are invisible from Java and @JvmOverloads is not allowed on interface members. OpenApiDocumentationTest guards that the Swagger spec stays complete.

What is left uncovered is unreachable or not worth testing: Kotlin DefaultImpls shims (dead — Kotlin 2.3 emits real JVM default methods), main() functions, and generated data class members.

One genuine finding: calculatePhiCoefficientMax's mirrored-branch zero guard is provably unreachable — that branch requires fn < fp, which forces fp >= 1, which makes the nominator at least 1. Harmless, left as is rather than contorting a test to reach it.

Swagger UI

The spec was structurally valid but bare: no field descriptions, no examples, no documented error responses. Now every schema and all 43 properties are described, both operations have two named request examples and a documented 400, and the result schemas carry full response examples.

The result schemas are documented from a rest-side OpenApiCustomizer rather than by annotating library types. Attaching examples to @ApiResponse does not work: springdoc replaces response content rather than merging it, so a @Content override either drops the inferred schema entirely or, with an explicit implementation, adds a second raw-typed copy next to the generic one. A test guards both failure modes.

Also

Spelling aligned to ARDoCo (per ardoco.de — Automating Requirements and Documentation Comprehension); GitHub URLs normalized to the lowercase form the poms already used.
rest imports the JUnit BOM ahead of the Spring Boot BOM: Boot pins an older JUnit platform than the Jupiter version inherited from the parent, and the mismatch made every test in the module fail with IllegalAccessError.
Docs rewritten; Usage-Via-Library gains the aggregation example that was missing entirely, and its dead s01.oss.sonatype.org snapshot URL is fixed. All JSON in the docs is copied from real output.

Verification

mvn clean verify green. Verified beyond the suite: CLI end-to-end including a pre-0.3.0 input file; REST live via curl — output matches the docs byte for byte, all four error paths return 400, /v3/api-docs lists only the two classification paths; Java interop compiled and run; dokka javadoc jar builds.

Left for follow-up

One pre-existing behaviour the new tests pin rather than change, since changing it would move reported numbers: macro/weighted averaging of phiCoefficient / phiCoefficientMax / phiOverPhiMax is linear — questionable for a correlation coefficient, worse for a ratio to its own max.

The rank metrics (MAP, LAG, ROC/AUC) are error-prone and untested. Notably RankMetricsCalculatorImpl.calculateWeightedAverage never divided `auc` by `sumOfWeights`, so the aggregated AUC was a weighted sum rather than an average, and its `all { it.auc == null }` branch produced a wrong denominator for mixed input. Rather than repair math nobody trusts, drop the whole family. Removes the calculator, calculation functions and result types, the `rank` and `aggRnk` CLI subcommands, the /rank-metrics REST controller, and the wiki page. Bumps the version to 0.3.0-SNAPSHOT since this is a breaking change.

calculateFBeta generalises the F-score to arbitrary beta and calculateF1 now delegates to it, removing the duplicated formula that SingleClassificationResult carried. Invalid betas are rejected with IllegalArgumentException instead of the IllegalStateException that `error(...)` produced. ConfusionMatrix bundles the four counts so metrics no longer have to be carried around as loose Ints, and so an aggregation can report its pooled counts. It validates non-negative counts on construction, which keeps derived accessors non-throwing. Also fills two long-standing coverage gaps: calculatePhiCoefficientMax and calculatePhiOverPhiMax had no tests at all. Every remaining metric now has its documented sentinel, its boundaries, reference values from the textbook definitions, and cross-cutting invariants pinned (67 tests, up from 6).

Splits the data structures so that provenance is stored once and everything derivable is derived: - ClassificationResult now carries the ConfusionMatrix its metrics came from, so an aggregation reports its pooled counts instead of dropping them. Previously calculateMicroAverage summed tp/fp/fn/tn and then threw the counts away by building a SingleClassificationResult<Nothing> with empty element sets. - AggregatedClassificationResult is reduced to the aggregated values plus the pooled matrix. The single results and the weights move up into the new ClassificationAggregationResult, which previously meant every /average response repeated its inputs three times. - ClassificationAggregationResult replaces the List<AggregatedClassificationResult> return of calculateAverages. Callers reach an aggregation by name (macroAverage/weightedAverage/microAverage) or by AggregationType instead of filtering a list whose order and cardinality were undocumented. The pooled confusion matrix, the unions of classified elements and the spread of a metric across the single results are derived on demand; asList() and the unions are functions rather than properties so they do not end up duplicated in JSON. - All F-beta scores are kept in fbetaScores, and betas are selectable per call (defaulting to F1 only). Beta 1.0 is always included so f1 stays available. Macro and weighted aggregation average the per-result F-beta scores; micro recalculates from the pooled matrix. - The internal ClassificationMetricValues carrier replaces the SingleClassificationResult<Nothing> abuse. Two guards close silent-corruption holes: a weights list of the wrong length used to walk off the end with IndexOutOfBoundsException (reachable straight from the REST /average endpoint), and a confusionMatrixSum smaller than the number of classified and expected elements used to produce a negative true-negative count and nonsense accuracy/phi values. The property is named fbetaScores rather than fScores on purpose: Jackson's legacy bean naming mangles getFScores() to "fscores" while jackson-module-kotlin reports the constructor parameter as "fScores", so the two would never match and every non-F1 score would be silently dropped on read. Tests go from 6 to 126, covering the set algebra, the sentinels, every metric of every aggregation type against hand-computed values, the structural invariants, the spread accessors and all rejection paths.

`classification` gains -b/--beta (repeatable or comma-separated) to request additional F-beta scores; the F1-score is always calculated. `aggCl` derives the betas from its input files and now writes the ClassificationAggregationResult object instead of a three-element array, so the aggregations are addressable by name and the inputs appear once rather than three times. The three duplicated ObjectMapper setups move into a single createObjectMapper() helper. It enables NullIsSameAsDefault so a hand-edited `"fbetaScores": null` falls back to the F1-only default, and disables FAIL_ON_UNKNOWN_PROPERTIES so files written by other versions stay readable. Two changes make SingleClassificationResult round-trip under any mapper configuration, which it did not do as written: confusionMatrix became a creator property (defaulted from the classified elements and checked against them in init) and ConfusionMatrix.total became a function. Both were getter-only properties, so Jackson serialized them but did not register them as deserializable, and a consumer using a default ObjectMapper could not read back this library's own output. Adds the first tests for the cli module: JSON shape and round-trip contracts (including the literal fbetaScores key, whose name is load-bearing), pre-0.3.0 result files, and end-to-end runs of both commands through picocli.

@primary

Both endpoints accept an optional `betas` array; the F1-score is always included. On /average the betas apply to every project, so they have to be given on the request level and per-request betas are rejected rather than silently ignored. /average now returns the ClassificationAggregationResult itself, so the response is keyed by macroAverage/weightedAverage/microAverage instead of a `classificationResults` array the caller had to filter. It also carries the pooled confusionMatrix, the weights and the betas, and its inputs appear once rather than once per aggregation type. AverageClassificationMetricsResponse existed only to name the list and is gone. Handler maps IllegalArgumentException to 400, so invalid input (a beta that is not greater than 0, a weights list whose length does not match the requests, a confusion matrix sum that is too small, mixed true-negative availability) is reported as a client error instead of a 500. Drops the @primary ObjectMapper bean in HomeController. It replaced Spring Boot's configured mapper with a bare one, discarding Boot's defaults; under Spring Boot 4 jackson-module-kotlin is auto-registered, so the bean was redundant. Adds the first tests for the rest module, covering both endpoints with and without a confusion matrix sum, weights and betas, all four rejection paths, and a guard that no rank mapping is registered. Also imports the JUnit BOM ahead of the Spring Boot BOM: Boot pins an older JUnit platform than the Jupiter version inherited from the ArDoCo parent, and the mismatch made every test in the module fail with an IllegalAccessError.

Removes the rank metrics page and every rank reference from the README and the wiki, and renumbers the sections that followed. Documents what changed in 0.3.0: the F-beta formula and the betas input, the confusion matrix that every result now exposes, and the ClassificationAggregationResult with its named accessors, pooled confusion matrix, element unions and metric spreads. Spells out the aggregation rule that macro and weighted F-beta scores are means of the per-result scores while the micro average is recalculated from the pooled counts, and why an aggregation cannot answer for a beta it was not built with. Also drops the previous false claim that the micro average applied to rank metrics. Usage-Via-Library gains the aggregation example that was missing entirely (section 4 used to be a bare link) plus an F-beta section, both in Kotlin and Java, and its dead s01.oss.sonatype.org snapshot URL is replaced with the one the root pom actually uses. All JSON in the CLI and REST pages is copied from real output rather than hand-written. The documented Java API is now pinned by JavaInteropTest, so the wiki's Java examples cannot silently stop compiling. It also guards that each overload documented for Java callers keeps existing, since Kotlin default arguments are invisible from Java and @jvmoverloads is not allowed on interface members.

A JaCoCo run showed three real gaps rather than just untested glue. Handler had 0% branch coverage: neither the NullPointerException branch nor the catch-all 500 was exercised, even though it is the class that decides how every failure reaches a client. It is now unit-tested for all four cases, which also pins that an unknown path is answered with 500 rather than 404. The invariants the result types enforce on direct construction - negative true negatives, F-beta scores without the F1 entry, a confusion matrix that disagrees with the classified elements, an empty aggregation, an empty metric spread - were only reachable through the calculator, which rejects most of them earlier. They are what keeps the derived accessors from throwing or going inconsistent when a result is built by hand or by a deserializer, so they are now tested directly. The CLI paths without -o and with blank lines in the input files were untested.

@Schema

The spec was structurally valid but bare: no field descriptions, no examples on any request or response, and only a generic "OK" per operation with no error response documented at all. Request DTOs get @Schema descriptions and examples, both operations get a description, two named request examples and a documented 400, and the controller gets a @tag and an explicit application/json media type instead of */*. The result schemas are documented from OpenApiConfiguration rather than by annotating them, because they live in the calculator module, which deliberately depends on neither Swagger nor Jackson. Descriptions are matched by property name, so they apply to the single results, the aggregations and the nested confusion matrices alike, and the two full response examples are attached to the schemas. Attaching those examples to @apiresponse instead does not work: springdoc replaces the response content rather than merging it, so a @content override either drops the inferred schema entirely or, with an explicit implementation class, adds a second raw-typed copy of it next to the generic one. A test guards against both, alongside checks that every schema and property stays described and that both operations keep their examples and their 400.

ardoco.de spells it ARDoCo, for Automating Requirements and Documentation Comprehension, so the two capitals at the front are part of the acronym. The repository used ArDoCo in prose, the CLI help header and the OpenAPI title while the Maven module names already used ARDoCo. Also normalizes the GitHub URLs to github.com/ardoco/metrics, which is the form the poms and the website already use; the mixed-case ones redirected but did not match anything else in the repository. Maven coordinates (io.github.ardoco), package names and the container image reference are lowercase identifiers and stay as they are.

File.listFiles() returns entries in filesystem order, which the Javadoc leaves unspecified and which differs between platforms and runs. aggCl therefore produced singleResults and weights in an arbitrary order, so its output was not reproducible across machines even for identical input. The aggregated numbers were unaffected, since macro, weighted and micro are all order-independent, but the JSON was not diffable. Sorting by file name makes the output deterministic and documents the order as part of the command's contract. This is what turned the CI build red: aggregationCommandTest asserted the weights in the order the files were written, which held on APFS but not on the runner. The new ordering test writes the files in an order that contradicts their names, so it fails on both platforms if the sort is removed.

The README is a quickstart that points at the wiki; release-to-release changes belong in the GitHub release notes, which is how this project has described every release so far. The breaking changes of 0.3.0 are recorded in the pull request and go into the release notes from there.

@ExceptionHandler

Handler declared a catch-all @ExceptionHandler(Exception) that mapped everything except NullPointerException to 500, which swallowed Spring's own exceptions. An unknown path was reported as 500 instead of 404, an unsupported method as 500 instead of 405, and a malformed or incomplete request body as 500 instead of 400. Extending ResponseEntityExceptionHandler lets those exceptions keep the status Spring already defines for them; only failures Spring does not know about reach the catch-all and are reported as a server error.

Copilot

Pull request overview

Reshapes the library into classification-only version 0.3.0, adds F-beta support, and introduces richer aggregation results.

Changes:

Removes rank metrics across library, CLI, REST, and documentation.
Adds F-beta metrics, pooled confusion matrices, named aggregations, and metric spreads.
Expands validation, serialization, API documentation, and automated tests.

Reviewed changes

Copilot reviewed 51 out of 51 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`README.md`	Updates classification-only branding.
`LICENSE.md`	Updates ARDoCo capitalization.
`pom.xml`	Bumps version to 0.3.0-SNAPSHOT.
`docs/Aggregation-of-Metrics.md`	Documents new aggregation model.
`docs/Classification-Metrics.md`	Documents F-beta and confusion matrices.
`docs/Home.md`	Removes rank-metric references.
`docs/Rank-Metrics.md`	Removes rank documentation.
`docs/Usage-Via-CLI.md`	Documents classification-only CLI.
`docs/Usage-Via-Library.md`	Documents new library API.
`docs/Usage-Via-REST-API.md`	Documents revised REST responses.
`calculator/.../ClassificationMetricsCalculator.kt`	Extends public classification API.
`calculator/.../RankMetricsCalculator.kt`	Removes rank calculator API.
`calculator/.../calculation/ClassificationMetrics.kt`	Adds F-beta calculation.
`calculator/.../calculation/RankMetrics.kt`	Removes rank calculations.
`calculator/.../internal/ClassificationMetricValues.kt`	Centralizes metric computation.
`calculator/.../internal/ClassificationMetricsCalculatorImpl.kt`	Implements new aggregation behavior.
`calculator/.../internal/RankMetricsCalculatorImpl.kt`	Removes rank implementation.
`calculator/.../result/AggregatedClassificationResult.kt`	Simplifies aggregated results.
`calculator/.../result/AggregatedRankMetricsResult.kt`	Removes aggregated rank result.
`calculator/.../result/ClassificationAggregationResult.kt`	Adds named aggregation container.
`calculator/.../result/ClassificationMetric.kt`	Adds selectable metric enum.
`calculator/.../result/ClassificationResult.kt`	Adds matrices and F-beta scores.
`calculator/.../result/ConfusionMatrix.kt`	Adds confusion-matrix value type.
`calculator/.../result/MetricSpread.kt`	Adds distribution summaries.
`calculator/.../result/RankMetricsResult.kt`	Removes rank result interface.
`calculator/.../result/SingleClassificationResult.kt`	Retains matrix and F-beta data.
`calculator/.../result/SingleRankMetricsResult.kt`	Removes single rank result.
`calculator/.../ClassificationAggregationTest.kt`	Tests aggregation semantics.
`calculator/.../ClassificationMetricsCalculatorTest.kt`	Tests calculator behavior.
`calculator/.../JavaInteropTest.java`	Verifies Java API usability.
`calculator/.../calculation/ClassificationMetricsTest.kt`	Expands metric formula tests.
`calculator/.../result/ConfusionMatrixTest.kt`	Tests matrix operations.
`calculator/.../result/ResultValidationTest.kt`	Tests result invariants.
`cli/.../App.kt`	Removes rank subcommands.
`cli/.../Json.kt`	Centralizes JSON configuration.
`cli/.../commands/AggregationClassificationCommand.kt`	Emits new aggregation object.
`cli/.../commands/AggregationRankCommand.kt`	Removes rank aggregation command.
`cli/.../commands/ClassificationCommand.kt`	Adds CLI beta selection.
`cli/.../commands/RankCommand.kt`	Removes rank command.
`cli/.../ClassificationCommandTest.kt`	Tests CLI behavior.
`cli/.../ResultSerializationTest.kt`	Tests JSON compatibility.
`rest/pom.xml`	Configures REST test dependencies.
`rest/.../Application.kt`	Updates OpenAPI metadata.
`rest/.../Handler.kt`	Maps invalid arguments to 400.
`rest/.../OpenApiConfiguration.kt`	Enriches generated schemas.
`rest/.../controller/ClassificationMetricsController.kt`	Adds F-beta and named aggregation endpoints.
`rest/.../controller/HomeController.kt`	Removes custom ObjectMapper bean.
`rest/.../controller/RankMetricsController.kt`	Removes rank endpoints.
`rest/.../HandlerTest.kt`	Tests exception mappings.
`rest/.../OpenApiDocumentationTest.kt`	Tests OpenAPI completeness.
`rest/.../controller/ClassificationMetricsControllerTest.kt`	Tests revised REST API.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

calculateFBeta squared the beta directly, which overflows to infinity above roughly 1.3e154. The whole expression then evaluated to NaN and the guard reported it as 0.0 instead of approaching the recall. For beta > 1 the equivalent form scaled by 1/beta^2 is used instead, which keeps both factors within [0, 1] over the entire finite beta domain and leaves ordinary betas bit-identical. The weighted mean divides by the sum of the weights, which nothing checked. All weights are 0 whenever every ground truth is empty, since the default weight is the size of the ground truth, and the REST endpoint passed caller-supplied weights straight through, so zero or negative weights produced NaN or out-of-range metrics rather than an error. f1 was only required to be present in fbetaScores, not to agree with it, so a hand-built or deserialized result could return different values from f1 and fbeta(1.0) - and aggregation reads the map. Both result types now enforce the equality that ClassificationResult documents. The CLI treated -s as omitted whenever it was negative, so an explicit -s -1 bypassed the confusion matrix validation instead of being rejected. The option is nullable now, and invalid input is reported with the reason and exit code 1 rather than a stack trace. The OpenAPI aggregation example put a string placeholder in singleResults, which its own schema declares as an array of single results, so the advertised example was unusable for clients and schema validators. Both examples are now the real serialized service output, and a test walks every example against its schema.

The CLI discarded the status that picocli returned, so every failure still exited 0. aggCl threw a raw stack trace on any file in -d that is not a classification result, including its own -o output on a re-run. calculateAccuracy was the only metric without a zero-denominator guard and returned NaN, which Jackson writes as the JSON string "NaN" and which therefore violates the number type the REST schema declares. Also states plainly what confusionMatrix means on an aggregation: only the micro average is calculated from the pooled matrix, while macro and weighted are means over the single results.

A cross-check against scikit-learn over 262 generated cases found metricInvariantsTest asserting two properties that do not hold: |phi/phiMax| <= 1 and phiMax >= |phi|. phiMax is Ferguson's maximum of the positive phi for the given marginals, so it does not bound a negatively correlated classification, which is limited by the most negative attainable phi instead. The ratio then leaves [-1, 1] without any bound, for example -2.0 for (tp, fp, fn, tn) = (0, 1, 2, 0). The suite passed only because each of its twelve rows with a negative phi happens to have equal marginals, which forces phiMax to 1.0 and collapses the ratio onto phi itself. Exhaustively over all confusion matrices with a total of at most 40: no case with a non-negative phi violates the bound, while 28.7% of the negatively correlated ones do. The maths is therefore left alone and the assertion is restricted to the regime the metric is defined for. Three rows with unequal marginals now cover the other regime, and negativePhiIsNotBoundedByPhiMaxTest pins the actual values so that the limitation is recorded rather than merely absent. Everything else agrees with scikit-learn to 1e-9: precision, recall, F1, F-beta, accuracy, specificity, the phi coefficient itself, the confusion matrix counts and all three aggregation types.

sonarqubecloud · 2026-08-26T15:12:05Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Copilot

Pull request overview

Copilot reviewed 51 out of 55 changed files in this pull request and generated 8 comments.

Suppressed comments (1)

calculator/src/main/kotlin/edu/kit/kastel/mcse/ardoco/metrics/internal/ClassificationMetricsCalculatorImpl.kt:56

Int.sum() can overflow even though every weight is non-negative. Two valid weights of Int.MAX_VALUE produce -2, so this rejects a mathematically valid REST/library request before weightedMean, whose Double accumulator could handle it. Test whether any weight is positive instead of summing them.

        require(weightsForAverage.sum() > 0) {

+            confusionMatrixSum?.let { sum ->
+                val trueNegatives = sum - (tp.size + fp.size + fn.size)
+                require(trueNegatives >= 0) {
+                    "The confusion matrix sum ($sum) must be at least the number of classified and expected elements (${tp.size + fp.size + fn.size})"
+                }
+                trueNegatives
+            }


+            "There must be exactly one weight per single result but there were ${weights.size} weights for ${singleResults.size} results"
+        }
+        require(weights.all { it >= 0 }) { "Weights must not be negative but were $weights" }
+        require(weights.sum() > 0) { "At least one weight must be greater than 0 but all weights were 0" }


+            truePositives + other.truePositives,
+            falsePositives + other.falsePositives,
+            falseNegatives + other.falseNegatives,
+            if (trueNegatives == null || other.trueNegatives == null) null else trueNegatives + other.trueNegatives


+    val confusionMatrix: ConfusionMatrix
+        get() = microAverage.confusionMatrix


-            error("Beta must be greater than 0 for F-beta score.")
+    init {
+        require(trueNegatives == null || trueNegatives >= 0) { "The number of true negatives must not be negative but was $trueNegatives" }
+        require(fbetaScores.containsKey(1.0)) { "The F-beta scores must contain the F1-score (beta 1.0)" }


+    override val phiOverPhiMax: Double?
 ) : ClassificationResult {
+    init {
+        require(fbetaScores.containsKey(1.0)) { "The F-beta scores must contain the F1-score (beta 1.0)" }


+ * @param precision the precision
+ * @param recall    the recall
+ * @param beta      the weight of the recall relative to the precision; must be finite and greater than 0
+ * @return the F-beta score; 0.0 iff beta&#178;*precision+recall=0


+ * 1.0 because nothing was classified wrongly, which matches the sentinel of [calculatePrecision], [calculateRecall] and [calculateSpecificity].
 *
- * @return the accuracy
+ * @return the accuracy; 1.0 iff TP+FP+FN+TN=0


dfuchss added 11 commits August 26, 2026 11:23

dfuchss requested a balanced review from Copilot August 26, 2026 13:15

Copilot started reviewing on behalf of dfuchss August 26, 2026 13:16 View session

Copilot AI reviewed Aug 26, 2026

View reviewed changes

dfuchss added 3 commits August 26, 2026 15:31

cleanup tests

ffe7989

dfuchss force-pushed the feature/only-classification branch from 4209892 to b24ffda Compare August 26, 2026 14:50

dfuchss requested a balanced review from Copilot August 26, 2026 15:50

Copilot started reviewing on behalf of dfuchss August 26, 2026 15:53 View session

Copilot AI reviewed Aug 26, 2026

View reviewed changes

Navigation Menu

Uh oh!

Classification-only metrics: drop rank metrics, richer aggregation, F-beta everywhere (0.3.0) - #56

Classification-only metrics: drop rank metrics, richer aggregation, F-beta everywhere (0.3.0)#56
dfuchss wants to merge 16 commits into
mainfrom
feature/only-classification

dfuchss commented Aug 26, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud Bot commented Aug 26, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		val confusionMatrix: ConfusionMatrix
		get() = microAverage.confusionMatrix

Uh oh!

Conversation

dfuchss commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1. Rank metrics removed

2. Aggregations keep their data

3. calculateAverages returns an object

4. F-beta everywhere

Bugs fixed along the way

Jackson traps worth knowing about

Tests

Swagger UI

Also

Verification

Left for follow-up

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud Bot commented Aug 26, 2026

Quality Gate passed

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dfuchss commented Aug 26, 2026 •

edited

Loading

3. `calculateAverages` returns an object