Skip to content

Default highlightable? to false for returnable: false fields - #1131

Merged
jwils merged 1 commit into
mainfrom
highlight-returnable-false-defaulting
Apr 22, 2026
Merged

Default highlightable? to false for returnable: false fields#1131
jwils merged 1 commit into
mainfrom
highlight-returnable-false-defaulting

Conversation

@jwils

@jwils jwils commented Apr 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • highlightable? now returns false for returnable: false fields when no explicit highlightable: is set, providing storage savings by default since non-returnable fields don't need to remain in _source for highlighting
  • source_excludes_paths now checks highlightable? when deciding what to exclude from _source, so explicitly highlightable: true fields stay available for search highlight snippets even when returnable: false
  • Adds highlightable: true to the test schema's internal_name and internal_details widget fields to demonstrate the explicit opt-in pattern
  • Adds acceptance test covering filtering, sorting, grouping, aggregating, and highlighting of hidden (returnable: false, highlightable: true) fields

Why only highlightable?

highlightable? is the only -able? predicate where returnable: false creates a meaningful conflict. The other predicates all operate on indexed data (inverted index / doc values), which is always present regardless of _source settings:

  • sortable? — Sorting uses doc values, not _source
  • filterable? — Filtering runs against the inverted index
  • groupable? — Grouping uses terms aggregations on indexed data
  • aggregatable? — Aggregations (min, max, avg, cardinality, etc.) use indexed data
  • sub_aggregatable? — Nested sub-aggregations also work on indexed data

Highlighting is unique because the datastore must read the original text from _source to produce contextual snippets (<em>matched text</em>). When returnable: false excludes a field from _source, highlighting becomes impossible — making it the only predicate where returnable: false should influence the default.

Test plan

  • All 1627 elasticgraph-schema_definition unit tests pass locally
  • All 1336 elasticgraph-graphql unit tests pass locally (1 pre-existing unrelated timezone failure)
  • Full CI build passes

🤖 Generated with Claude Code

@jwils
jwils force-pushed the highlight-returnable-false-defaulting branch from d874301 to f8854db Compare April 16, 2026 16:36
Comment thread config/schema/widgets.rb Outdated
@jwils
jwils force-pushed the highlight-returnable-false-defaulting branch from f8854db to b27f90b Compare April 22, 2026 16:49
@jwils
jwils enabled auto-merge (squash) April 22, 2026 16:50
When a field has `returnable: false` and no explicit `highlightable:`
setting, `highlightable?` now returns false. This provides storage
savings by default, since the primary reason for `returnable: false` is
to exclude field data from `_source`. Users can still opt in to
highlighting with `returnable: false, highlightable: true`.

Also updates `source_excludes_paths` to check `highlightable?` when
deciding what to exclude from `_source`, so that explicitly
highlightable fields remain available for search highlight snippets.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jwils
jwils force-pushed the highlight-returnable-false-defaulting branch from b27f90b to b4f1e3d Compare April 22, 2026 16:50
@jwils
jwils merged commit c705c4d into main Apr 22, 2026
25 checks passed
@jwils
jwils deleted the highlight-returnable-false-defaulting branch April 22, 2026 17:11
jwils added a commit that referenced this pull request Apr 23, 2026
Distills the patterns Myron Marston applies when reviewing merged PRs
(naming at the caller's level of abstraction, respond_to? as a code
smell, wrapper-class DI pattern, load-bearing tests, RBS/YARD hygiene,
etc.) into an edit-first skill.

Invoked as /myron-polish, the skill walks the current branch's diff
against main and applies fixes directly rather than producing review
comments. It loops — re-scan, apply edits, run script/lint --fix /
script/spellcheck -w / script/type_check / script/run_gem_specs — until
a full iteration makes zero edits and every verification command passes.
Capped at 8 iterations.

Source material: Myron's review bodies and inline comments on merged
PRs #974, #973, #1067, #1066, #1108, #1120, #1131, #1134, #1144, #1107.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants