Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 688 |
🟢 Coverage 51.84% diff coverage
Metric Results Coverage variation Report missing for c10b1061 Diff coverage ✅ 51.84% diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (c10b106) Report Missing Report Missing Report Missing Head commit (2597514) 90957 22653 24.91% Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#3211) 1985 1029 51.84% Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%1 Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
This was referenced Aug 8, 2026
This comment was marked as outdated.
This comment was marked as outdated.
dschmidt
force-pushed
the
feat/graph-search-query
branch
from
August 18, 2026 16:51
1b61ef6 to
fb1e41b
Compare
dschmidt
force-pushed
the
feat/graph-search-query
branch
4 times, most recently
from
August 31, 2026 18:37
c54ffe7 to
cc3e6f4
Compare
dschmidt
force-pushed
the
feat/graph-search-query
branch
from
September 1, 2026 05:48
cc3e6f4 to
af951f6
Compare
dschmidt
force-pushed
the
feat/graph-search-query
branch
from
September 1, 2026 07:11
08eddc9 to
ceb7df5
Compare
dschmidt
force-pushed
the
feat/graph-search-query
branch
3 times, most recently
from
September 3, 2026 08:33
2513263 to
71ca404
Compare
3 of 7 tasks
dschmidt
force-pushed
the
feat/graph-search-query
branch
3 times, most recently
from
September 12, 2026 15:38
beae60b to
654fba0
Compare
This comment was marked as outdated.
This comment was marked as outdated.
dschmidt
force-pushed
the
feat/graph-search-query
branch
from
September 12, 2026 16:23
654fba0 to
b2296c5
Compare
dschmidt
force-pushed
the
feat/graph-search-query
branch
20 times, most recently
from
October 3, 2026 14:59
dd45fed to
a441468
Compare
MS Graph style search endpoint: searchRequest with KQL queryString, from/size pagination (values outside the spec's bounds are rejected), entityTypes validated to driveItem, hits as driveItems with webUrl. Pages are stable: both engines and the cross-space merge break score ties by id, and OpenSearch counts every match. No engine pages beyond the first 10000 matches, the OpenSearch result window, so a from and size reaching beyond it are rejected. Properties the endpoint does not evaluate yet (sortProperties, geohashDefinition) answer 501 instead of being ignored, an unknown $expand answers 400, and a failing search service keeps its status. The proto carries the whole aggregation schema: results are positional, one per option at every level, metrics travel as accumulators (sum, count, min, max) so they merge across spaces whatever their kind, and aggregation filters name the selected buckets (terms, ranges) instead of query fragments. Hits reuse what the drive item listing has, which moves a little: the thumbnails helper is named for the drive item it fills (setDriveItemThumbnailsByID, it served shares only), the web URL of an id gets a helper (webURLForID), and the facets come out of the proto through one generic mapping.FromProto instead of a copy per facet.
Term buckets on both engines: bleve folds them from doc values (its facets cannot nest and read numbers as prefix-coded terms), OpenSearch uses terms aggregations. A request gets every bucket, up to 65535 per space with all levels counted (the OpenSearch default for search.max_buckets, held on bleve as well); beyond that it is refused. The aggregation package merges the per-space results by position, so several aggregations on one field stay apart, and shapes them per BucketDefinition (minimum count, sort, size) at every level. The range parser and the metric accumulators the merge and the validation rely on come with it. It also validates aggregations against the index mapping (a field the index knows and a client may name, a kind that fits its type; the mapping overrides mark the internal fields): the search service relies on it, the graph endpoint asks it first to save the round trip, next to its own checks against the spec, and with the field names of the request so an error reads like the request. A request names a field like the driveItem property, exactly so. A number is keyed as it is, a bool as true or false on both engines, an empty value is no bucket. Pinned in the parity suite as AGG-01 to 03, 17, 34, 37, 39 and 41 to 43. Ranges, metrics and sub-aggregations are declined until the engines evaluate them.
Numeric and date ranges on both engines: bleve folds them from doc values, OpenSearch uses range and date_range aggregations. Bounds are numbers or RFC3339 dates, one kind per aggregation, anything else is rejected. Every requested range is answered, an empty one with a count of zero, with no space answering too. AGG-04, 05 and 07 to 09 in the parity suite.
sum/min/max/avg over a numeric field: bleve folds the accumulators from doc values, OpenSearch reads them from a stats aggregation, the service layer reduces them to the value of the kind after the cross-space merge. A metric without a single value has no value. AGG-06, 18 and 21 in the parity suite.
Sub-aggregations on both engines: bleve folds child buckets below their parent through the same collector, OpenSearch nests them natively, so a request stays one search per space. AGG-10 to AGG-15, 20, 35, 40 and 44 in the parity suite, AGG-14 as a known OpenSearch divergence.
Server-issued aggregationFilterToken on buckets, consumed verbatim via searchRequest.aggregationFilters. The graph layer decodes a filter into its buckets (a term, a range, or an or(...) of them) and the engines match them natively: a key as the exact value of the field, a range from-inclusive and to-exclusive, so a drilldown returns exactly the matches the bucket counted. Range tokens use the MS Graph spelling (range(min, 1980), range(2010, max, to="le")). AGG-22 to AGG-33, 36 and 38 in the parity suite.
dschmidt
force-pushed
the
feat/graph-search-query
branch
from
October 3, 2026 15:39
a441468 to
2597514
Compare
dschmidt
marked this pull request as ready for review
October 3, 2026 15:46
Contributor
Author
|
I've spent quite some time on polishing this - I think it's ready for a first iteration of human feedback :) Let me know if you want to keep it as a self contained single PR or if I should split it up into more granular ones - every commit makes sense on its own. |
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.
MS-Graph-style
POST /graph/v1beta1/search/query(spec: opencloud-eu/libre-graph-api#34) with pagination, terms, range, metric and nested sub-aggregations plus aggregation filter tokens, on bleve and OpenSearch.Best reviewed commit by commit:
Aggregations are exact up to a limit: without
sizeevery bucket is returned,sizeonly shapes the response after the cross-space merge. Both engines answer 400 for more than 65535 buckets per space, all levels counted (OpenSearch'ssearch.max_bucketsdefault). Both engines also answer 400 for a page beyond the first 10000 matches (from + size > 10000, the OpenSearch result window);moreResultsAvailableis false on the last reachable page. Aggregations and aggregationFilters accept driveItem properties only (facets plus name, size, lastModifiedDateTime, mimeType, @libre.graph.tags), spelled like the property; the KQLqueryStringis untouched and keeps every index field and alias. sortProperties and geohashDefinition answer 501 until their follow-ups land.Behavior is pinned in the engine parity suite (AGG-01..45, drilldowns included, plus a 10050-song cardinality case) against bleve and a real OpenSearch. AGG-14 is a known OpenSearch deviation: MimeType's wildcard mapping serves no doc values, follow-up is a keyword sibling.
Honest thumbnails on search hits (
$expand=thumbnails) need #3210 plus additional changes on top, coming as a follow-up PR.Currently the LoCs are distributed on the commits as following:
It might be worth to split especially the first commits into individual PRs - or not.. I'm happy to do whatever helps to make this more reviewable :)