25 nop-nosql Audit Remediation

Plan Status: completed Last Reviewed: 2026-05-20 Source: ai-dev/audits/2026-05-20-deep-audit-nop-nosql-post-plan23/ Related: 23-nop-nosql-business-pattern-implementation.md (completed)

Purpose

Fix all confirmed live defects and contract drifts identified by the deep audit of nop-nosql (38 findings across 10 dimensions), bringing the module to production quality.

Current Baseline

• Plan 23 completed: 6 business patterns + 4 primitive interfaces + 30 Testcontainers tests, all passing
• Deep audit completed 2026-05-20: 0 P0, 5 P1, 15 P2, 18 P3 findings
• Module compiles and all tests pass (mvn compile + mvn test clean)
• 3 systemic issues dominate: EVALSHA reliability, raw .join() error masking, silent no-op/null returns

Goals

• Fix all 5 P1 findings (production reliability and API contract defects)
• Fix all P2 findings that represent confirmed live defects or contract drifts
• Remove dead code and cosmetic issues (P3) that are trivially fixable
• Achieve clean closure audit by independent sub-agent

Non-Goals

• Adding new features or business patterns beyond Plan 23 scope
• IoC beans.xml registration (explicitly out of scope per Plan 23)
• Refactoring the interface hierarchy (e.g., INosqlHashOperations inheritance from IAsyncMap)
• Performance optimization or benchmarking
• Renaming INosqlListOperations methods from leftPop/rightPop to direction-neutral names (separate decision, deferred)

Scope

In Scope

• All findings from ai-dev/audits/2026-05-20-deep-audit-nop-nosql-post-plan23/summary.md
• Source files in nop-persistence/nop-nosql/ (core + lettuce sub-modules)
• Test file TestLettuceNosqlService.java
• Design doc ai-dev/design/nop-nosql/architecture.md (status table update)

Out Of Scope

• Other modules (nop-stream, nop-job, etc.)
• New business patterns or new test infrastructure
• API breaking changes that would affect downstream consumers outside this repo

Execution Plan

Phase 1 - EVALSHA Fallback (Critical Reliability Fix)

Status: completed Targets: nop-nosql-lettuce/.../LettuceExecutor.java

• Item Types: Fix

• D14-03: Add EVAL fallback in LettuceExecutor.evalScript() — when Redis returns NOSCRIPT error (script not cached, e.g. after restart), catch the error and retry with full EVAL using the script source. The SHA1 hash and script source are both already available from RedisScripts.

Exit Criteria:

• evalScript() catches RedisNoScriptException and retries with EVAL using full script text
• Existing lock unlock / rate limiter / putIfAbsent / removeIfMatch / message CAS operations continue to work
• ./mvnw compile -pl nop-persistence/nop-nosql/nop-nosql-lettuce -am -DskipTests passes
• No owner-doc update required (internal reliability improvement, no contract change)
• ai-dev/logs/ corresponding date entry updated

Phase 2 - Replace Raw .join() with FutureHelper.syncGet() (Error Propagation Fix)

Status: completed Targets: All 10 Lettuce implementation files that have sync methods in nop-nosql-lettuce/.../impl/

• Item Types: Fix

• D09-01 / D14-02 / D14-07: Replace all 66 .toCompletableFuture().join() calls in sync wrapper methods with FutureHelper.syncGet(), which properly unwraps CompletionException to expose the real cause. FutureHelper.syncGet() accepts CompletionStage<T> so .toCompletableFuture() can be dropped. Each impl file needs the import added and all .join() patterns replaced. (Count: LettuceHashOperations=14, LettuceZSetOperations=9, LettuceSetOperations=9, LettuceRanking=8, LettuceSessionStore=7, LettuceQueue=7, LettuceCounter=5, LettuceMessageService=3, LettuceRateLimiter=2, LettuceLock=2)

Exit Criteria:

• Zero .toCompletableFuture().join() calls remain in sync wrapper methods across all 10 impl files (grep confirms)
• All sync methods now propagate the real cause (e.g. RedisCommandExecutionException) instead of CompletionException
• ./mvnw compile -pl nop-persistence/nop-nosql/nop-nosql-lettuce -am -DskipTests passes
• No owner-doc update required (error behavior improvement, no contract signature change)
• ai-dev/logs/ corresponding date entry updated

