reclassify some of the ops

mgravell · Jul 31, 2026 · Jul 31, 2026 · Jul 31, 2026 · Jul 31, 2026 · Jul 31, 2026
commit cc8dbe982f2426778bb7689cbeebfc119967ccdd
diff --git a/src/NRedisStack/Auxiliary.cs b/src/NRedisStack/Auxiliary.cs
@@ -84,7 +84,7 @@ internal static async Task<RedisResult> ExecuteAsync(this IServer server, int? d
     }
 
     public static List<RedisResult> ExecuteBroadcast(this IDatabase db, string command)
-        => db.ExecuteBroadcast(CommandFlags.None, command);
+        => db.ExecuteBroadcast(SerializedCommand.Uncategorized(command));
 
     public static List<RedisResult> ExecuteBroadcast(this IDatabase db, CommandFlags category, string command)
         => db.ExecuteBroadcast(new SerializedCommand(category, command));
@@ -111,7 +111,7 @@ public static List<RedisResult> ExecuteBroadcast(this IDatabase db, SerializedCo
     }
 
     public static async Task<List<RedisResult>> ExecuteBroadcastAsync(this IDatabaseAsync db, string command)
-        => await db.ExecuteBroadcastAsync(CommandFlags.None, command);
+        => await db.ExecuteBroadcastAsync(SerializedCommand.Uncategorized(command));
 
     public static async Task<List<RedisResult>> ExecuteBroadcastAsync(this IDatabaseAsync db, CommandFlags category, string command)
         => await db.ExecuteBroadcastAsync(new SerializedCommand(category, command));

diff --git a/src/NRedisStack/Bloom/BloomCommandBuilder.cs b/src/NRedisStack/Bloom/BloomCommandBuilder.cs
@@ -80,7 +80,7 @@ public static SerializedCommand Reserve(RedisKey key, double errorRate, long cap
             args.Add(BloomArgs.NONSCALING);
         }
 
-        return new(CommandCategories.WriteChecked, BF.RESERVE, args);
+        return new(CommandCategories.WriteAccumulating, BF.RESERVE, args);
     }
 
     public static SerializedCommand ScanDump(RedisKey key, long iterator)

diff --git a/src/NRedisStack/CountMinSketch/CmsCommandBuilder.cs b/src/NRedisStack/CountMinSketch/CmsCommandBuilder.cs
@@ -34,12 +34,12 @@ public static SerializedCommand Info(RedisKey key)
 
     public static SerializedCommand InitByDim(RedisKey key, long width, long depth)
     {
-        return new(CommandCategories.WriteChecked, CMS.INITBYDIM, key, width, depth);
+        return new(CommandCategories.WriteAccumulating, CMS.INITBYDIM, key, width, depth);
     }
 
     public static SerializedCommand InitByProb(RedisKey key, double error, double probability)
     {
-        return new(CommandCategories.WriteChecked, CMS.INITBYPROB, key, error, probability);
+        return new(CommandCategories.WriteAccumulating, CMS.INITBYPROB, key, error, probability);
     }
 
     public static SerializedCommand Merge(RedisValue destination, long numKeys, RedisValue[] source,

diff --git a/src/NRedisStack/CuckooFilter/CuckooCommandBuilder.cs b/src/NRedisStack/CuckooFilter/CuckooCommandBuilder.cs
@@ -134,7 +134,7 @@ public static SerializedCommand Reserve(RedisKey key, long capacity,
             args.Add(expansion);
         }
 
-        return new(CommandCategories.WriteChecked, CF.RESERVE, args);
+        return new(CommandCategories.WriteAccumulating, CF.RESERVE, args);
     }
 
     public static SerializedCommand ScanDump(RedisKey key, long iterator)

diff --git a/src/NRedisStack/RedisStackCommands/CommandCategories.cs b/src/NRedisStack/RedisStackCommands/CommandCategories.cs
@@ -37,12 +37,30 @@ internal static class CommandCategories
     internal const CommandFlags WriteLastWins = (CommandFlags)(16 << 13);
 
     /// <summary>Cumulative write; a replay double-applies and changes the result.</summary>
