-
Notifications
You must be signed in to change notification settings - Fork 25.7k
Consolidates troubleshooting content into the "Returning semantic field embeddings in _source" section #137233
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Consolidates troubleshooting content into the "Returning semantic field embeddings in _source" section #137233
Conversation
|
Pinging @elastic/core-docs (Team:Docs) |
🔍 Preview links for changed docs |
ℹ️ Important: Docs version tagging👋 Thanks for updating the docs! Just a friendly reminder that our docs are now cumulative. This means all 9.x versions are documented on the same page and published off of the main branch, instead of creating separate pages for each minor version. We use applies_to tags to mark version-specific features and changes. Expand for a quick overviewWhen to use applies_to tags:✅ At the page level to indicate which products/deployments the content applies to (mandatory) What NOT to do:❌ Don't remove or replace information that applies to an older version 🤔 Need help?
|
Mikep86
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good overall, thank you for following up on this! I left a couple comments about some minor adjustments we should consider.
docs/reference/elasticsearch/mapping-reference/semantic-text.md
Outdated
Show resolved
Hide resolved
docs/reference/elasticsearch/mapping-reference/semantic-text.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Mike Pellegrini <[email protected]>
docs/reference/elasticsearch/mapping-reference/semantic-text.md
Outdated
Show resolved
Hide resolved
docs/reference/elasticsearch/mapping-reference/semantic-text.md
Outdated
Show resolved
Hide resolved
| }, | ||
| "fields": [ | ||
| "_inference_fields" | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the version with exclude_vectors to false, this is the preferred way to retrieve the field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we start with the preferred and current way of doing things? We can document the old way but it should be a small warning section without needing to repeat the entire response (it's the same).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ve reordered the sections so that the 9.2 method appears first.
I also added a warning to the 9.0-9.1 method to emphasize that users on 9.2 or later should use the newer approach instead.
I thought it might work better to keep both methods as equivalent sections instead of putting the 9.0-9.1 version in a warning. Since this documentation set covers all versions starting from 9.0, it feels clearer to show both approaches while highlighting which versions they apply to. I labeled the current recommended method as applicable from version 9.2 to make that distinction clear.
Let me know what you think!
Mikep86
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The content looks good, thank you for the iterations! I left one comment about mentioning serverless in the applies_to tag.
Tangential to this PR, I think we can make a few changes to Example: Reindex while preserving embeddings to make it clearer:
- Use an
applies_totag to indicate that this example only applies to 9.2+ and serverless - Remove the section that uses the
fieldsparam. Thefieldsparam can't be used during reindexing, so mentioning it as part of reindexing is confusing.
That change could be it's own PR though. WDYT?
Thank you, really good catches! In my latest commit, I made the following changes:
What do you think about it now? |
Mikep86
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Structure looks good to me, I left a few comments about minor edits
docs/reference/elasticsearch/mapping-reference/semantic-text.md
Outdated
Show resolved
Hide resolved
docs/reference/elasticsearch/mapping-reference/semantic-text.md
Outdated
Show resolved
Hide resolved
docs/reference/elasticsearch/mapping-reference/semantic-text.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Mike Pellegrini <[email protected]>
Co-authored-by: Mike Pellegrini <[email protected]>
Mikep86
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you for the iterations!
jimczi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I didn't meant to block the merging with my old request for changes. :LGTM
szabosteve
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a few comments.
docs/reference/elasticsearch/mapping-reference/semantic-text.md
Outdated
Show resolved
Hide resolved
| 3. The embeddings generated for this chunk. | ||
|
|
||
| ## Returning semantic field embeddings using `fields` [return-embeddings-fields] | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason not to add an applies_to tag that lists 9.0 and 9.1 and marks it unavailable from 9.2?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes - I didn’t want to mark this as unavailable from 9.2, since it’s not technically unavailable, it’s just not recommended to use.
I thought specifically listing 9.0 and 9.1 might be a bit misleading, as it could imply that this parameter is only available starting from 9.0. But I’m open to adding the 9.0 and 9.1 tags here, I was thinking about that myself too. Do you think it would be better?
Co-authored-by: István Zoltán Szabó <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If Jim and Mike are happy, I'm happy 😄 and we'll revisit this page to break it up into logical subpages and cleanup versioning conditionals as much as possible in future work
Thx!
…ld embeddings in _source" section (elastic#137233) * Consolidates the Troubleshooting section * Update docs/reference/elasticsearch/mapping-reference/semantic-text.md Co-authored-by: Mike Pellegrini <[email protected]> * Test applies switch tabs * Adds version specific examples * Syntax fix * Fixes syntax * Adds separate tabs for each versions * Adds only two version tahs * Deletes tabs, uses section level applies-to instead * Removes response block, adds intros and warning * Reorganizes content, adds applies_to where necessary * Update docs/reference/elasticsearch/mapping-reference/semantic-text.md Co-authored-by: Mike Pellegrini <[email protected]> * Update docs/reference/elasticsearch/mapping-reference/semantic-text.md Co-authored-by: Mike Pellegrini <[email protected]> * Adds additional information, changes admonition types * Update docs/reference/elasticsearch/mapping-reference/semantic-text.md Co-authored-by: István Zoltán Szabó <[email protected]> * Shorten tab titles --------- Co-authored-by: Mike Pellegrini <[email protected]> Co-authored-by: István Zoltán Szabó <[email protected]>
…-json
* upstream/main:
Mute org.elasticsearch.xpack.inference.action.filter.ShardBulkInferenceActionFilterBasicLicenseIT testLicenseInvalidForInference {p0=false} elastic#137691
Mute org.elasticsearch.xpack.inference.action.filter.ShardBulkInferenceActionFilterBasicLicenseIT testLicenseInvalidForInference {p0=true} elastic#137690
[LTR] Fix feature display order when using explain. (elastic#137671)
Remove extra RemoteClusterService instances in unit test (elastic#137647)
Fix `ComponentTemplatesFileSettingsIT.testSettingsApplied` (elastic#137669)
Consolidates troubleshooting content into the "Returning semantic field embeddings in _source" section (elastic#137233)
Update bundled JDK to 25.0.1 (elastic#137640)
resolve indices for prefixed _all expressions (elastic#137330)
ESQL: Add TopN support for exponential histograms (elastic#137313)
allows field caps to be cross project (elastic#137530)
ESQL: Add exponential histogram percentile function (elastic#137553)
Wait for nodes to have downloaded databases in `GeoIpDownloaderIT` (elastic#137636)
Tighten on when THROTTLE decision can be returned (elastic#136794)
Mute org.elasticsearch.xpack.esql.qa.single_node.GenerativeMetricsIT test elastic#137655
Add a test for two little known conditional processor paths (elastic#137645)
Extract a common ORIGIN constant (elastic#137612)
Remove early phase failure in batched (elastic#136889)
Returning correct index mode from get data streams api (elastic#137646)
[ML] Manage AD results indices (elastic#136065)
…-json
* upstream/main:
Mute org.elasticsearch.xpack.inference.action.filter.ShardBulkInferenceActionFilterBasicLicenseIT testLicenseInvalidForInference {p0=false} elastic#137691
Mute org.elasticsearch.xpack.inference.action.filter.ShardBulkInferenceActionFilterBasicLicenseIT testLicenseInvalidForInference {p0=true} elastic#137690
[LTR] Fix feature display order when using explain. (elastic#137671)
Remove extra RemoteClusterService instances in unit test (elastic#137647)
Fix `ComponentTemplatesFileSettingsIT.testSettingsApplied` (elastic#137669)
Consolidates troubleshooting content into the "Returning semantic field embeddings in _source" section (elastic#137233)
Update bundled JDK to 25.0.1 (elastic#137640)
resolve indices for prefixed _all expressions (elastic#137330)
ESQL: Add TopN support for exponential histograms (elastic#137313)
allows field caps to be cross project (elastic#137530)
ESQL: Add exponential histogram percentile function (elastic#137553)
Wait for nodes to have downloaded databases in `GeoIpDownloaderIT` (elastic#137636)
Tighten on when THROTTLE decision can be returned (elastic#136794)
Mute org.elasticsearch.xpack.esql.qa.single_node.GenerativeMetricsIT test elastic#137655
Add a test for two little known conditional processor paths (elastic#137645)
Extract a common ORIGIN constant (elastic#137612)
Remove early phase failure in batched (elastic#136889)
Returning correct index mode from get data streams api (elastic#137646)
[ML] Manage AD results indices (elastic#136065)
…ld embeddings in _source" section (elastic#137233) * Consolidates the Troubleshooting section * Update docs/reference/elasticsearch/mapping-reference/semantic-text.md Co-authored-by: Mike Pellegrini <[email protected]> * Test applies switch tabs * Adds version specific examples * Syntax fix * Fixes syntax * Adds separate tabs for each versions * Adds only two version tahs * Deletes tabs, uses section level applies-to instead * Removes response block, adds intros and warning * Reorganizes content, adds applies_to where necessary * Update docs/reference/elasticsearch/mapping-reference/semantic-text.md Co-authored-by: Mike Pellegrini <[email protected]> * Update docs/reference/elasticsearch/mapping-reference/semantic-text.md Co-authored-by: Mike Pellegrini <[email protected]> * Adds additional information, changes admonition types * Update docs/reference/elasticsearch/mapping-reference/semantic-text.md Co-authored-by: István Zoltán Szabó <[email protected]> * Shorten tab titles --------- Co-authored-by: Mike Pellegrini <[email protected]> Co-authored-by: István Zoltán Szabó <[email protected]>
This PR merges the "Troubleshooting semantic_text fields" section into the "Returning semantic field embeddings in
_source" section to reduce duplication and improve clarity.It also adds an annotated example response that explains key fields (
inference_id,model_settingsandembeddings) in the_inference_fieldsobject.Related issue: elastic/docs-content#2955
(The scope of this issue has changed, as per this comment and my discussion with @Mikep86 )