Improve machine-readable logging and add LLM documentation #170
Conversation
- Add array parameter tests to RepositoryLoggerTest - Add multiple parents dependency test to CacheDependencyTest - Add unrelated resources independence test with ETag verification - Add CACHE_DEPENDENCY_TESTS.md documentation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis PR modernizes the logging system in BEAR.QueryRepository by replacing printf-style formatted logging with structured context-based logging. The interface signature changes from variadic template parameters to an operation name and context array, with all call sites updated accordingly. Supporting demos, test resources, and documentation are added. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
BREAKING CHANGE: RepositoryLoggerInterface signature changed
from log(string $template, ...$values) to log(string $operation, array $context = [])
- Output now uses JSON format: {"op":"save-value","uri":"...","tags":[...]}
- Arrays are properly serialized as JSON arrays
- Easier to parse and analyze programmatically
- Update callers in ResourceStorage, QueryRepository, DonutRepository
- Update tests to use new format
Define schema for all repository log operations: - save-value, save-view, save-etag, save-donut, save-donut-view - invalidate-etag, put-query-repository, purge-query-repository - put-donut, try-donut-view, found-donut-view - try-donut, no-donut-found, refresh-donut
- llms.txt: Concise project overview - llms-full.txt: Comprehensive documentation with all attributes, modules, and architecture - Links to JSON schema on GitHub Pages
koriym
changed the title
koriym
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 1.x #170 +/- ##
===========================================
Coverage 100.00% 100.00%
+ Complexity 245 241 -4
===========================================
Files 52 52
Lines 732 729 -3
===========================================
- Hits 732 729 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
koriym
force-pushed
the
test-enhancement-cache-dependency
branch
from
df1e0e3 to
f5fa2ba
Compare
Demonstrates cache behavior with meaningful log output: - 3-level dependency chain (LevelOne -> LevelTwo -> LevelThree) - Multiple parents depending on same child (ParentA/ParentB -> ChildC) - Cache hit/miss, depends-on, and cascade invalidation logging
Each request now starts with a request-start log entry, making it easy for AI to identify request boundaries when analyzing the cache operation logs.
Demonstrates donut caching (static outer + dynamic inner): - try-donut-view, found-donut-view operations - put-donut, save-donut, save-donut-view operations - refresh-donut when inner content is invalidated - Etag 'r' suffix indicates refresh vs full regeneration
- Add reference to BEAR.Sunday llms-full.txt for conceptual docs - Add request-start and donut cache operations to log schema - Add method property for request-start operation - Update CLAUDE.md with documentation links
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tests/CACHE_DEPENDENCY_TESTS.md`:
- Around line 79-80: The table entries reference a misspelled test class name
"UtiTagTest" instead of "UriTagTest"; update both occurrences in the document so
the test references match the actual test class. Locate the rows containing
`UriTag::__invoke()` / `UtiTagTest::testInvoke` and `UriTag::fromAssoc()` /
`UtiTagTest::testFromAssoc` and replace `UtiTagTest` with `UriTagTest` so the
`UriTag::__invoke()` and `UriTag::fromAssoc()` tests are correctly named.
🧹 Nitpick comments (5)
docs/schemas/repository-log.json (1)
12-31: Some operations lack conditional validation rules.The
openum includes operations likeput-query-repository,purge-query-repository,try-donut-view,found-donut-view,try-donut,no-donut-found, andrefresh-donut, but these don't have corresponding conditional rules in theallOfsection to enforce their required fields.Consider adding conditional rules for these operations for schema completeness, or document that they have no required fields beyond
op.CLAUDE.md (2)
9-11: Consider using proper Markdown link syntax for URLs.The bare URLs work but could be formatted as Markdown links for consistency with linting rules and better readability.
♻️ Suggested formatting
For conceptual documentation, see: -- BEAR.Sunday: https://bearsunday.github.io/llms-full.txt +- BEAR.Sunday: [llms-full.txt](https://bearsunday.github.io/llms-full.txt) - This package: docs/llms-full.txt
205-209: Same suggestion for Documentation section URLs.♻️ Suggested formatting
## Documentation -- BEAR.Sunday LLM docs: https://bearsunday.github.io/llms-full.txt -- Cache manual: https://bearsunday.github.io/manuals/1.0/en/cache.html +- BEAR.Sunday LLM docs: [llms-full.txt](https://bearsunday.github.io/llms-full.txt) +- Cache manual: [cache.html](https://bearsunday.github.io/manuals/1.0/en/cache.html) - Log schema: docs/schemas/repository-log.jsonsrc/RepositoryLogger.php (1)
34-37: Consider handlingjson_encodefailure explicitly.
json_encode()returnsfalseon failure (e.g., invalid UTF-8 sequences in context values). The(string)cast converts this to an empty string, which could silently produce incomplete logs. Consider usingJSON_THROW_ON_ERRORfor stricter error handling, or at minimumJSON_INVALID_UTF8_SUBSTITUTEto handle malformed strings gracefully.♻️ Suggested improvement
return implode(PHP_EOL, array_map( - static fn (array $log): string => (string) json_encode($log, JSON_UNESCAPED_SLASHES), + static fn (array $log): string => (string) json_encode($log, JSON_UNESCAPED_SLASHES | JSON_INVALID_UTF8_SUBSTITUTE), $this->logs, ));Or for stricter handling:
static fn (array $log): string => json_encode($log, JSON_UNESCAPED_SLASHES | JSON_THROW_ON_ERROR),tests/Fake/fake-app/src/Resource/Page/Dep/ChildC.php (1)
1-17: Consider addingdeclare(strict_types=1)and type declarations for consistency.The fake resource is functionally correct and follows the same pattern as
ParentAandParentB. For consistency with the main codebase, consider adding strict types and type annotations.♻️ Optional improvement
<?php + +declare(strict_types=1); namespace FakeVendor\HelloWorld\Resource\Page\Dep; use BEAR\RepositoryModule\Annotation\Cacheable; use BEAR\Resource\ResourceObject; #[Cacheable] class ChildC extends ResourceObject { - public $body = ['child-c' => 1]; + /** `@var` array{child-c: int} */ + public array $body = ['child-c' => 1]; - public function onGet() + public function onGet(): static { return $this; } }
tests/CACHE_DEPENDENCY_TESTS.md
Outdated
| | `UriTag::__invoke()` | `UtiTagTest::testInvoke` | URI to tag string conversion | | ||
| | `UriTag::fromAssoc()` | `UtiTagTest::testFromAssoc` | Generate tags from array data | |
There was a problem hiding this comment.
Typo in test class name: UtiTagTest → UriTagTest
The test class name appears to be misspelled in two places.
📝 Proposed fix
-| `UriTag::__invoke()` | `UtiTagTest::testInvoke` | URI to tag string conversion |
-| `UriTag::fromAssoc()` | `UtiTagTest::testFromAssoc` | Generate tags from array data |
+| `UriTag::__invoke()` | `UriTagTest::testInvoke` | URI to tag string conversion |
+| `UriTag::fromAssoc()` | `UriTagTest::testFromAssoc` | Generate tags from array data |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | `UriTag::__invoke()` | `UtiTagTest::testInvoke` | URI to tag string conversion | | |
| | `UriTag::fromAssoc()` | `UtiTagTest::testFromAssoc` | Generate tags from array data | | |
| | `UriTag::__invoke()` | `UriTagTest::testInvoke` | URI to tag string conversion | | |
| | `UriTag::fromAssoc()` | `UriTagTest::testFromAssoc` | Generate tags from array data | |
🤖 Prompt for AI Agents
In `@tests/CACHE_DEPENDENCY_TESTS.md` around lines 79 - 80, The table entries
reference a misspelled test class name "UtiTagTest" instead of "UriTagTest";
update both occurrences in the document so the test references match the actual
test class. Locate the rows containing `UriTag::__invoke()` /
`UtiTagTest::testInvoke` and `UriTag::fromAssoc()` / `UtiTagTest::testFromAssoc`
and replace `UtiTagTest` with `UriTagTest` so the `UriTag::__invoke()` and
`UriTag::fromAssoc()` tests are correctly named.
|
@coderabbitai Fixed UtiTagTest → UriTagTest in aeda788. |
|
Tip For best results, initiate chat on the files or code changes.
|
Summary
Machine-Readable Logging (BREAKING CHANGE)
RepositoryLoggerInterface::log()signature to structured format{"op":"save-value","uri":"...","tags":[...],"ttl":3600}LLM Documentation
docs/llms.txt- Concise project overviewdocs/llms-full.txt- Comprehensive documentationdocs/schemas/repository-log.json- Log format schemaTest Enhancement
Dependencies
Files Changed
src/RepositoryLogger.php,src/RepositoryLoggerInterface.phpsrc/ResourceStorage.php,src/QueryRepository.php,src/DonutRepository.phpdocs/llms.txt,docs/llms-full.txt,docs/schemas/repository-log.jsontests/RepositoryLoggerTest.php,tests/CacheDependencyTest.phpBreaking Change
検証：
AIの深い理解を試すために以下のログで質問
シナリオ
ログ
{"op":"request-start","uri":"page://self/dep/level-one"} {"op":"cache-miss","uri":"page://self/dep/level-one"} {"op":"put-query-repository","uri":"page://self/dep/level-one"} {"op":"invalidate-etag","tags":["_dep_level-one_"]} {"op":"cache-miss","uri":"page://self/dep/level-two"} {"op":"put-query-repository","uri":"page://self/dep/level-two"} {"op":"invalidate-etag","tags":["_dep_level-two_"]} {"op":"cache-miss","uri":"page://self/dep/level-three"} {"op":"put-query-repository","uri":"page://self/dep/level-three"} {"op":"invalidate-etag","tags":["_dep_level-three_"]} {"op":"save-etag","uri":"page://self/dep/level-three","etag":"3378406738","surrogateKeys":["_dep_level-three_"]} {"op":"save-value","uri":"page://self/dep/level-three","tags":["3378406738","_dep_level-three_"],"ttl":31536000} {"op":"depends-on","parent":"page://self/dep/level-two","child":"page://self/dep/level-three","childTags":["_dep_level-three_"]} {"op":"save-etag","uri":"page://self/dep/level-two","etag":"3579205492","surrogateKeys":["_dep_level-three_","_dep_level-two_"]} {"op":"cache-hit","uri":"page://self/dep/level-three"} {"op":"save-value","uri":"page://self/dep/level-two","tags":["3579205492","_dep_level-two_","_dep_level-three_"],"ttl":31536000} {"op":"depends-on","parent":"page://self/dep/level-one","child":"page://self/dep/level-two","childTags":["_dep_level-two_","_dep_level-three_"]} {"op":"save-etag","uri":"page://self/dep/level-one","etag":"3093767641","surrogateKeys":["_dep_level-two_","_dep_level-three_","_dep_level-one_"]} {"op":"cache-hit","uri":"page://self/dep/level-two"} {"op":"save-value","uri":"page://self/dep/level-one","tags":["3093767641","_dep_level-one_","_dep_level-two_","_dep_level-three_"],"ttl":31536000} {"op":"request-start","uri":"page://self/dep/level-one"} {"op":"cache-hit","uri":"page://self/dep/level-one"} {"op":"request-start","uri":"page://self/dep/level-three","method":"purge"} {"op":"purge-query-repository","uri":"page://self/dep/level-three"} {"op":"invalidate-etag","tags":["_dep_level-three_"]} {"op":"request-start","uri":"page://self/dep/level-one"} {"op":"cache-miss","uri":"page://self/dep/level-one"} {"op":"put-query-repository","uri":"page://self/dep/level-one"} {"op":"invalidate-etag","tags":["_dep_level-one_"]} {"op":"cache-miss","uri":"page://self/dep/level-two"} {"op":"put-query-repository","uri":"page://self/dep/level-two"} {"op":"invalidate-etag","tags":["_dep_level-two_"]} {"op":"cache-miss","uri":"page://self/dep/level-three"} {"op":"put-query-repository","uri":"page://self/dep/level-three"} {"op":"invalidate-etag","tags":["_dep_level-three_"]} {"op":"save-etag","uri":"page://self/dep/level-three","etag":"3378406738","surrogateKeys":["_dep_level-three_"]} {"op":"save-value","uri":"page://self/dep/level-three","tags":["3378406738","_dep_level-three_"],"ttl":31536000} {"op":"depends-on","parent":"page://self/dep/level-two","child":"page://self/dep/level-three","childTags":["_dep_level-three_"]} {"op":"save-etag","uri":"page://self/dep/level-two","etag":"3579205492","surrogateKeys":["_dep_level-three_","_dep_level-two_"]} {"op":"cache-hit","uri":"page://self/dep/level-three"} {"op":"save-value","uri":"page://self/dep/level-two","tags":["3579205492","_dep_level-two_","_dep_level-three_"],"ttl":31536000} {"op":"depends-on","parent":"page://self/dep/level-one","child":"page://self/dep/level-two","childTags":["_dep_level-two_","_dep_level-three_"]} {"op":"save-etag","uri":"page://self/dep/level-one","etag":"3093767641","surrogateKeys":["_dep_level-two_","_dep_level-three_","_dep_level-one_"]} {"op":"cache-hit","uri":"page://self/dep/level-two"} {"op":"save-value","uri":"page://self/dep/level-one","tags":["3093767641","_dep_level-one_","_dep_level-two_","_dep_level-three_"],"ttl":31536000} {"op":"request-start","uri":"page://self/dep/parent-a"} {"op":"cache-miss","uri":"page://self/dep/parent-a"} {"op":"put-query-repository","uri":"page://self/dep/parent-a"} {"op":"invalidate-etag","tags":["_dep_parent-a_"]} {"op":"cache-miss","uri":"page://self/dep/child-c"} {"op":"put-query-repository","uri":"page://self/dep/child-c"} {"op":"invalidate-etag","tags":["_dep_child-c_"]} {"op":"save-etag","uri":"page://self/dep/child-c","etag":"2837164676","surrogateKeys":["_dep_child-c_"]} {"op":"save-value","uri":"page://self/dep/child-c","tags":["2837164676","_dep_child-c_"],"ttl":31536000} {"op":"depends-on","parent":"page://self/dep/parent-a","child":"page://self/dep/child-c","childTags":["_dep_child-c_"]} {"op":"save-etag","uri":"page://self/dep/parent-a","etag":"60172706","surrogateKeys":["_dep_child-c_","_dep_parent-a_"]} {"op":"cache-hit","uri":"page://self/dep/child-c"} {"op":"save-value","uri":"page://self/dep/parent-a","tags":["60172706","_dep_parent-a_","_dep_child-c_"],"ttl":31536000} {"op":"request-start","uri":"page://self/dep/parent-b"} {"op":"cache-miss","uri":"page://self/dep/parent-b"} {"op":"put-query-repository","uri":"page://self/dep/parent-b"} {"op":"invalidate-etag","tags":["_dep_parent-b_"]} {"op":"cache-hit","uri":"page://self/dep/child-c"} {"op":"depends-on","parent":"page://self/dep/parent-b","child":"page://self/dep/child-c","childTags":["_dep_child-c_"]} {"op":"save-etag","uri":"page://self/dep/parent-b","etag":"1005023576","surrogateKeys":["_dep_child-c_","_dep_parent-b_"]} {"op":"cache-hit","uri":"page://self/dep/child-c"} {"op":"save-value","uri":"page://self/dep/parent-b","tags":["1005023576","_dep_parent-b_","_dep_child-c_"],"ttl":31536000} {"op":"request-start","uri":"page://self/dep/child-c","method":"purge"} {"op":"purge-query-repository","uri":"page://self/dep/child-c"} {"op":"invalidate-etag","tags":["_dep_child-c_"]} {"op":"request-start","uri":"page://self/dep/parent-a"} {"op":"cache-miss","uri":"page://self/dep/parent-a"} {"op":"put-query-repository","uri":"page://self/dep/parent-a"} {"op":"invalidate-etag","tags":["_dep_parent-a_"]} {"op":"cache-miss","uri":"page://self/dep/child-c"} {"op":"put-query-repository","uri":"page://self/dep/child-c"} {"op":"invalidate-etag","tags":["_dep_child-c_"]} {"op":"save-etag","uri":"page://self/dep/child-c","etag":"2837164676","surrogateKeys":["_dep_child-c_"]} {"op":"save-value","uri":"page://self/dep/child-c","tags":["2837164676","_dep_child-c_"],"ttl":31536000} {"op":"depends-on","parent":"page://self/dep/parent-a","child":"page://self/dep/child-c","childTags":["_dep_child-c_"]} {"op":"save-etag","uri":"page://self/dep/parent-a","etag":"60172706","surrogateKeys":["_dep_child-c_","_dep_parent-a_"]} {"op":"cache-hit","uri":"page://self/dep/child-c"} {"op":"save-value","uri":"page://self/dep/parent-a","tags":["60172706","_dep_parent-a_","_dep_child-c_"],"ttl":31536000} {"op":"request-start","uri":"page://self/dep/parent-b"} {"op":"cache-miss","uri":"page://self/dep/parent-b"} {"op":"put-query-repository","uri":"page://self/dep/parent-b"} {"op":"invalidate-etag","tags":["_dep_parent-b_"]} {"op":"cache-hit","uri":"page://self/dep/child-c"} {"op":"depends-on","parent":"page://self/dep/parent-b","child":"page://self/dep/child-c","childTags":["_dep_child-c_"]} {"op":"save-etag","uri":"page://self/dep/parent-b","etag":"1005023576","surrogateKeys":["_dep_child-c_","_dep_parent-b_"]} {"op":"cache-hit","uri":"page://self/dep/child-c"} {"op":"save-value","uri":"page://self/dep/parent-b","tags":["1005023576","_dep_parent-b_","_dep_child-c_"],"ttl":31536000}回答