Repository navigation
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Sort on the resolved id sort field
When id is mapped as a text field with a keyword/sort sub-field and HasSortableIdField remains true, IdTiebreakerField computes the sort-safe idSortFieldName, but this appends the raw resolved idField instead. Explicit sorts are normalized through GetSortFieldName, while the automatic tiebreaker is not, so a no-explicit-sort or default search_after query can still sort on id and hit Fielddata is disabled on text fields instead of using the sortable sub-field; append the sort-safe field name here (and in the analogous SearchAfter path).
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…#305) DefaultSortQueryBuilder/SearchAfterQueryBuilder unconditionally appended an id sort for deterministic pagination, but never verified that id was actually sortable server-side or that the model even implements IIdentity. This broke queries against externally-managed indexes (e.g. Logstash-written partitions with no id field) with "all shards failed" or "Fielddata is disabled on text fields" errors, and, more subtly, still failed after removing IIdentity from the model because GetResolvedField() falls back to the literal field name instead of null when nothing is mapped. Both builders now delegate to a shared IdTiebreakerField.TryResolve<T>() helper that skips the tiebreaker when the model doesn't implement IIdentity or when the index opts out via the new IIndex.HasSortableIdField (default true, so existing library-owned indexes are unaffected). DailyIndex/MonthlyIndex also gain an overridable GetIndexMappingFilter() so externally-managed indexes can point the mapping resolver at their real (unversioned) naming pattern, plus a warning log when no index matches the filter instead of failing silently.
2604b7f to
a641c7f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a641c7fd0a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Tie-break same-date candidates by highest parsed index version before falling back to ordinal name ordering, so a custom mapping filter that matches multiple versions picks v10 over v9. Pin the search-after cursor extension semantics with unit tests, document the all-null-cursor behavior, and reset the paging mode when SearchAfterPaging is disabled.
The all-null and mixed-null cursor cases are already pinned by SearchAfterQueryExtensionsTests; keep the new class focused on the null-reference, empty-array, token, and mode-reset cases it uniquely covers. Document that SearchAfter/SearchBefore always enable search-after paging, even when they clear the cursor.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f9f6f461b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cbc2462993
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 25703b89a0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73c2be7ea4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Bring the PR branch up to main at a61d541. Use GitHub's conflict-free merge tree for the verified main and PR heads; retain the PR head as the first parent.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Reference System.IO.Hashing directly where XxHash64 is used instead of relying on a transitive dependency. Migrate both test assemblies to xUnit's current Parallelization attribute while keeping tests sequential.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Share continuation identity across Live and PIT without changing PIT ownership. Reject reset or replaced sessions before cursor mutation and again after BeforeQuery, while preserving virtual dispatch, same-mode reapplication, and retry behavior.
niemyjski
changed the title
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Fixes #305. Externally managed indexes can opt out of automatic ID sorting while existing implementations retain their defaults. Cursor paging now preserves its Live/PIT session, respects PIT ownership, rejects incomplete results, bypasses cursor caches, and reverses copies of caller sorts. External-index documentation includes a complete constructor and mapping example.
Validation: clean .NET 8/10 builds and consumer checks; 152 core tests; 960 full ES9 tests; 63 related PIT cases on ES8 after the test repair, plus the prior 957-test full ES8 pass on unchanged runtime code. Formatting and independent review passed.
Final-commit Build/tests and CodeQL are green on
d4484525; package and release publishing steps were skipped. CLA passed on the final commit. Required GitHub reviewer approval is the only remaining external gate. Native macOS full runs encountered transport resets; unchanged binaries passed on Linux. Releases and package publication remain held.Verification and implementation details
IIndex.HasSortableIdFielddefaults totrue; cursor signatures and nullable return annotations remain compatible. Live paging without a safe automatic ID sort requires explicit stable, unique sorts.MappingIndexPatternandGetIndexDate()support unversioned external daily indexes. The complete documentation example compiles with nullable warnings treated as errors on both target frameworks.BeforeQuery.