Phase 3 - Fix Silent No-Op and Null Returns

Status: completed Targets: LettuceMessageService.java, NosqlCache.java, LettuceSessionStore.java

• Item Types: Fix

• D15-09 / D09-02: Fix forEachEntryAsync() to return CompletableFuture.completedFuture(null) instead of literal null (prevents NPE on chaining)

• D09-02: Fix forEachEntry() — either implement properly using HSCAN, or throw UnsupportedOperationException with clear message

• D09-03 / D15-08: Fix getMessageService() — throw UnsupportedOperationException (message pub/sub is not implemented by this module) instead of returning null

• D15-07: Fix NosqlCache.stats() — return new CacheStats(0, 0, 0, 0) instead of null, or annotate with @Nullable and document

Exit Criteria:

• forEachEntryAsync() never returns null CompletionStage
• forEachEntry() either works or throws UnsupportedOperationException
• getMessageService() either works or throws UnsupportedOperationException
• NosqlCache.stats() never returns null (or is annotated @Nullable)
• ./mvnw compile -pl nop-persistence/nop-nosql/nop-nosql-lettuce -am -DskipTests passes
• No owner-doc update required
• ai-dev/logs/ corresponding date entry updated

Phase 4 - Remove Unused Dependency and Dead Code

Status: completed Targets: nop-nosql-core/pom.xml, various impl files

• Item Types: Fix

• D01-01: Remove nop-dao dependency from nop-nosql-core/pom.xml (confirmed zero imports of io.nop.dao in all 20 core source files)

• D17-03: Remove empty LettuceNosqlService class (zero methods, zero usages)

• D17-04: Remove empty NosqlConstants interface (zero constants, zero usages)

• D02-02: Remove unused sync() method from LettuceCounter and LettuceQueue (note: Phase 7's base class will re-introduce inherited sync(), which is acceptable — the goal is eliminating duplicated boilerplate, not removing the method entirely)

• D17-02: Remove unused RedisAdvancedClusterAsyncCommands import from files that don't use it directly (check after other changes)

Exit Criteria:

• nop-dao removed from nop-nosql-core/pom.xml
• LettuceNosqlService.java deleted
• NosqlConstants.java deleted
• ./mvnw compile -pl nop-persistence/nop-nosql/nop-nosql-core,nop-persistence/nop-nosql/nop-nosql-lettuce -am -DskipTests passes
• No owner-doc update required
• ai-dev/logs/ corresponding date entry updated

Phase 5 - Add Sync Methods to INosqlListOperations

Status: completed Targets: INosqlListOperations.java, LettuceListOperations.java

• Item Types: Fix

• D15-01 / D19-F1: Add sync convenience methods to INosqlListOperations matching the pattern of sibling interfaces: getSize(), clear(), add(Object), addAll(Collection<?>), getRange(long, int), trim(long, long), leftPop(), rightPop(), leftPopMulti(int), forEachItem(Consumer<Object>)

• Implement the sync methods in LettuceListOperations using FutureHelper.syncGet() (not raw .join())

Exit Criteria:

• INosqlListOperations has sync methods matching the sync+async convention of sibling interfaces
• LettuceListOperations implements all new sync methods
• ./mvnw compile -pl nop-persistence/nop-nosql/nop-nosql-lettuce -am -DskipTests passes
• ai-dev/design/nop-nosql/architecture.md §3.4 updated to note sync methods added
• ai-dev/logs/ corresponding date entry updated

Phase 6 - Defensive Programming and API Documentation

Status: completed Targets: LettuceCounter.java, LettuceRedisConnectionProvider.java, INosqlSessionStore.java, INosqlHashOperations.java, LettuceLock.java, various interfaces

• Item Types: Fix

• D14-04: Add defensive NumberFormatException handling in LettuceCounter.toLong() — if GET returns non-numeric value, throw NopException with ErrorCode instead of propagating raw NumberFormatException

• D14-06: Add isStarted() guard in LettuceRedisConnectionProvider.getConnection() — throw clear error instead of NPE if called after stop()

• D03-02: Add Javadoc to INosqlSessionStore.set() documenting that HMSET+PEXPIRE is non-atomic (PEXPIRE may fail silently)

• D03-03: Add Javadoc to INosqlHashOperations methods inherited from INosqlKeyValueOperations (putExAsync, getExAsync, setTimeoutAsync) documenting that expiry applies to the entire hash key, not individual fields

• D03-05: Add Javadoc to business pattern interfaces documenting edge-case return values: INosqlQueue.dequeue() returns null on empty, INosqlRanking.getRank() returns -1 for missing member, INosqlCounter.get() returns 0 for missing key, INosqlRanking.getScore() returns 0.0 for missing member

• D14-01: Add Javadoc to LettuceLock clarifying single-thread-per-instance usage model

• D14-05: Add Javadoc to LettuceRateLimiter noting that caller must use consistent config across calls

• D15-05: Add defensive type check in LettuceRateLimiter for Lua eval result — validate result is Object[] with expected length before casting, throw NopException with ErrorCode if format is unexpected

Exit Criteria:

• toLong() wraps NumberFormatException into a proper NopException
• getConnection() throws clear error after stop()
• All Javadoc additions are in place for edge-case return values and non-atomicity warnings
• ./mvnw compile -pl nop-persistence/nop-nosql/nop-nosql-lettuce -am -DskipTests passes
• No owner-doc update required (Javadoc is in-source documentation)
• ai-dev/logs/ corresponding date entry updated

Phase 7 - Code Quality Cleanup

Status: completed Targets: Various impl files

• Item Types: Fix

• D02-01: Extract AbstractLettuceOperations base class with client field, async() and sync() methods; update all 11 impl classes to extend it

• D17-01: Fix import order in NosqlCache.java (jakarta imports between io.nop and java.util)

• D17-05: Remove AI-style inline comments in LettuceSessionStore that restate Redis command names

• D17-06: Remove AI-style comment in LettuceRanking

• D15-02: Fix raw type cast (Map) in LettuceHashOperations.putAllAsync — use helper or suppress with comment

• D15-03: Fix raw type cast (Map) in LettuceMessageService.putAllAsync — same pattern as D15-02, same fix

• D15-04: Fix unnecessary redundant cast in LettuceSessionStore.getAsync

• D17-07: Remove AI-style block comment in LettuceRateLimiter

Exit Criteria:

• AbstractLettuceOperations extracted and all 11 impl classes extend it
• Import order in NosqlCache follows java.* → jakarta.* → third-party → io.nop.*
• AI-style inline comments removed from LettuceSessionStore and LettuceRanking
• ./mvnw compile -pl nop-persistence/nop-nosql/nop-nosql-lettuce -am -DskipTests passes
• No owner-doc update required
• ai-dev/logs/ corresponding date entry updated

Phase 8 - Test Edge Cases

Status: completed Targets: TestLettuceNosqlService.java

• Item Types: Proof

• D16-03: Add edge case tests (all behind @Testcontainers(disabledWithoutDocker=true)):

  • Counter: get() on non-existent key returns 0
  • Queue: dequeueBatch(maxCount) when fewer items exist
  • Ranking: getRank() for missing member returns -1
  • Ranking: getScore() for missing member returns 0.0
  • Ranking: getTopN() on empty ranking
  • RateLimiter: tryAcquire(0) permits behavior
  • Lock: double unlock() is idempotent (no exception)
  • SessionStore: get() for non-existent session returns null or empty map

Exit Criteria:

• All 8+ edge case tests added and pass (or skip when no Docker)
• ./mvnw test -pl nop-persistence/nop-nosql/nop-nosql-lettuce -am passes (tests skip without Docker)
• No owner-doc update required
• ai-dev/logs/ corresponding date entry updated

Closure Gates

• All 5 P1 findings have landed (EVALSHA fallback, .join() replacement, INosqlListOperations sync methods, forEachEntry null fix, nop-dao removal)
• All P2 live defects and contract drifts have been fixed
• Dead code (LettuceNosqlService, NosqlConstants, unused sync() methods) removed
• Edge case tests cover all 6 business patterns
• No silent no-op or null returns remain in public API (except where annotated @Nullable)
• ./mvnw compile -pl nop-persistence/nop-nosql/nop-nosql-core,nop-persistence/nop-nosql/nop-nosql-lettuce -am -DskipTests passes
• ./mvnw test -pl nop-persistence/nop-nosql/nop-nosql-lettuce -am passes
• Independent sub-agent closure audit completed and evidence recorded
• ai-dev/design/nop-nosql/architecture.md §6 status table updated
• ai-dev/logs/ has corresponding date entry

Deferred But Adjudicated

D01-02: INosqlListOperations uses Redis command names (leftPop/rightPop)

• Classification: out-of-scope improvement
• Why Not Blocking Closure: Renaming methods is a breaking API change requiring consumer migration. The audit finding is about naming aesthetics, not functional correctness. The current names are unambiguous and match Lettuce terminology that Redis users expect. A rename decision should be made deliberately with downstream impact analysis.
• Successor Required: no

D03-01: Ranking vs ZSetOperations inconsistent null handling

• Classification: watch-only residual
• Why Not Blocking Closure: INosqlRanking and INosqlZSetOperations serve different abstraction levels. Ranking is a business pattern that normalizes edge cases (returning 0/-1), while ZSetOperations is a lower-level primitive closer to raw Redis semantics. The difference is intentional per the layered architecture.
• Successor Required: no

D03-06: INosqlLock.isHeld() checks local state only

• Classification: watch-only residual
• Why Not Blocking Closure: isHeld() is designed as a fast local check (no Redis round-trip). Adding a Redis call would change the performance profile. The Javadoc in Phase 6 will document this behavior clearly.
• Successor Required: no

D19-F2: Primitive layer uses 3 different names for "element count"

• Classification: optimization candidate
• Why Not Blocking Closure: The naming variation (getSize/size/card) reflects the different underlying Redis commands (LLEN/SCARD/ZCARD) and the different interface inheritance chains. Unifying would require breaking changes to multiple interfaces.
• Successor Required: no

D03-04 / D15-06: CompletionStage vs CompletableFuture return type inconsistency

• Classification: watch-only residual
• Why Not Blocking Closure: Some async methods return CompletionStage<Void> (from IAsyncMap inheritance) while others return CompletableFuture<Void>. Both are valid Java async types and Lettuce API works with both. The inconsistency is inherited from the parent interface IAsyncMap and would require upstream changes to resolve.
• Successor Required: no

D01-03: INosqlZSetOperations uses Redis-specific terminology (card, revRank)

• Classification: out-of-scope improvement
• Why Not Blocking Closure: INosqlZSetOperations is a primitive-level interface (not a business pattern). Using Redis terminology at this level is acceptable — it maps closely to the underlying data model. Renaming would be a breaking change for consumers who expect Redis-aligned naming at the primitive layer.
• Successor Required: no

D17-08: protected methods that could be private

• Classification: optimization candidate
• Why Not Blocking Closure: Protected visibility on async()/sync() methods is intentional for extensibility — subclasses or test extensions may need access. After Phase 7 extracts the base class, these methods will be in the base class where protected is the standard pattern.
• Successor Required: no

Non-Blocking Follow-ups

• Consider adding Jedis driver implementation alongside Lettuce (if demand arises)
• Consider adding metrics/tracing integration for Redis operations
• Consider adding connection pool configuration options beyond basic RedisConfig

Closure

Status Note: All 8 phases executed and verified. 38 audit findings addressed (5 P1 fixed, 15 P2 fixed, 18 P3 fixed/adjudicated). Independent Oracle closure audit passed after D14-04 fix. Build compiles, all 37 tests pass (skipped without Docker as designed).

Closure Audit Evidence:

• Reviewer / Agent: Oracle (ses_1bc9868b7ffejiiljS29yn1cZL, bg_60b3e340) + re-verification (ses_1bc927467ffeEoDTlPfb6dO6kk)
• Evidence: All closure gates verified against source code. One initial failure (D14-04 toLong missing NopException wrap) fixed and re-verified.

Follow-up:

• no remaining plan-owned work