Provide resolver cache configuration#77
Merged
Merged
Conversation
…e-cache-behavior # Conflicts: # embedding/parsing/context.go # embedding/parsing/instruction.go
MykytaPimonovTD
approved these changes
Jun 30, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the fragmentation resolver to support a configurable cache size (enabling small-cache tests) and adds test coverage to prove least-recently-used (LRU) eviction behavior in the resolver cache, as requested in issue #62.
Changes:
- Made
fragmentation.NewResolveraccept a cache limit and return an error for invalid limits, introducingDefaultResolverCacheLimitfor the default behavior. - Updated embedding/parsing and embedding orchestration to construct resolvers using the default cache limit (handling the new constructor signature).
- Added fragmentation tests that exceed the cache capacity to validate LRU eviction, plus a guard test for invalid cache limits.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| fragmentation/resolver.go | Exposes a default cache limit constant and changes NewResolver to accept a cache limit and validate it. |
| fragmentation/fragmentation_test.go | Adds LRU eviction coverage (insert beyond limit) and invalid-limit coverage; introduces small test helpers. |
| embedding/processor.go | Updates processor construction to create a resolver with the default cache limit using the new API. |
| embedding/parsing/instruction.go | Updates instruction content resolution to lazily create a resolver using the default cache limit when needed. |
| embedding/parsing/instruction_test.go | Adds a test to verify default/shared resolver behavior leads to cached results across repeated content reads. |
| embedding/parsing/context.go | Updates context creation to use a resolver constructed with the default cache limit. |
| embedding/orchestration.go | Updates orchestration to create a shared resolver with the default cache limit for processing required docs. |
| // Returns: | ||
| // *Resolver - resolver with an independent cache. | ||
| // error - when cacheLimit is below one. | ||
| func NewResolver(cacheLimit int) (*Resolver, error) { |
Collaborator
Author
There was a problem hiding this comment.
It is the whole PR intention, to have it configurable.
Comment on lines
+68
to
+71
| resolver, err := fragmentation.NewResolver(fragmentation.DefaultResolverCacheLimit) | ||
| if err != nil { | ||
| return Processor{}, err | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
This cache limit is not intended to be configured by the user, for now.
Oleg-Melnik
approved these changes
Jul 1, 2026
Oleg-Melnik
left a comment
Oleg-Melnik
left a comment
Collaborator
There was a problem hiding this comment.
LGTM with Copilot’s comments addressed before merging.
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
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.
This PR provides configuration for the resolver cache and cache tests.
Resolves this issue.