Add EXCLUDEEMPTY option to TS.MRANGE / TS.MREVRANGE

[mumez]

Summary

• Adds Redis 8.10's EXCLUDEEMPTY option to TS.MRANGE / TS.MREVRANGE.

Changes

• RsTsMRangeOptions: added excludeEmpty/isExcludeEmpty accessors (defaulting to false) and a CRC-style class comment; EXCLUDEEMPTY is intentionally kept out of asArray's output since it belongs at the end of the command, not among the pre-FILTER options.
• RsRedisEndpoint.extension.st: tsMRangeArgsFor:cmdName:range:options:aggOptions:filterBuilder:groupBy: now appends EXCLUDEEMPTY as the final token when options isExcludeEmpty; tsExecuteMRange:rangeBy:filterBy:aggregationBy:groupBy:using: was refactored into smaller private helpers (tsMRangeOptionsFrom:, tsMRangeAggregationOptionsFrom:, tsGroupByFrom:, tsReconcileGroupBy:with:, tsValidateExcludeEmpty:notWithGroupBy:, tsMRangeResultFor:range:options:aggOptions:filterBuilder:groupBy:) and now signals RsError invalidArguments when excludeEmpty is combined with groupBy:, matching Redis's own restriction.
• Since TS.MRANGE and TS.MREVRANGE share this executor, both commands get the feature from one implementation.
• Added/extended tests: RsTsMRangeOptionsTest, RsTsMRangeTest, RsTsMRevRangeTest.
• Added doc/scripting-features/feature-ts-mrange-mrevrange-excludeempty.scripting.md documenting the orchestration used to implement this.

Commits Included

ee6fccc Add EXCLUDEEMPTY option to TS.MRANGE / TS.MREVRANGE

Files Changed

doc/scripting-features/feature-ts-mrange-mrevrange-excludeempty.scripting.md
src/RediStick-TimeSeries-Tests/RsTsMRangeOptionsTest.class.st
src/RediStick-TimeSeries-Tests/RsTsMRangeTest.class.st
src/RediStick-TimeSeries-Tests/RsTsMRevRangeTest.class.st
src/RediStick-TimeSeries/RsRedisEndpoint.extension.st
src/RediStick-TimeSeries/RsTsMRangeOptions.class.st

[mumez]

Hermes Agent Code Review

Verdict: Changes Requested re EXCLUDEEMPTY placement (1 warning + 3 suggestions).

Solid, well-tested feature: the shared-executor reuse for MREVRANGE, the deliberate exclusion of EXCLUDEEMPTY from asArray, and the conflicting-group validation all follow the package conventions. CI is green on Pharo 11/12/13. My comments are below — the main one is the wire-order discrepancy between the implementation and the PR's own documented design.

⚠️ Warning

• RsRedisEndpoint.extension.st:623 — EXCLUDEEMPTY is emitted before the FILTER token, but both the official Redis grammar and this PR's own feature doc require it to be the last argument (after FILTER filterExpr... and after any GROUPBY). CI passes against Redis Stack, so the current server tolerates the position, but the code does not match the documented contract.

💡 Suggestions

• RsTsMRangeOptionsFrom / tsGroupByFrom / tsMRangeAggregationOptionsFrom (543–570) — three identical "if block nil else build" shapes differing only in the instantiated class. DRY up into one helper, or the duplication is fine if you prefer explicit call sites.
• tsValidateExcludeEmpty:notWithGroupBy: (583) — the notWithGroupBy: selector reads awkwardly; consider a name that states the constraint positively.
• RsTsMRangeTest.class.st:193 — raise: Error is too broad; assert the specific RsError so it can't pass on an unrelated failure.

✅ Looks Good

• EXCLUDEEMPTY correctly omitted from asArray — exactly as documented.
• Single shared executor covers both TS.MRANGE and TS.MREVRANGE with a dedicated MREVRANGE test.
• EXCLUDEEMPTY+GROUPBY conflict raised client-side as RsError invalidArguments, mirroring the server rule.

---

Reviewed by Hermes Agent

[mumez]

⚠️ Placement discrepancy. Here (and edited around line 618-623) EXCLUDEEMPTY is appended right after the AGGREGATION options and before args add: 'FILTER', so the wire order is ... AGGREGATION ... EXCLUDEEMPTY FILTER filterExpr.... The PR body and the feature doc both state EXCLUDEEMPTY is the last token (after FILTER filterExpr... and after the optional GROUPBY ... REDUCE ... clause), and the official grammar ... FILTER <l=v...> [GROUPBY label REDUCE reducer] [EXCLUDEEMPTY] agrees. Since you require the flag to be excluded from asArray specifically because it "belongs at the very end of the whole command", it should be appended at the very end — i.e. after the groupBy ifNotNil: [args addAll: groupBy asArray] line, not after aggOptions. Tests pass against the current Redis Stack, so the server tolerates the earlier position, but the code should match your own documented design to avoid breaking on stricter parsing. Fix: move this append to the last statement of the method.

[mumez]

💡 Naming / class comments — the selector tsValidateExcludeEmpty:notWithGroupBy: is hard to read (a notWithGroupBy: keyword reads oddly). Consider naming it so the constraint is stated positively, e.g. tsValidateExcludeEmptyCompatibleWithGroupBy:... or fold the check into a single well-named validator. Not blocking.

[mumez]

💡 raise: Error is too broad — it will pass if any error is raised, not just the intended invalid-arguments signal. Assert the specific error class (RsError, ideally the invalidArguments instance) so the test actually guards the client-side validation and won't turn green on an unrelated failure.

[mumez]

Thanks for the review! Addressed the naming and test-assertion suggestions in 6ae5a71.

Regarding the ⚠️ warning on EXCLUDEEMPTY placement: I tested this empirically against a live Redis 8.10 server before changing it, and moving EXCLUDEEMPTY after FILTER/GROUPBY (as the command's syntax_fmt metadata field states) actually breaks the command:

TS.MRANGE 1000 2000 FILTER type=dbg EXCLUDEEMPTY
-> ERR TSDB: failed parsing labels

FILTER consumes all subsequent tokens as filter expressions, so a trailing EXCLUDEEMPTY gets swallowed as an unterminated filter expr rather than recognized as a flag. Keeping it before FILTER (its original position in this PR) works correctly:

TS.MRANGE 1000 2000 EXCLUDEEMPTY FILTER type=dbg
-> (correct result)

This actually matches the Redis docs' own worked example (TS.MRANGE - 500 WITHLABELS EXCLUDEEMPTY FILTER s=1), which contradicts the syntax_fmt field in the same doc page. I kept the original position and added a comment in the code explaining this discrepancy so it isn't "corrected" back to the broken ordering later. GROUPBY+EXCLUDEEMPTY mutual exclusion still errors correctly regardless of position, confirmed against the live server too. Full test suite (212/212) still green after all fixes.