fix: allow lazy load evaluations when $inited key is not set

[kinyoklion]

Summary

In lazy load / daemon mode, the SDK previously gated all flag evaluations on whether the $inited key existed in the persistent store (e.g. Redis). In daemon mode, an external process (Relay Proxy, another SDK) populates the store, but $inited may not always be present — causing every evaluation to return CLIENT_NOT_READY even though flag data is available for on-demand fetching.

This PR changes LazyLoad to match the Go, Java, and .NET SDK daemon-mode pattern:

• Initialized() always returns true — the data system is always considered initialized since it can fetch on demand. This matches Go/Java/.NET's NullDataSource.IsInitialized() → true.
• Initialize() always sets kValid immediately — the StartAsync future resolves right away, matching Go: close(closeWhenReady), Java: CompletableFuture.completedFuture(null), .NET: Task.FromResult(true).
• Warning logged once at init when the store's $inited key is missing, consistent with the Node SDK pattern where the data source initializes immediately but the store state drives the warning.
• No changes to client_impl.cpp — PreEvaluationChecks and AllFlagsState use their original logic; since Initialized() returns true, evaluations proceed naturally without special casing.

Removed

• CanEvaluateWhenNotInitialized() virtual method (removed from IDataSystem interface and LazyLoad)
• RefreshInitState(), initialized_ member, Keys::kInitialized — no longer needed since Initialized() is unconditional

Files changed:

• lazy_load_system.cpp — Initialize() always sets kValid; Initialized() always returns true; removed RefreshInitState()
• lazy_load_system.hpp — removed RefreshInitState() declaration, initialized_ member, kInitialized key
• lazy_load_system_test.cpp — replaced old Initialized() caching/TTL tests with tests for: always-true Initialized(), immediate kValid status, warning behavior based on store state

Review & Testing Checklist for Human

• reader_->Initialized() error handling in Initialize(): The store Initialized() check is called directly (not cached/TTL-gated). If the persistent store is unreachable at SDK startup, verify this doesn't throw or block indefinitely. The warning path handles false, but an exception or hang would be problematic.
• Store-unreachable evaluation behavior: Since Initialized() always returns true, PreEvaluationChecks will never return CLIENT_NOT_READY for LazyLoad. If the store is completely down, evaluations will attempt to fetch and return defaults. Verify this is acceptable behavior (Go SDK behaves identically via NullDataSource).
• Contract test verification: This fix addresses Issue feat: eventsource client #1 from PR chore: Add support for persistent store contract tests. #502's persistence test failures. After merging fix: allow lazy load evaluations when $inited key is not set #508 then chore: Add support for persistent store contract tests. #502, run the 10 daemon-mode persistent data store contract tests end-to-end against Redis to verify they pass.
• spy_logger_backend test API: Tests use Contains(0, LogLevel::kWarn, "$inited") — verify the 0 parameter (message index?) is correct. If it means "first message," then tests are correct; if it's a match count, could be wrong.

Notes

• This PR was not built or tested locally (CMake toolchain unavailable on dev machine). CI is the first compilation check — all 42 checks pass.
• The approach now matches Go (NullDataSource.IsInitialized() → true, Start() → close(closeWhenReady)), Java (NullDataSource.isInitialized() → true, start() → CompletableFuture.completedFuture(null), updateStatus(VALID)), and .NET (NullDataSource.Initialized → true, Start() → Task.FromResult(true)) daemon mode patterns.
• Related: PR #502 (persistence contract tests) — that PR has the TTL field name fix; this PR addresses the initialization gating issue.

Link to Devin session | Requested by: @kinyoklion

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment and CI monitoring

[kinyoklion]

Devin, review the human checklist for items which you can verify.

[devin-ai-integration]

I've reviewed the human checklist items. Here's what I can verify:

1. Warning log volume ⚠️ Confirmed — no rate limiting

PreEvaluationChecks (line 430) and AllFlagsState (line 190) both log a warning on every call when !Initialized() and CanEvaluateWhenNotInitialized() is true. There is no "log once" or rate-limiting guard.

In a daemon-mode deployment where $inited is never set (which is the normal case when not using Relay Proxy), every single flag evaluation will produce a warning. This could be extremely noisy under load.

Recommendation: Add a std::once_flag or std::atomic<bool> member to ClientImpl to log the warning only once per client lifetime. This is consistent with the pattern used in other SDKs (Go's NullDataSource avoids the warning entirely; Erlang logs once on first access).

2. StartAsync() behavior ⚠️ Confirmed — future may never resolve

StartAsync() (line 162-178) registers a status change listener that calls pr->set_value(true) only when data_system_->Initialized() returns true. LazyLoad::Initialize() (line 80-85) only sets kValid if Initialized() returns true at startup. If $inited is never set in the store, the status stays at kInitializing and the future never resolves.

In entity_manager.cpp line 226, the contract test service calls init.wait_for(waitForClient) with a timeout (default 5s), so it won't hang — but the client will never report as "started." This is acceptable for daemon mode since evaluations proceed via the CanEvaluateWhenNotInitialized() path, but worth documenting.

3. AllFlagsState when store is empty/unreachable ✓ Verified — behavior is reasonable

When AllFlagsState proceeds (line 206), it calls data_system_->AllFlags() which goes through LazyLoad::AllFlags(). This triggers RefreshAllFlags() which fetches from the store. If the store is:

• Empty (no flags, but reachable): Returns an empty map → AllFlagsState returns an empty-but-valid result (not an error). This is correct — there genuinely are no flags.
• Unreachable: RefreshAll catches the error, logs it, sets status to kInterrupted, and the cache returns whatever was previously cached (or empty if first call). This degrades gracefully.

Both cases are reasonable. The previous behavior (returning {} immediately) was actually worse because it didn't even attempt to fetch data that might be available.

4. Contract test verification ✓ Verified via PR #502 CI

PR #502 (which includes this fix via merge) has all 45 CI checks passing, including the Redis persistent store contract tests:

• 10 daemon-mode tests PASS (uses default prefix, uses custom prefix, ignores database initialization flag, can disable cache, caches flag for duration × 2, caches flag forever, plus prefix variants)
• 12 read-write tests properly SKIPPED via suppressions file (not yet implemented)
• No regressions in any other contract tests

---

Summary: Items 3 and 4 are verified and good. Items 1 and 2 are confirmed as noted in the checklist — item 1 (warning volume) is the most actionable concern. Want me to add a "log once" guard for the warning?