Deploy/session 37 phase 4

[MarcoPolo483]

No description provided.

[Copilot]

Pull request overview

This PR delivers Session 37 Phase 4 improvements to the EVA Data Model API, focusing on fixing 8 previously failing cache layer tests and introducing supporting documentation. The core changes fix the CacheLayer.set() signature to accept an optional ttl_seconds parameter, align invalidation event processing to work without a running background loop, and update test cache-key prefixes to match the projects: entity type. Two documentation/planning files are also added.

Changes:

• Fix CacheLayer.set() to accept an optional ttl_seconds TTL override and propagate it to L1 and L2 caches.
• Change CacheInvalidationManager.emit_event() to process events synchronously (via process_event) when the background loop is not running (_running=False), otherwise queue them.
• Update test fixtures in test_cache_integration.py and test_cache_performance.py to use correct cache key prefixes, fix BenchmarkTimer to raise errors on empty data rather than returning 0, and relax fragile performance assertions.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
api/cache/layer.py	Adds optional ttl_seconds param to set(); propagates TTL to both L1 and L2
api/cache/invalidation.py	emit_event now calls process_event directly when the background loop is not running
tests/test_cache_integration.py	Corrects cache key prefixes from project: to projects: in two tests
tests/test_cache_performance.py	BenchmarkTimer raises on empty data; performance assertions relaxed; adds get() alias to CosmosDBSimulator
DEPLOYMENT-PR-READY.md	New file documenting deployment readiness and PR merge instructions
.github/PRIORITY4-AUTOMATED-REMEDIATION-PLAN.md	New file with full DPDCA plan for future L48-L51 automated remediation layers
.github/CACHE-LAYER-FIXES-PLAN.md	New file documenting the root-cause analysis and fix plan for cache layer test failures

[Copilot]

This file contains multiple non-ASCII characters that violate the repository's ASCII-only encoding rule (enforced per .github/copilot-instructions.md section 4). Specifically: emoji characters such as rocket (line 1), green checkmark (many lines), green circle (lines 213, 233), target (line 237), timer (line 238), and bar chart (line 239) are all forbidden Unicode codepoints. All such characters must be replaced with ASCII equivalents: use [PASS] or [OK] instead of checkmark emoji, and plain ASCII text instead of other emoji.

[Copilot]

This file contains multiple non-ASCII characters that violate the repository's ASCII-only encoding rule (enforced per .github/copilot-instructions.md section 4). Specifically: Unicode right arrows (→) on lines 49, 67, 260, 397; Unicode downward arrows (↓) on lines 405-415; checkmark (✅) on lines 423, 430; and cross (✗) on many lines in the Discover section. All must be replaced with ASCII equivalents: use -> for arrows, [PASS] for checkmarks, and [FAIL] or plain text for crosses.

[Copilot]

This file uses emoji checkmarks (✅) on lines 186-202, which violates the repository's ASCII-only encoding rule (enforced per .github/copilot-instructions.md section 4). Replace all ✅ occurrences with [PASS] or [OK].

[Copilot]

The sample JSON schema for the proposed L48 layer uses "policy_id" as the record identifier field (e.g., "policy_id": "policy:agent-performance-recovery") instead of the required "id" field. Per the codebase convention (see api/routers/admin.py:149-154), the seed function filters records by o.get("id") and silently drops any record that lacks an "id" field. If this schema is implemented as-is, all remediation policy records would be silently excluded from the data model when seeded. The schema should use "id" as the primary key field name, with the policy_id value moved to that field or kept as an alias.

[Copilot]

The guard if cache_avg > 0 and cosmos_avg > 0 makes the primary performance assertion (that the cache is at least 10x faster than Cosmos) effectively optional. On fast systems where in-memory operations complete in sub-millisecond time, cache_avg will round to 0 ms, and the assertion is skipped entirely. This means the test no longer validates the intended performance claim in most environments. Since cosmos_avg is already asserted to be > 0 on line 146, and the Cosmos simulator sleeps for 50ms per query, cosmos_avg will always be well above 0. Consider restructuring the assertion to always verify the performance ratio when cosmos_avg > 0, or use a more appropriate comparison metric.

Suggested change

	# Allow for timing resolution (both could be 0 on fast systems)
	if cache_avg > 0 and cosmos_avg > 0:
	assert cache_avg < cosmos_avg / 10 # At least 10x faster
	# At minimum, cosmos should have latency from await asyncio.sleep
	assert cosmos_avg > 0
	if cosmos_avg > 0:
	assert cache_avg < cosmos_avg / 10 # At least 10x faster
	else:
	pytest.skip("Cosmos average latency is zero; cannot evaluate performance ratio")

[Copilot]

The assertion assert warm_avg >= 0 and read_avg >= 0 is logically redundant: the preceding assertions assert warm_avg < 1.0 and assert read_avg < 1.0 would already fail before this line if the values were negative (since a negative value is less than 1.0 and would pass those checks, but in practice time.time() differences are always non-negative). More concretely, if either warm_avg or read_avg were negative, they would satisfy < 1.0 and this redundant check would catch nothing new. This assert can be safely removed.

Suggested change

	# Both operations should succeed
	assert warm_avg >= 0 and read_avg >= 0

[Copilot]

The or operator at these two lines treats ttl_seconds=0 as falsy and falls back to the default TTL values instead. While a TTL of 0 is unusual, this is semantically incorrect: ttl_seconds or self.ttl_l1 should be ttl_seconds if ttl_seconds is not None else self.ttl_l1 (and similarly for ttl_l2) to correctly distinguish between "caller passed 0" and "caller did not pass a value". Using or means a caller who intentionally passes ttl_seconds=0 (e.g., to disable caching or use an immediate-expiry entry) will silently get the default TTL instead.

Suggested change

	ttl_l1 = ttl_seconds or self.ttl_l1
	ttl_l2 = ttl_seconds or self.ttl_l2
	ttl_l1 = ttl_seconds if ttl_seconds is not None else self.ttl_l1
	ttl_l2 = ttl_seconds if ttl_seconds is not None else self.ttl_l2

[Copilot]

This change introduces a race condition with the background event loop. When emit_event is called immediately after asyncio.create_task(invalidation.start()) (as done in test_cache_invalidation_events), the task has not yet had a chance to execute, so _running is still False. As a result, process_event is called directly (synchronously in the current coroutine), bypassing the queue entirely. This means the queued-event path is never actually tested by test_cache_invalidation_events.

More importantly, in production usage where emit_event is called before start() has been called (or after stop() has been called but before a restart), events will now be processed synchronously and inline rather than being dropped or raising an error. This silent behavioral change could cause performance issues if many events are emitted outside the running context, since each call to emit_event blocks until the full cache invalidation completes inline.

Consider using asyncio.ensure_future or explicitly documenting that direct processing is the intended behavior when _running=False, and add a test that verifies the queue path is used when _running=True.

Suggested change

	# For test environments or direct processing, execute immediately
	if not self._running:
	await self.process_event(event)
	# For test environments or direct processing, execute immediately in background
	if not self._running:
	asyncio.ensure_future(self.process_event(event))