knowsys / knowsys/nemo

Mixed numeric #min/#max compare storage types instead of values

Open
#805 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
289
Forks
21
PR merge metrics
No merged PRs in 30d

Description

## Description

`#min` and `#max` return the wrong result when an aggregate group mixes integer and floating-point values.

The aggregate processors compare `StorageValueT` directly. Its derived ordering is by enum variant (`Int64 < Float < Double`) rather than numeric magnitude, so the result is biased toward the storage type:

- `#min` prefers an integer even when it is numerically larger.
- `#max` prefers a double even when it is numerically smaller.

## Reproduction

```prolog
input(10).
input(1.0).

result("min", #min(?value)) :- input(?value).
result("max", #max(?value)) :- input(?value).
```

Run:

```console
cargo run -q -p nemo-cli -- \
--report none --print-facts idb mixed_minmax.rls
```

Actual output:

```text
result("max", "1"^^).
result("min", 10).
```

Expected numeric extrema:

```text
result("max", "10"^^).
result("min", "1"^^).
```

This matches the existing mixed-numeric promotion used by the `MIN`/`MAX` built-ins and by `#sum` after #801.

## Impact

Rules that aggregate heterogeneous numeric data silently produce incorrect analytical results while execution succeeds.

## Cause

`MinAggregateGroupProcessor` and `MaxAggregateGroupProcessor` use `<` and `>` on `StorageValueT`. That ordering is intentionally type-first for internal storage and is not suitable for numeric aggregate semantics.

The fix should be local to the aggregate processors; changing global `StorageValueT::Ord` could affect storage and sorting behavior.

## Acceptance criteria

- Mixed integer/float, integer/double, and float/double groups use numeric magnitudes for both `#min` and `#max`.
- Mixed groups follow Nemo's existing numeric promotion behavior.
- Same-type integer, float, and double behavior remains unchanged.
- Focused regression tests cover both input orders so results do not depend on row order.
- Existing tests, Clippy, formatting, and documentation checks pass.

## Related work

- #638 reports string `#min`; it is related to semantic comparison but does not cover mixed numeric aggregates.
- #800 / #801 fixed mixed numeric `#sum` only.

Reproduced twice on `main` at `d109c690250ff341198ee61111f0da2b115f8362`.

Contributor guide

Open the contributing guide

Research direction

Start by locating MinAggregateGroupProcessor and MaxAggregateGroupProcessor, then compare their numeric handling with the existing MIN/MAX promotion and the #sum changes from #801. Run the provided nemo-cli reproduction first and find the aggregate test location. Done means mixed integer/float/double inputs produce numeric extrema in both input orders while same-type behavior and the requested checks remain passing.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
data
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.