DOC-6994 Document JSON HIGHLIGHT/SUMMARIZE support and limitations [PARKED] - #3858
Open
dwdougherty wants to merge 4 commits into
Open
DOC-6994 Document JSON HIGHLIGHT/SUMMARIZE support and limitations [PARKED]#3858dwdougherty wants to merge 4 commits into
dwdougherty wants to merge 4 commits into
Conversation
MOD-16530 relaxes the engine's blanket rejection of HIGHLIGHT and SUMMARIZE on JSON indexes. Four surfaces touched the topic and all three of the ones with prose disagreed with each other and with the engine, so the rules now live in one place on the Highlighting page and indexing, ft.search and the administration overview point at it instead of restating. The surprise was which direction the docs were wrong in. indexing/_index.md already described roughly the post-fix behaviour and shipped a worked example, while the engine had rejected every JSON highlight since 2024 — so this change mostly makes the docs true rather than new. Two rules were never documented anywhere, and they are the substance of the addition: JSON requires an explicit RETURN, and a single-value path resolving to an array or object is accepted but silently skipped. Nothing here was executed. The fix is unreleased, so every example output is transcribed from the assertions in RediSearch PR 9410's pytest suite. I left out a SUMMARIZE reply body and the array-skip reply on purpose — their output is dialect-dependent and the tests only assert substring containment, so inventing those two blocks was the one way this page could have shipped fabricated output. Learned: docs claimed post-fix behaviour years before the engine had it; the fix made prose true rather than adding to it Constraint: every FT.SEARCH reply on the Highlighting page is transcribed from RediSearch PR 9410 pytest assertions, not from a running engine — re-derive from the tests rather than hand-editing Directive: do not add SUMMARIZE or array-skip reply bodies to the JSON examples without running a build that has the fix; their output is dialect-dependent Rejected: hand-editing data/commands_redisearch.json and data/commands_core.json | regenerated at release-docs time, so the edit would be silently superseded Rejected: page-level bannerText as the unreleased-feature guard | it banners the whole page, which would misdescribe 900 lines of GA content on ft.search.md — the version statement is the right section-level guard Recheck: the version lines in both TODO comments, once RediSearch backport PR 11046 merges and a maintenance release actually ships Gaps: no engine verification was possible; the vendored tmp/RediSearch checkout predates the fix and contradicts it Ticket: DOC-6994 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dwdougherty
added
do not merge yet
parked
Contributor
Contributor
🧠 Redis MemoryFound 7 related items from repository history:
Memory updated at 6e50845 |
Contributor
andy-stark-redis
approved these changes
Aug 26, 2026
andy-stark-redis
left a comment
andy-stark-redis
left a comment
Contributor
There was a problem hiding this comment.
Couple of minor comments but otherwise LGTM.
10 tasks
…t.md Co-authored-by: andy-stark-redis <164213578+andy-stark-redis@users.noreply.github.com>
Collaborator
Author
|
Thank you for the review, @andy-stark-redis! |
oshadmi
reviewed
Aug 30, 2026
oshadmi
reviewed
Aug 30, 2026
oshadmi
reviewed
Aug 30, 2026
oshadmi
reviewed
Aug 30, 2026
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
3 participants
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.
Documents JSON
HIGHLIGHT/SUMMARIZEbehaviour after MOD-16530 relaxes the engine's blanket rejection.The canonical rules live in one
## JSON indexessection on the Highlighting page;indexing/_index.md,ft.search.mdandadministration/overview.mdpoint at it instead of restating. Before this change all three disagreed with each other and with the engine —indexing/_index.mdalready described roughly the post-fix behaviour and shipped a worked example, while the engine had rejected every JSON highlight since 2024. Two rules were never documented anywhere: JSON requires an explicitRETURN, and a single-value JSONPath resolving to an array or object is accepted but silently skipped.The engine work is done — every backport has merged and MOD-16530 is Closed/Done — but nothing has shipped. The latest tags on the affected lines are
v8.4.10,v8.6.8,v8.8.0,v8.10.0. Every Jira fix version is stillreleased: false, with 8.4.14 / rse 8.6.12 / rse 8.8.6 targeting 2026-09-30 and Open Source 8.12 targeting 2026-10-30.Merging now would tell every current reader that JSON highlighting works when the engine still returns
HIGHLIGHT/SUMMARIZE is not supported with JSON indexes.Open question blocking merge: the pages now assert specific patch versions, and the 8.4 number is disputed — see checklist item 1.
Park manifest
Ticket: DOC-6994
Parked at: 2026-08-25 (re-parked 2026-09-08)
Trigger to pick up: a non-prerelease 8.4.x tag exists whose ancestry includes PR #11048's merge commit
d526e7ec03931f36469a7f216b76e1ec089f67da. Deliberately phrased as an ancestry check rather than a tag name, because the target patch number has already moved once (8.4.13 → 8.4.14) and is still disputed. Test it by finding the first 8.4.x tag abovev8.4.10and runninggh api repos/RediSearch/RediSearch/compare/d526e7ec03931f36469a7f216b76e1ec089f67da...<tag> --jq '{status,behind_by}', expectingbehind_by: 0. As of 2026-09-08 no such tag exists (v8.4.11,v8.4.13,v8.4.14all 404).Labels: parked, do not merge yet
Pinned sources (state observed at re-park time, 2026-09-08)
Backports target the release branches directly, so these are the branches the releases are cut from. Changes since the 2026-08-25 park are marked ▲.
masterbd3ec5dca46c22b54f0673104b64fd9020febade, updated 2026-08-21T16:29:33Zgh api repos/RediSearch/RediSearch/pulls/9410 --jq '{state,merged,merge_commit_sha,updated_at}'mastera3a09eb58e2ec51940fbd1c1402a2c80a8032a04, updated 2026-08-24T23:54:41Zgh api repos/RediSearch/RediSearch/pulls/11106 --jq '{state,merged,merge_commit_sha,updated_at}'8.4d526e7ec03931f36469a7f216b76e1ec089f67da— this is the trigger commitgh api repos/RediSearch/RediSearch/pulls/11048 --jq '{state,merged,merge_commit_sha,merged_at}'8.625d7b56de47e896ee9a60677459aa1148128c1d2. Was OPEN at park time and recorded as "the blocker" — that half of the old trigger is now satisfied. Landed with 3 cherry-pick conflicts resolved (Rustrlookuploader absent from 8.6, so those file changes were dropped; C loader used instead).gh api repos/RediSearch/RediSearch/pulls/11046 --jq '{state,merged,merge_commit_sha,merged_at}'8.8fe67f90b86ab478c0060e649670ee1b83e76b2b9gh api repos/RediSearch/RediSearch/pulls/11044 --jq '{state,merged,merge_commit_sha,updated_at}'8.10553687a37d9905d6a1538aa22a01f137e8c14a5cgh api repos/RediSearch/RediSearch/pulls/11043 --jq '{state,merged,merge_commit_sha,updated_at}'8.2merged: false). Confirmed by oshadmi in review: "8.2 doesn't include the fix."gh api repos/RediSearch/RediSearch/pulls/11049 --jq '{state,merged,updated_at}'8.6-rse518ca89cbc9830d505cea94f35bd999f4ad396f8gh api repos/RediSearch/RediSearch/pulls/11047 --jq '{state,merged,merge_commit_sha,updated_at}'8.8-rsebfb2aa1186d9272166f6e76bedc12805ef04a01agh api repos/RediSearch/RediSearch/pulls/11045 --jq '{state,merged,merge_commit_sha,updated_at}'v8.4.10,v8.6.8,v8.8.0,v8.10.0. Nothing shipped — unchanged since park.gh api 'repos/RediSearch/RediSearch/tags?per_page=100' --jq '.[].name' | grep -E '^v8\.(4|6|8|10)\.'released: false. v8.4.13 has been removed from the list, and rse 8.6.11 / 8.8.5 were replaced by 8.6.12 / 8.8.6. Jira updated 2026-08-31.getJiraIssue MOD-16530getJiraIssue MOD-17663Observed shape the page assumes
Semantics — confidence HIGH. Held identically across dialects 1–3+ in the merged test suite; this is the shape of the page and it survived review unchanged:
RETURNnaming the field → works, output matches HASH for the same content.RETURN, orRETURN 0→ rejected. Rationale on the page: withoutRETURN, JSON loads as one serialized value so individual fields never reach the highlighter.RETURNnot required.Version identifiers — confidence LOW, and NEW since the last park. The pages did not assert version numbers at park time; they now assert five, none of which can be checked against a tag because nothing has shipped. Tick each off individually:
v8.4.14for the 8.4 line — disputed. oshadmi's review comment (2026-08-30) said 8.4.13; Jira's fix version changed to 8.4.14 on 2026-08-31, the day after. The pages currently say 8.4.14, following Jira as the system of record. Not confirmed by a human.v8.6.10for the 8.6 line — from Jira fix versions only.v8.8.1for the 8.8 line — from Jira fix versions only.v8.10.1for the 8.10 line — from Jira fix versions only.Behaviour identifiers — confidence LOW. Transcribed from a diff and a test suite, never from a running engine. Error strings are prose inside
QueryError_SetErrorcalls, exactly what gets reworded before release:HIGHLIGHT/SUMMARIZE on JSON indexes requires RETURN with explicit field names—src/aggregate/aggregate_request.c,AREQ_ApplyContext, PR #9410 diff; asserted asno_return_errorintests/pytests/test_json.py.HIGHLIGHT/SUMMARIZE is not supported for JSON fields with multi-value JSONPath—src/aggregate/aggregate_request.c,AREQ_HasMultiValueHighlightFields, PR #9410 diff; asserted asmulti_value_error.Property `alias` is not in schema— asserted asalias_errorintests/pytests/test_json.py. Pre-existing schema validation, not added by #9410 — if it's absent on unpark, the cause is a different change, not a rename.Noise-cancelling <b>Bluetooth</b> headphones, anddescriptionleft un-highlighted underHIGHLIGHT FIELDS 1 name— output shape fromtest_highlight_single_value_json. The doc example reuses that fixture's content but its own key (item:1) and index name (itemIdx).RETURN 0producing the same error as omittingRETURN— asserted intest_json.py, not separately stated in the C diff.master. ▲ Partial evidence gained: the 8.6 backport carries commitb8bf56a87f20f7f307d06aea4247bff103a3e9cc"Validate multi-value JSONPath for all highlighted fields", so follow-up validation is on the release branches. But that commit's subject is about validating all highlighted fields, which is not verbatim #11106's "independent of the schema field type" — so treat this as supporting, not confirming.Two traps for whoever unparks this:
for dialect in range(1, MAX_DIALECT + 1)asserting success. The page correctly says nothing about dialects — do not "fix" it to claim DIALECT 3+ is rejected on the strength of that commit message.merged, not closure. The PR list makes #11049 look like a merged 8.2 backport. It ismerged: false.Re-check checklist
Harvested from the branch's
/reflecttrailers (commit999fc5008), plus review outcomes and predicted-to-change items. Two items from the previous manifest are now done and dropped: both<!-- TODO(DOC-6994) -->comments are resolved and deleted from the content, and the 8.6-backport half of the old trigger is satisfied.Highest risk:
AREQ_HasMultiValueHighlightFieldson the shipped 8.4 tag rather than relying onb8bf56a8's subject line.Constrainttrailer: everyFT.SEARCHreply on the Highlighting page came from pytest assertions, not execution.Also:
Directivetrailer: a SUMMARIZE reply body and the array-skip reply were deliberately omitted — their output is dialect-dependent and the tests only assert substring containment. Add them only with a real build; do not reconstruct them by hand.highlight.mdis stale.data/commands_redisearch.json(~1061, ~1112) anddata/commands_core.json(~11354, ~11409). They still carry the old blanket "not currently supported" summaries; upstreamcommands.jsonwas updated in #9410. Deliberately not hand-edited — regenerated at release-docs time, so an edit now would be silently superseded.bannerTextsubstitution still reads correctly./parkStep 4 normally wants a page-levelbannerText; it was deliberately not added, because it banners the entire page and would misdescribe ~900 lines of GA content onft.search.md. The section-level version statement is the guard instead. Not an oversight.Gapstrailer:tmp/RediSearchwas last fetched 2026-07-30 and itsmasterpredates and contradicts this fix. Do not use it to verify — re-fetch, or read upstream viagh.hugoand confirm the cross-links still resolve (#json-indexesonhighlight.md, linked fromindexing/_index.md×2,ft.search.md×2, andadministration/overview.md).On unpark, then
When the trigger fires, run
/unpark 3858. It reconciles the docs against the now-settled source and takes the PR through the normal/reflect→/finalizepipeline to merge./finalizeis deferred until then — squashing now would discard the episodic trailers this manifest was built from. Thedo not merge yetguard holds until/finalizecompletes.