+    /// <remarks>
+    /// Also carries a second, less obvious class of command: one whose replay leaves state *correct* but
+    /// returns an error, because the server rejects the repeat ("Index already exists", "key already
+    /// exists", "compaction rule does not exist"). Most module DDL behaves this way. That is not literally
+    /// cumulative, but the ladder has no rung for "idempotent, yet errors on replay", and the alternatives
+    /// are worse: <see cref="WriteChecked"/> and <see cref="WriteLastWins"/> both sit at or below the
+    /// default policy ceiling, so a lost reply would be replayed *by default* and report failure for an
+    /// operation that actually succeeded. Sitting here keeps that closed unless the caller deliberately
+    /// raises the ceiling, while still allowing the known-never-sent replay (see <see cref="Never"/>),
+    /// which is the case that matters for riding out <c>-LOADING</c> during setup.
+    /// </remarks>
     internal const CommandFlags WriteAccumulating = (CommandFlags)(20 << 13);
 
     /// <summary>Server administration, e.g. <c>FT.CONFIG SET</c>.</summary>
     internal const CommandFlags ServerAdmin = (CommandFlags)(24 << 13);
 
     /// <summary>Never replay, under any policy.</summary>
+    /// <remarks>
+    /// The only category a retry policy cannot override. StackExchange.Redis tests for it before both the
+    /// <c>MaxCommandRetryCategory</c> comparison and the known-never-sent (<c>NotApplied</c>) bypass, so
+    /// unlike the rungs below it this holds even when a caller raises the ceiling - which they may do as
+    /// far as <c>Never</c> itself. Use it where a replay would corrupt state or report a spurious error
+    /// even though the original attempt succeeded, and where forgoing retry entirely is the lesser cost.
+    /// </remarks>
     internal const CommandFlags Never = (CommandFlags)(31 << 13);
 
     /// <summary>
@@ -57,10 +75,17 @@ internal static class CommandCategories
     /// </remarks>
     internal const CommandFlags ServerSpecific = (CommandFlags)(1 << 18);
 
+    /// <summary>
+    /// The rung bits (13-17): the severity ladder on its own, without <see cref="ServerSpecific"/>. A
+    /// category is only meaningful if it names a rung, since <see cref="ServerSpecific"/> alone leaves
+    /// StackExchange.Redis to substitute a default rung.
+    /// </summary>
+    internal const CommandFlags LadderMask = Never;
+
     /// <summary>
     /// The bits a caller-supplied category may occupy: the retry-category region (13-17) plus
     /// <see cref="ServerSpecific"/> (18). Anything else is masked off before dispatch, so a stray
     /// <c>FireAndForget</c> or replica preference cannot leak in through the category parameter.
     /// </summary>
-    internal const CommandFlags Mask = Never | ServerSpecific;
+    internal const CommandFlags Mask = LadderMask | ServerSpecific;
 }
diff --git a/src/NRedisStack/RedisStackCommands/SerializedCommand.cs b/src/NRedisStack/RedisStackCommands/SerializedCommand.cs
@@ -30,30 +30,65 @@ public class SerializedCommand
     /// </summary>
     internal CommandFlags EffectiveFlags => BaseFlags | CommandCategory;
 
+    // the unvalidated path: the obsolete ctors and Uncategorized() deliberately yield None, which the
+    // validating ctors reject
+    private SerializedCommand(string command, object[] args, CommandFlags category)
+    {
+        CommandCategory = category & CommandCategories.Mask;
+        Command = command;
+        Args = args;
+    }
+
+    /// <summary>
+    /// Builds a command with no declared category, which StackExchange.Redis will never retry. Only for
+    /// entry points that take a bare command string and so have nothing to categorize.
+    /// </summary>
+    internal static SerializedCommand Uncategorized(string command, params object[] args)
+        => new(command, args, CommandFlags.None);
+
     [Obsolete("Specify the command's side-effect category, so that it can participate in WithRetry; uncategorized commands are never retried.")]
     public SerializedCommand(string command, params object[] args)
