feat(xorq-search): plumb search term to JS as highlight_phrase via sd channel#765
Open
paddymul wants to merge 3 commits into
Open
feat(xorq-search): plumb search term to JS as highlight_phrase via sd channel#765paddymul wants to merge 3 commits into
paddymul wants to merge 3 commits into
Conversation
Pins the xorq equivalent of #758 polars Search → SDResult, but using `highlight_phrase` (list) rather than `highlight_regex` — ibis `StringValue.contains` is a literal substring match, so a phrase match on the JS side matches the actual filter semantics. Both tests fail today: the xorq path (XorqAutocleaning._apply_xorq_ops bypassing the lisp interpreter) has no sd channel and the search handler returns a bare expr. - test_search_op_delivers_highlight_phrase_into_displayer_args: end-to-end through XorqBuckarooWidget with quick_command_args, asserts highlight_phrase lands in displayer_args for each ibis-String column (a=name, b=role) and the integer column (c=score) stays clean. - test_empty_search_drops_highlight_from_displayer_args: clearing the search box pulls the highlight back out — symmetric to the empty-val short-circuit in _xorq_search. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: acbdd8eb66
ℹ️ 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".
Contributor
📦 TestPyPI package publishedpip install --index-strategy unsafe-best-match --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple/ buckaroo==0.14.2.dev26006473751or with uv: uv pip install --index-strategy unsafe-best-match --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple/ buckaroo==0.14.2.dev26006473751MCP server for Claude Codeclaude mcp add buckaroo-table -- uvx --from "buckaroo[mcp]==0.14.2.dev26006473751" --index-strategy unsafe-best-match --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple/ buckaroo-table📖 Docs preview🎨 Storybook preview |
… channel Mirrors #758 for the xorq backend. The xorq path doesn't go through configure_buckaroo's lisp interpreter (ibis exprs can't .copy()), so it can't reuse the SDResult machinery from #755 directly. Instead this adds an analogous sd channel inside XorqAutocleaning: - Handlers in _XORQ_OP_HANDLERS may now return either a bare expr (legacy) or (expr, sd_updates). _apply_xorq_ops accumulates the per-column sd entries across ops, merging col-by-col. - handle_ops_and_clean runs the accumulated updates through _rekey_op_sd_to_internal (the same helper PandasAutocleaning uses since #758) so orig-named entries land on buckaroo's internal a/b/c letter keys and compose cleanly with the summary_sd that XorqDataflow._get_summary_sd produces (also keyed by letter). - _xorq_search returns the filtered expr plus {col: {'highlight_phrase': [val]}} for every ibis-String column. Uses highlight_phrase (list of literal needles) rather than highlight_regex because ibis StringValue.contains is a literal substring match — matching the filter semantics on the highlight side avoids regex-metacharacter divergence. Scope: only the search command is wired today. The sd channel itself is generic — other ops can opt in by returning (expr, sd_updates). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Add an assert in handle_ops_and_clean that result_expr.columns == df.columns. The rekey runs against the input expr — correct today because the only sd-producing handler is _xorq_search (filter preserves identity), but a future op that renames/drops columns would silently mis-map the sd entries onto the wrong letter keys. - _apply_xorq_ops: type the signature (-> Tuple[Any, Dict[...]]) and collapse the dict accumulation to setdefault().update() — same semantics, less noise. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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.
Summary
Mirrors #758 for the xorq backend. The xorq path doesn't go through
configure_buckaroo's lisp interpreter (ibis exprs can't.copy()), so it can't reuse theSDResultmachinery from #755 directly. Instead this PR adds an analogous sd channel insideXorqAutocleaning:_XORQ_OP_HANDLERSmay now return either a bare expr (legacy) or(expr, sd_updates)._apply_xorq_opsaccumulates the per-column sd entries across ops, merging col-by-col.handle_ops_and_cleanruns the accumulated updates through_rekey_op_sd_to_internal(the same helper used byPandasAutocleaning.handle_ops_and_cleanin feat(polars-search): plumb search term to JS as highlight_regex via SDResult #758) so the orig-named entries land on buckaroo's internala/b/cletter keys and compose cleanly with thesummary_sdthatXorqDataflow._get_summary_sdproduces._xorq_searchreturns the filtered expr plus{col: {'highlight_phrase': [val]}}for every ibis-String column.Uses
highlight_phrase(list of literal needles) rather thanhighlight_regex: ibisStringValue.containsis a literal substring match, so a phrase highlight matches the actual filter semantics.Scope: only the search command is wired today. The sd channel itself is generic — other ops can opt in by returning
(expr, sd_updates).Test plan
pytest tests/unit/test_xorq_buckaroo_widget.py tests/unit/test_xorq_stats_v2.py tests/unit/test_xorq_df_stats_v2.py— 78 pass locallyTests added
TestSearchHighlight.test_search_op_delivers_highlight_phrase_into_displayer_args— end-to-end throughXorqBuckarooWidgetwithquick_command_args = {'search': ['admin']}. Assertshighlight_phrase == ['admin']lands indisplayer_argsfor both ibis-String columns (a=name,b=role) and the integer column (c=score) stays clean.TestSearchHighlight.test_empty_search_drops_highlight_from_displayer_args— clearing the search box ("") pulls the highlight back out, symmetric to the empty-val short-circuit in_xorq_search.TDD: failing-tests commit was pushed first; CI run on that commit will be visible failing before the implementation commit lands.
🤖 Generated with Claude Code