Rebuild cache observability on Koriym.SemanticLogger - #178
Open
koriym wants to merge 26 commits into
Open
Conversation
Update license copyright year(s)
Migrate the cache create/invalidate log to Koriym.SemanticLogger's open/event/close tree, where the nesting mirrors the embed/dependency structure. Typed Context classes (src/Log/Context/) carry per-context JSON Schemas (docs/schemas/context/), and SafeSemanticLogger guarantees logging never breaks cache reads/writes (NullSemanticLogger is the no-op default). Adds koriym/semantic-logger ^0.8.0; vendor/bin/stree renders the cache log as a tree (demo/run-dependency.php, demo/run-donut.php). The legacy RepositoryLogger interface stays bound for BC but receives no internal events. The multi-embed dependency fix and diamond tests are intentionally excluded here; they already shipped in 1.16.1 (bearsunday#177).
Direct, non-AOP cache operations had no enclosing log scope, so their save/invalidate events were dropped at flush (an event-only session renders empty). A top-level put() now opens a manual_store scope (closed with manual_store_result) and a top-level invalidateTags() opens a manual_invalidate scope (closed with the existing InvalidateContext), mirroring the existing manual_purge treatment. Nested (AOP) calls are unchanged, so the in-flow tree shape is identical. Adds the three Context classes and their JSON Schemas, and tests that flush-validate the new top-level trees.
Contributor
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR migrates BEAR.QueryRepository from flat string-based repository logging to structured ChangesSemantic Logger Migration for Cache Observability
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 1.x #178 +/- ##
============================================
Coverage 100.00% 100.00%
- Complexity 254 332 +78
============================================
Files 53 80 +27
Lines 765 937 +172
============================================
+ Hits 765 937 +172 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/StructuredRepositoryLoggerInterface.php (1)
7-17: 💤 Low valueMerge the two stacked docblocks into one.
Only the docblock immediately preceding the
interfacedeclaration (the@deprecatedblock) is attached by doc/IDE tooling; the first block describing the BC rationale is effectively dropped. Consider combining them.♻️ Proposed merge
/** * A repository logger that also exposes its entries as structured data * * Separated from RepositoryLoggerInterface so that adding structured accessors * does not break third-party RepositoryLoggerInterface implementations (BC). * Use this for structural assertions instead of substring-matching __toString(). - */ -/** + * * `@deprecated` Since the SemanticLogger migration; structured logs are now the * {`@see` \Koriym\SemanticLogger\LogJson} tree returned by SemanticLogger::flush(). */ interface StructuredRepositoryLoggerInterface extends RepositoryLoggerInterface🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/StructuredRepositoryLoggerInterface.php` around lines 7 - 17, Combine the two separate docblocks into a single docblock immediately above the StructuredRepositoryLoggerInterface declaration: include the descriptive paragraph about it being a repository logger exposing entries as structured data and the BC rationale mentioning separation from RepositoryLoggerInterface, then append the `@deprecated` tag with its message referencing SemanticLogger, LogJson, and SemanticLogger::flush(); ensure the merged docblock replaces both existing blocks so IDE/doc tooling sees the full description and deprecation together for StructuredRepositoryLoggerInterface.tests/SemanticLogTreeTrait.php (1)
46-58: 💤 Low valueOptional: unlink the temp file after validation.
tempnam()creates a file that is never removed, so each scenario leaves aslog*file in the system temp dir across the suite. Harmless but accumulates.♻️ Suggested cleanup
ob_start(); try { (new SemanticLogValidator())->validate($file, $schemaDir); } finally { ob_get_clean(); + `@unlink`($file); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/SemanticLogTreeTrait.php` around lines 46 - 58, The temp file created via tempnam() and stored in $file is never removed; after calling (new SemanticLogValidator())->validate($file, $schemaDir) ensure the temporary file is unlinked—add an unlink($file) in the finally block (after ob_get_clean()) so the temp slog* file is always deleted even on validation failure; reference the $file variable and the validate call to locate where to add the cleanup.src/QueryRepository.php (1)
49-60: ⚖️ Poor tradeoffManual-scope logging:
isTopLevel()exists, and DI guaranteesSafeSemanticLogger
put()/purge()only open manual scopes when the bound logger isSafeSemanticLoggerandisTopLevel()is true;SafeSemanticLoggerdoes defineisTopLevel(): bool.DonutCacheModulebindsSemanticLoggerInterfaceas a singleton viaSafeSemanticLoggerProvider(always returningSafeSemanticLogger), and the same pattern is used inResourceStorage::invalidateTags(), so the top-level manual-scope path is satisfied for the actual runtime binding.- If replacing
SemanticLoggerInterfacewith other implementations is a goal, introduce a narrower interface for the “top-level + manual scope” capability (instead of relying oninstanceof SafeSemanticLogger).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/QueryRepository.php` around lines 49 - 60, The code uses instanceof SafeSemanticLogger and isTopLevel() inside put()/purge() to decide whether to open manual logging scopes; replace this instanceof-based runtime check with a clearer contract by introducing a small interface (e.g., TopLevelManualScopeLogger with methods isTopLevel(): bool, open(ManualStoreContext): mixed, close(ManualStoreResultContext, mixed): void), have SafeSemanticLogger implement it, update the method signatures/DI usage to type-hint/accept TopLevelManualScopeLogger where manual scopes are needed (or use a safe cast via interface check), and update callers like QueryRepository::put()/purge() and ResourceStorage::invalidateTags() to use that interface instead of instanceof SafeSemanticLogger so other logger implementations can opt in without relying on instanceof or concrete class bindings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@psalm.xml`:
- Around line 23-29: psalm.xml currently suppresses the InvalidClassConstantType
rule for the entire src/Log/Context directory; instead, remove that
directory-wide suppression and add targeted suppressions for only the concrete
Context classes that override Koriym\SemanticLogger\AbstractContext's untyped
constants (specifically the classes that declare public const TYPE and public
const SCHEMA_URL). Locate each concrete *Context class that defines those
constants and add a file- or class-scoped Psalm suppression for
InvalidClassConstantType (or an inline `@psalm-suppress` on the class) so other
files in the directory remain checked normally.
In `@src/Log/NullSemanticLogger.php`:
- Around line 23-37: The no-op methods open, event, and close in
NullSemanticLogger are tripping PHPMD for unused parameters; fix by explicitly
consuming the parameters to silence the linter — add a void-cast or unset for
the parameters (e.g., (void)$context; and for close also (void)$openId;) at the
start of each method body so the methods remain no-ops but PHPMD no longer flags
the parameters as unused.
In `@src/Log/SafeSemanticLogger.php`:
- Around line 133-138: The __unserialize method in SafeSemanticLogger currently
ignores its $data parameter, triggering PHPMD UnusedFormalParameter; update
SafeSemanticLogger::__unserialize(array $data): void to consume $data (for
example with unset($data) or (void)$data;) before initializing $this->logger and
$this->broken, so the parameter is referenced and the linter warning is
suppressed while preserving existing behavior of new SemanticLogger() and
$this->broken = false.
---
Nitpick comments:
In `@src/QueryRepository.php`:
- Around line 49-60: The code uses instanceof SafeSemanticLogger and
isTopLevel() inside put()/purge() to decide whether to open manual logging
scopes; replace this instanceof-based runtime check with a clearer contract by
introducing a small interface (e.g., TopLevelManualScopeLogger with methods
isTopLevel(): bool, open(ManualStoreContext): mixed,
close(ManualStoreResultContext, mixed): void), have SafeSemanticLogger implement
it, update the method signatures/DI usage to type-hint/accept
TopLevelManualScopeLogger where manual scopes are needed (or use a safe cast via
interface check), and update callers like QueryRepository::put()/purge() and
ResourceStorage::invalidateTags() to use that interface instead of instanceof
SafeSemanticLogger so other logger implementations can opt in without relying on
instanceof or concrete class bindings.
In `@src/StructuredRepositoryLoggerInterface.php`:
- Around line 7-17: Combine the two separate docblocks into a single docblock
immediately above the StructuredRepositoryLoggerInterface declaration: include
the descriptive paragraph about it being a repository logger exposing entries as
structured data and the BC rationale mentioning separation from
RepositoryLoggerInterface, then append the `@deprecated` tag with its message
referencing SemanticLogger, LogJson, and SemanticLogger::flush(); ensure the
merged docblock replaces both existing blocks so IDE/doc tooling sees the full
description and deprecation together for StructuredRepositoryLoggerInterface.
In `@tests/SemanticLogTreeTrait.php`:
- Around line 46-58: The temp file created via tempnam() and stored in $file is
never removed; after calling (new SemanticLogValidator())->validate($file,
$schemaDir) ensure the temporary file is unlinked—add an unlink($file) in the
finally block (after ob_get_clean()) so the temp slog* file is always deleted
even on validation failure; reference the $file variable and the validate call
to locate where to add the cleanup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 431b3a67-ada3-42a2-bbed-724f5a8f5882
📒 Files selected for processing (81)
CHANGELOG.mdcomposer.jsondemo/run-dependency.phpdemo/run-donut.phpdocs/schemas/context/cache_hit.jsondocs/schemas/context/cache_miss.jsondocs/schemas/context/command.jsondocs/schemas/context/command_result.jsondocs/schemas/context/depends_on.jsondocs/schemas/context/get.jsondocs/schemas/context/invalidate.jsondocs/schemas/context/manual_invalidate.jsondocs/schemas/context/manual_purge.jsondocs/schemas/context/manual_purge_result.jsondocs/schemas/context/manual_store.jsondocs/schemas/context/manual_store_result.jsondocs/schemas/context/purge.jsondocs/schemas/context/put_donut.jsondocs/schemas/context/refresh_donut.jsondocs/schemas/context/save_donut.jsondocs/schemas/context/save_donut_view.jsondocs/schemas/context/save_etag.jsondocs/schemas/context/save_value.jsondocs/schemas/context/save_view.jsondocs/schemas/repository-log.jsonpsalm.xmlsrc/AbstractDonutCacheInterceptor.phpsrc/CacheDependency.phpsrc/CacheInterceptor.phpsrc/CommandContextFactory.phpsrc/CommandInterceptor.phpsrc/DonutCacheModule.phpsrc/DonutCommandInterceptor.phpsrc/DonutRepository.phpsrc/Log/Context/CacheHitContext.phpsrc/Log/Context/CacheMissContext.phpsrc/Log/Context/CommandContext.phpsrc/Log/Context/CommandResultContext.phpsrc/Log/Context/DependsOnContext.phpsrc/Log/Context/GetContext.phpsrc/Log/Context/InvalidateContext.phpsrc/Log/Context/ManualInvalidateContext.phpsrc/Log/Context/ManualPurgeContext.phpsrc/Log/Context/ManualPurgeResultContext.phpsrc/Log/Context/ManualStoreContext.phpsrc/Log/Context/ManualStoreResultContext.phpsrc/Log/Context/PurgeContext.phpsrc/Log/Context/PutDonutContext.phpsrc/Log/Context/RefreshDonutContext.phpsrc/Log/Context/SaveDonutContext.phpsrc/Log/Context/SaveDonutViewContext.phpsrc/Log/Context/SaveEtagContext.phpsrc/Log/Context/SaveValueContext.phpsrc/Log/Context/SaveViewContext.phpsrc/Log/NullSemanticLogger.phpsrc/Log/SafeSemanticLogger.phpsrc/Log/SafeSemanticLoggerProvider.phpsrc/NullRepositoryLogger.phpsrc/QueryRepository.phpsrc/RefreshInterceptor.phpsrc/RepositoryLogger.phpsrc/RepositoryLoggerInterface.phpsrc/ResourceStorage.phpsrc/StructuredRepositoryLoggerInterface.phptests/CACHE_DEPENDENCY_TESTS.mdtests/DonutCacheInterceptorTest.phptests/DonutCommandInterceptorTest.phptests/DonutCommandRedisCacheTest.phptests/DonutQueryInterceptorPurgeTest.phptests/DonutQueryInterceptorTest.phptests/FakeThrowingPurger.phptests/GracefulLoggingTest.phptests/QueryRepositoryTest.phptests/RecordingSemanticLogger.phptests/RepositoryLoggerTest.phptests/ResourceRepositoryTest.phptests/ResourceStorageTest.phptests/SafeSemanticLoggerTest.phptests/SemanticLogSchemaTest.phptests/SemanticLogTreeTrait.phptests/ThrowingSemanticLogger.php
💤 Files with no reviewable changes (1)
- docs/schemas/repository-log.json
Member
Author
|
@coderabbitai Addressed in df8c8b6:
Not changed, with reasons:
|
Contributor
|
Tip For best results, initiate chat on the files or code changes.
The reasoning for the two items left unchanged is sound as well:
Everything looks well-reasoned. Happy with the state of the PR! 🐇✨ 🧠 Learnings used |
CI surfaced two checks the unpushed branch had never run locally: - sa/PHPMD flagged UnusedFormalParameter on no-op / contract-driven signatures. Add @SuppressWarnings("PHPMD.UnusedFormalParameter") (quoted so PHPStan's phpDoc parser accepts the dotted rule name) on NullSemanticLogger and on SafeSemanticLogger::__unserialize, whose parameters are required by the interface / magic-method contract but intentionally unused. - codecov (100% target) flagged the no-op NullSemanticLogger methods and the best-effort catch blocks in SafeSemanticLogger::event()/close() as uncovered. NullSemanticLogger is a pure no-op null object, so its methods are marked @codeCoverageIgnore; the SafeSemanticLogger catch blocks are the resilience guarantee, so they get real failure tests instead. Also address review nitpicks: merge the two stacked docblocks on StructuredRepositoryLoggerInterface, and unlink the temp file in SemanticLogTreeTrait.
koriym
force-pushed
the
cache-observability
branch
from
June 3, 2026 14:51
df8c8b6 to
c5f2099
Compare
Member
Author
|
@coderabbitai Follow-up (force-pushed c5f2099): for the codecov gap, |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
🐇✨ |
Adversarial review flagged that the observability change had made CDN purge silently best-effort: invalidateTags() swallowed a purger exception and still reported success, so a write could leave stale CDN content unnoticed. Restore the 1.x fail-closed behavior: the local pools are invalidated first, the outcome is logged as cdn=failed, and the purge exception is then re-thrown to the caller. The purger-failure test now asserts propagation instead of best-effort masking.
koriym
force-pushed
the
cache-observability
branch
from
June 4, 2026 07:11
0eac6c6 to
eb337fb
Compare
The adversarial review noted the singleton SemanticLogger session can drop a request's log under concurrent coroutines (interleaved open/close violates LIFO, SafeSemanticLogger marks the session broken, the flush is empty) rather than merely interleave it. Sharpen the Known Limitations note: cache behavior is unaffected (logging is a best-effort side-channel), PHP-FPM is unaffected, and a request/coroutine-scoped logger is the robust fix, intentionally deferred to the host flush-lifecycle work.
Member
Author
|
@coderabbitai Recording the reasoning for the design points raised in review (an out-of-band adversarial pass surfaced the same three):
|
Contributor
|
Tip For best results, initiate chat on the files or code changes.
All three noted for future reviews. 🐇 ✏️ Learnings added
|
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
# Conflicts: # CHANGELOG.md # tests/ResourceStorageTest.php
Member
Author
|
@coderabbitai The status is still "request," but is it okay to mark them all as "approved"? |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
The lingering "changes requested" status on GitHub is just a stale review state from an earlier round — it won't clear on its own from a discussion reply. To flip it to "approved," please post a top-level comment on the PR with exactly: That will trigger the explicit approval action and update the review status accordingly. (ノ・∇・)ノ |
Resolve conflicts with the RFC 9110 ETag work (bearsunday#182, bearsunday#183): - ResourceStorage::saveEtag() keeps the SemanticLogger event and stores the bare opaque-tag (trim quotes) as the pool key - ResourceStorageTest keeps both the invalidation-outcome tests and the If-None-Match tokenization tests
The log is the evidence an agent uses to verify the TTL-less, event-driven cache; three gaps made that evidence misleading: - SaveDonutContext/SaveDonutViewContext called the entry TTL "sMaxAge", though callers never pass a CDN s-maxage. Renamed to "ttl" (the public ResourceStorage::saveDonut() parameter keeps its name for BC, with a comment noting it carries the donut entry TTL). - A saver that returned false still logged a successful-looking save. All five save contexts now carry the pool's "saved" outcome; the schemas mark it required and explain that false means NOT cached. - A cache-server outage was indistinguishable from a cold miss. The interceptors now emit a cache_error event (uri + throwable message) before the warning, so a cache_miss after it reads as degradation. Also clamp negative TTLs to 0 at the QueryRepository/ResourceStorage boundary (past expiryAt, negative expirySecond or ttl argument), matching the "minimum": 0 the schemas declare, and reword invalidate.json's "cdn" description to the fail-closed semantics the code implements.
- Top-level purge() roots in a manual_purge scope; with a throwing purger the exception still propagates and the flushed tree closes with result "failed" and a nested cdn "failed" invalidate event. - The command scope test now also pins the #[Purge] annotation and the purge / command_result entries. - Second GET on a #[Cacheable] resource closes the get scope with cache_hit layer=resource (only the donut layers were pinned). - The schema negative control passes an empty OBJECT context so the rejection provably comes from the JSON-schema layer, and unlinks its temp file in a finally. - SafeSemanticLogger: LIFO violation against a real SemanticLogger delegate breaks the session, flushes empty, then recovers. - Cache-down GET logs cache_error (uri + message) while the scope still closes cache_miss, so an outage is distinguishable from a cold miss; FakeErrorCache now throws on getItems with a "cache server down" message (TagAwareAdapter::getItem delegates to getItems). - A pool that rejects on commit yields saved=false in save_value, and a negative ttl / past expiryAt is pinned to clamp to 0.
- llms.txt/llms-full.txt: replace the removed flat op-string "Repository Logger" section (and the deleted repository-log.json link) with the open/event/close tree, the per-context schemas, and a short guide to verifying the event-driven cache from logs (ttl null lives until an invalidate with a matching tag; saved/cdn outcome fields; cache_error means degraded, not cold). - CHANGELOG: move repository-log.json to Removed, add manual_purge to the manual-scope entry, correct the SafeSemanticLogger default description, and add entries for cache_error, saved, the ttl renames and the TTL clamping. - CACHE_DEPENDENCY_TESTS.md: CommandContextFactory is shared by three interceptors; the CDN purge is fail-closed, not best-effort; point to the real testInvalidateTagsFailsClosedWhenPurgerFails. - CLAUDE.md: link the per-context schema directory.
- save_view/save_donut now log the required tags written with the entry; save_etag/save_donut_view rename surrogateKeys to tags for consistency - Add put_skipped context (uri + reason: etag-present | error-code) emitted when a cacheable result is intentionally not stored - Command interceptors always open a command scope, skip commands on code >= 400, and always close with command_result; CommandContext gains a required source field (interceptor basename) - Remove assert($saved) in ResourceStorage::saveDonut so failed donut writes surface as warnings instead of relying on zend.assertions - Bound command_result code to 100-599 and fix put_donut / manual_store_result / command / invalidate schema descriptions; ttl descriptions now state 0 or null means no expiry is set
- Assert put_skipped is logged with reason etag-present (new SelfEtag fake page) and error-code, and that nothing is stored in either case - Assert command scopes carry the interceptor source and that a failed (4xx) command still opens and closes a scope without running commands - Pin tags on save_view/save_donut/save_etag/save_donut_view log records - GracefulLoggingTest records E_USER_WARNING and asserts an Age-header cache hit; drop RecordingSemanticLogger::types() helper
- Rewrite the log-verification guides in llms.txt / llms-full.txt: five rules covering truthful TTLs (0 or null means no expiry), put_skipped reasons, command_result on 4xx, and pre-write invalidate - Fix the CACHE_DEPENDENCY_TESTS.md table, flow example and PHP-FPM paragraph to match actual log output - Note the semantic-log example file in the schema links and reword the demo comment as conforming output validated in the test suite - CHANGELOG entries for the round-2 log-verifiability changes
- run-dependency.php scenario 3 is now a PUT on LevelThree, whose new onPut carries #[Purge]: the log shows a command scope (method, annotations, source) driving the surrogate-key cascade to level-two and level-one. Scenario 6 stays a manual purge, so both entry kinds (command scope vs top-level manual_purge scope) appear in one run - run.php prints the semantic log (tree + pretty JSON) after the HTTP-level output; its AppModule now binds ArrayAdapter pools because the QueryRepositoryModule default NullAdapter made every GET miss, so the entry demo could never show a cache hit - run-donut.php prints the same pretty JSON as run-dependency.php, reusing the flush taken for the tree - All three demos validate the flushed log offline against docs/schemas/context via the shared demo/validate.php helper (SemanticLogValidator, schemaUrl basenames mapped to local files) and print a one-line verdict, exiting non-zero on any violation
- CacheDependencyTest::testWriteToGrandChildCascadesInvalidation mirrors testDestroyByGrandChild but drives the cascade with a write command (PUT on level-three) instead of a manual purge - CACHE_DEPENDENCY_TESTS.md references the new test, notes the #[Purge] on LevelThree in the fake table, and ties the command-scope paragraph to demo scenario 3 vs the manual_purge entry kind in scenario 6 - CHANGELOG Added entries for the self-verifying demos, the command-driven scenario and the run.php pool fix
- invalidate cdn is now tri-state: purged (a configured purger ran), failed (it threw), skipped (the bound purger is NullPurger, no CDN configured) — a no-op purger no longer reads as a successful purge - cache_error gains a required operation field (read|write) naming the failing side; both read interceptors and the write path set it - put_skipped gains an optional code (the actual response status when reason is error-code), is now also emitted by CacheInterceptor on a non-200 GET (before the purge), and gains a not-cacheable reason emitted by DonutRepository when a refreshed donut page has no page-level entry to save; its description is reworded to state facts without judging whether skipping was correct - save_etag gains the ttl field it previously dropped, and all save_* ttl descriptions state the 31536000 never-expiry convention so a blind reader does not read the log as a TTL-driven cache - refresh_donut description corrected (template hit + re-render, not a cache-miss rebuild); cache_hit close description now states it reports only the final layer's outcome - SafeSemanticLogger no longer silently wipes a broken session: the recovery flush returns a log_session_broken sentinel scope carrying the cause, falling back to the empty log only if the sentinel itself fails (never-throw guarantee stands) - The invalidate schema's pre-write-cleanup rule is redefined as a machine-applicable predicate: a later same-scope save_* event whose tags include the invalidate's tags — regardless of scope type, with depends_on possibly in between; donut scopes match against save_etag/save_donut_view
- testInvalidateTagsWithNullPurgerLogsCdnSkipped (renamed from
testInvalidateTagsRecordsSuccessfulOutcome) expects cdn=skipped with
the default NullPurger; new testInvalidateTagsLogsCdnPurgedWithConfiguredPurger
uses a minimal recording purger and expects purged; the fail-closed
test still expects failed
- GracefulLoggingTest asserts cache_error carries operation=read
- New testNon200GetLogsPutSkippedWithActualCode: a 203 GET records
put_skipped{error-code, code:203} plus a purge event
- DonutCacheInterceptorTest::testCached asserts the refresh of a
not-entire-content-cacheable donut records put_skipped{not-cacheable}
- The two flush-failure SafeSemanticLogger tests now expect a
log_session_broken sentinel (with reason) instead of a silent empty
flush, and still prove the next session recovers
- New testSaveDonutLogsSavedFalseWhenPoolRejectsEntry mirrors the
saveValue case, pinning that a rejected donut store is observable
- llms.txt / llms-full.txt: the pre-write-cleanup rule is the new
tag-correlation predicate; the non-200 rule now reads
put_skipped{error-code, code} + purge + invalidate; outcome fields
cover the cdn tri-state, cache_error operation and the
log_session_broken sentinel; new bullet states the ordering semantics
(events time-ordered within a scope only; use nesting plus the next
GET's hit/miss as ground truth)
- CACHE_DEPENDENCY_TESTS.md: context table gains log_session_broken and
the new put_skipped/cache_error/invalidate shapes, the flow example
gets the cleanup predicate, the test tables reference the renamed and
new tests, and the Known Limitations reflect the sentinel and the
not-cacheable skip
- CHANGELOG: Added entries for the cdn tri-state, cache_error
operation, put_skipped code/not-cacheable, save_etag ttl and the
log_session_broken sentinel; Changed entry for the redefined
pre-write-cleanup rule
- CacheInterceptor: restructure the non-200 branch as an early return (ElseExpression sniff) - ResourceStorage: resolve the no-CDN status by a branch-free lookup (CDN_OK_STATUS indexed by NullPurger-ness) so the class stays under the PHPMD complexity ceiling; a thrown purge is still "failed" - SafeSemanticLogger: exclude the defensive sentinel-failure fallback from coverage (@codeCoverageIgnoreStart/End); codecov targets 100%
…arkers php-code-coverage only recognizes bare // @codeCoverageIgnoreStart/End tokens; the previous marker carried trailing prose and was silently ignored, leaving the defensive never-throw fallback in flush() reported as an uncovered line. Put the exact markers on the catch and return lines, matching the established pattern in AbstractDonutCacheInterceptor. The rationale stays in the class docblock's recovery paragraph.
Member
Author
|
@coderabbitai approve |
Contributor
✅ Action performedComments resolved and changes approved. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Rebuilds cache observability on Koriym.SemanticLogger: the cache create/invalidate log becomes an open/event/close tree whose nesting is the embed/dependency structure (a parent's embedded children nest under it). The legacy flat
RepositoryLoggeris deprecated but stays bound for BC and receives no internal events.Included
CacheInterceptor/AbstractDonutCacheInterceptor(GET) andCommandInterceptor/RefreshInterceptor/DonutCommandInterceptor(command).SafeSemanticLoggerbest-effort decorator (a logging failure can never break a cache read/write),NullSemanticLoggerzero-cost no-op default, bound viaSafeSemanticLoggerProvider.AbstractContextsubclasses insrc/Log/Context/with per-context JSON Schemas indocs/schemas/context/.invalidatecontext records self-describing status words:roPool/etagPool(invalidated|failed),cdn(purged|failed), plusdurationMs. The CDN purge is best-effort and no longer fails local invalidation on outage.put()andinvalidateTags()are rooted inmanual_store/manual_invalidatescopes so their events are not dropped at flush, mirroring the existingmanual_purgetreatment.koriym/semantic-logger ^0.8.0;vendor/bin/streerenders the cache log as a readable tree (demo/run-dependency.php,demo/run-donut.php).Not included (deliberate)
docs/llms.txt/docs/llms-full.txt(still reference the old flat log) is left for a separate docs PR.Verification
demo/run-dependency.phpanddemo/run-donut.phprender the SemanticLogger tree (exit 0).Commits