-        : this(CommandFlags.None, command, args)
+        : this(command, args, CommandFlags.None)
     {
     }
 
     [Obsolete("Specify the command's side-effect category, so that it can participate in WithRetry; uncategorized commands are never retried.")]
     public SerializedCommand(string command, ICollection<object> args)
-        : this(CommandFlags.None, command, args.ToArray())
+        : this(command, args.ToArray(), CommandFlags.None)
     {
     }
 
+    /// <param name="category">
+    /// The command's side-effect category; see the <c>CommandFlags.CommandRetry*</c> values. Must contain
+    /// at least one category bit.
+    /// </param>
+    /// <param name="command">The command name.</param>
+    /// <param name="args">The command arguments.</param>
+    /// <exception cref="ArgumentOutOfRangeException">
+    /// If <paramref name="category"/> contains no category bits.
+    /// </exception>
     public SerializedCommand(CommandFlags category, string command, params object[] args)
+        : this(command, args, Validate(category))
     {
-        CommandCategory = category & CommandCategories.Mask;
-        Command = command;
-        Args = args;
     }
 
+    /// <inheritdoc cref="SerializedCommand(CommandFlags, string, object[])"/>
     public SerializedCommand(CommandFlags category, string command, ICollection<object> args)
-        : this(category, command, args.ToArray())
+        : this(command, args.ToArray(), Validate(category))
     {
     }
 
+    // A category naming no rung is a mistake worth surfacing rather than silently accepting: it means the
+    // caller passed default/None, or only flags that do not belong here (e.g. FireAndForget), or only the
+    // server-specific bit - and the result would quietly be "never retry" wearing the costume of a
+    // declaration. Tested against the ladder rather than the full mask precisely so that the
+    // server-specific bit on its own does not satisfy it.
+    private static CommandFlags Validate(CommandFlags category)
+        => (category & CommandCategories.LadderMask) != 0
+            ? category
+            : throw new ArgumentOutOfRangeException(nameof(category), category,
+                "No command category specified; expected one of the CommandFlags.CommandRetry* values.");
+
     /// <inheritdoc />
     public override string ToString() => Args is { Length: > 0 }
         ? (Command + " " + string.Join(" ", Args))

diff --git a/src/NRedisStack/Search/SearchCommandBuilder.cs b/src/NRedisStack/Search/SearchCommandBuilder.cs
@@ -16,19 +16,32 @@ public static SerializedCommand Aggregate(string index, AggregationRequest query
         List<object> args = [index];
         query.SerializeRedisArgs();
         args.AddRange(query.GetArgs());
-        // WITHCURSOR leaves a cursor behind on the serving node; replaying elsewhere would both orphan
-        // that cursor and hand back a cursor id the follow-up FT.CURSOR calls cannot use
-        return new(query.IsWithCursor() ? CommandCategories.ReadOnly | CommandCategories.ServerSpecific : CommandCategories.ReadOnly, FT.AGGREGATE, args);
+        return new(AggregateCategory(query), FT.AGGREGATE, args);
     }
 
+    /// <summary>
+    /// A plain aggregate is a pure read, but WITHCURSOR allocates cursor state on the serving node, so
+    /// every replay leaks another cursor (held until its idle timeout) and returns an id the caller's
+    /// follow-up FT.CURSOR calls will not be using.
+    /// </summary>
+    /// <remarks>
+    /// Never, not merely a high rung. ServerSpecific is not enough, because it only strips failover and a
+    /// same-server replay orphans a cursor just as effectively. Nor is WriteAccumulating: that relies on
+    /// the policy's default ceiling, and a caller may legitimately raise MaxCommandRetryCategory - which
+    /// says "I accept double-applied writes", not "I accept leaked cursors". Never is the only category
+    /// no policy can override, since the ceiling itself may be set as high as Never.
+    /// </remarks>
+    private static CommandFlags AggregateCategory(AggregationRequest query)
+        => query.IsWithCursor() ? CommandCategories.Never : CommandCategories.ReadOnly;
+
     public static SerializedCommand AliasAdd(string alias, string index)
     {
-        return new(CommandCategories.WriteChecked, FT.ALIASADD, alias, index);
+        return new(CommandCategories.WriteAccumulating, FT.ALIASADD, alias, index);
     }
 
     public static SerializedCommand AliasDel(string alias)
     {
-        return new(CommandCategories.WriteLastWins, FT.ALIASDEL, alias);
+        return new(CommandCategories.WriteAccumulating, FT.ALIASDEL, alias);
     }
 
     public static SerializedCommand AliasUpdate(string alias, string index)
@@ -50,7 +63,7 @@ public static SerializedCommand Alter(string index, Schema schema, bool skipInit
         {
             f.AddSchemaArgs(args);
         }
-        return new(CommandCategories.WriteChecked, FT.ALTER, args);
+        return new(CommandCategories.WriteAccumulating, FT.ALTER, args);
     }
 
     [Obsolete("Starting from Redis 8.0, use db.ConfigGet instead")]
@@ -79,19 +92,24 @@ public static SerializedCommand Create(string indexName, FTCreateParams paramete
             f.AddSchemaArgs(args);
         }
 
-        return new(CommandCategories.WriteChecked, FT.CREATE, args);
+        return new(CommandCategories.WriteAccumulating, FT.CREATE, args);
     }
 
     public static SerializedCommand CursorDel(string indexName, long cursorId)
     {
-        // deleting an already-deleted cursor is a no-op, but only the node holding it can do it
-        return new(CommandCategories.WriteLastWins | CommandCategories.ServerSpecific, FT.CURSOR, "DEL", indexName, cursorId);
+        // RediSearch errors ("Cursor does not exist") rather than treating a repeat delete as an OK no-op,
+        // so replaying a delete whose reply was merely lost reports a failure for cleanup that actually
+        // succeeded. Never rather than a high rung, since the policy ceiling is caller-settable and
+        // forgoing retry costs little here - an undeleted cursor expires by idle timeout anyway.
+        return new(CommandCategories.Never, FT.CURSOR, "DEL", indexName, cursorId);
     }
 
     public static SerializedCommand CursorRead(string indexName, long cursorId, int? count = null)
     {
-        // reading advances the cursor, so a replay silently skips a page rather than re-reading one
-        const CommandFlags Category = CommandCategories.WriteAccumulating | CommandCategories.ServerSpecific;
+        // Reading advances the cursor, so a replay silently skips a page rather than re-reading one - i.e.
+        // data loss, not just a wasted round trip. Never for the same reason as CursorDel: no lower rung
+        // survives a caller-raised MaxCommandRetryCategory.
+        const CommandFlags Category = CommandCategories.Never;
         return ((count == null) ? new(Category, FT.CURSOR, "READ", indexName, cursorId)
             : new SerializedCommand(Category, FT.CURSOR, "READ", indexName, cursorId, "COUNT", count));
     }
@@ -135,8 +153,8 @@ public static SerializedCommand DictDump(string dict)
 
     public static SerializedCommand DropIndex(string indexName, bool dd = false)
     {
-        return ((dd) ? new(CommandCategories.WriteLastWins, FT.DROPINDEX, indexName, "DD")
-            : new SerializedCommand(CommandCategories.WriteLastWins, FT.DROPINDEX, indexName));
+        return ((dd) ? new(CommandCategories.WriteAccumulating, FT.DROPINDEX, indexName, "DD")
+            : new SerializedCommand(CommandCategories.WriteAccumulating, FT.DROPINDEX, indexName));
     }
 
     public static SerializedCommand Explain(string indexName, string query, int? dialect)
@@ -190,8 +208,8 @@ public static SerializedCommand ProfileAggregate(string IndexName, AggregationRe
 
         query.SerializeRedisArgs();
         args.AddRange(query.GetArgs());
-        // as for Aggregate: profiling a cursored aggregate still creates the cursor
-        return new(query.IsWithCursor() ? CommandCategories.ReadOnly | CommandCategories.ServerSpecific : CommandCategories.ReadOnly, FT.PROFILE, args);
+        // profiling a cursored aggregate still allocates the cursor, so it carries the same category
+        return new(AggregateCategory(query), FT.PROFILE, args);
     }
 
     public static SerializedCommand SpellCheck(string indexName, string query, FTSpellCheckParams? spellCheckParams = null)

diff --git a/src/NRedisStack/Tdigest/TdigestCommandBuilder.cs b/src/NRedisStack/Tdigest/TdigestCommandBuilder.cs
@@ -27,7 +27,7 @@ public static SerializedCommand CDF(RedisKey key, params double[] values)
 
     public static SerializedCommand Create(RedisKey key, long compression = 100)
     {
-        return new(CommandCategories.WriteChecked, TDIGEST.CREATE, key, TdigestArgs.COMPRESSION, compression);
+        return new(CommandCategories.WriteAccumulating, TDIGEST.CREATE, key, TdigestArgs.COMPRESSION, compression);
     }
 
     public static SerializedCommand Info(RedisKey key)

diff --git a/src/NRedisStack/TimeSeries/TimeSeriesCommandsBuilder.cs b/src/NRedisStack/TimeSeries/TimeSeriesCommandsBuilder.cs
@@ -16,12 +16,12 @@ public static class TimeSeriesCommandsBuilder
     public static SerializedCommand Create(string key, long? retentionTime = null, IReadOnlyCollection<TimeSeriesLabel>? labels = null, bool? uncompressed = null, long? chunkSizeBytes = null, TsDuplicatePolicy? duplicatePolicy = null)
     {
         var parameters = new TsCreateParams(retentionTime, labels, uncompressed, chunkSizeBytes, duplicatePolicy);
-        return new(CommandCategories.WriteChecked, TS.CREATE, parameters.ToArray(key));
+        return new(CommandCategories.WriteAccumulating, TS.CREATE, parameters.ToArray(key));
     }
 
     public static SerializedCommand Create(string key, TsCreateParams parameters)
     {
-        return new(CommandCategories.WriteChecked, TS.CREATE, parameters.ToArray(key));
+        return new(CommandCategories.WriteAccumulating, TS.CREATE, parameters.ToArray(key));
     }
 
     #endregion
@@ -101,13 +101,13 @@ public static SerializedCommand CreateRule(string sourceKey, TimeSeriesRule rule
         var args = new List<object> { (RedisKey)sourceKey };
         args.AddRule(rule);
         args.Add(alignTimestamp);
-        return new(CommandCategories.WriteChecked, TS.CREATERULE, args);
+        return new(CommandCategories.WriteAccumulating, TS.CREATERULE, args);
     }
 
     public static SerializedCommand DeleteRule(string sourceKey, string destKey)
     {
         var args = new List<object> { (RedisKey)sourceKey, (RedisKey)destKey };
-        return new(CommandCategories.WriteLastWins, TS.DELETERULE, args);
+        return new(CommandCategories.WriteAccumulating, TS.DELETERULE, args);
     }
 
     #endregion

diff --git a/src/NRedisStack/TopK/TopKCommandBuilder.cs b/src/NRedisStack/TopK/TopKCommandBuilder.cs
@@ -61,6 +61,6 @@ public static SerializedCommand Query(RedisKey key, params RedisValue[] items)
 
     public static SerializedCommand Reserve(RedisKey key, long topk, long width = 7, long depth = 8, double decay = 0.9)
     {
-        return new(CommandCategories.WriteChecked, TOPK.RESERVE, key, topk, width, depth, decay);
+        return new(CommandCategories.WriteAccumulating, TOPK.RESERVE, key, topk, width, depth, decay);
     }
 }