Upgrade motley-slayer to >=0.8.1 (advanced-search extra) - #51
Conversation
…arch extra Bump the `motley-slayer` floor from 0.7.4 to 0.8.1 and switch the extra from the legacy `embedding-search` to `advanced-search` (the current name; `embedding-search` is now only a graph-less legacy alias). Updated in every reference: both `pyproject.toml` extras, `.mcp.json`, the `harness.py` install-hint message, and the floor-guard regex in `test_dev1546_distinct_dim_values.py`. `uv.lock` relocked (0.8.0 → 0.8.1; adds `ladybug` from advanced-search). 0.8.x fixes the filter-on-aggregate / query-declared-measure / inline- aggregation cases (DEV-1443, DEV-1568) that the query-syntax-rejection analysis (PR #50) found to be the dominant `filter_construction` failure mode in slayer-mode runs — those now compile to `HAVING` instead of being rejected. Full non-integration suite green on 0.8.1: 3220 passed, 94 skipped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughRenames the Changesembedding-search → advanced-search rename
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/test_dev1546_distinct_dim_values.py (1)
602-629:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlign the floor-guard threshold with the new 0.8.1 policy.
This test still asserts
>=0.7.2, so it won’t catch regressions below the new intended floor (>=0.8.1) introduced in this PR.Suggested update
-def test_motley_slayer_floor_is_at_least_0_7_2(): - """``pyproject.toml`` must pin ``motley-slayer >= 0.7.2`` in BOTH +def test_motley_slayer_floor_is_at_least_0_8_1(): + """``pyproject.toml`` must pin ``motley-slayer >= 0.8.1`` in BOTH @@ - # Both extras must use a >= floor at least 0.7.2 (allow patch bumps). + # Both extras must use a >= floor at least 0.8.1 (allow patch bumps). @@ - assert version >= (0, 7, 2), ( - f"motley-slayer floor {raw!r} is below 0.7.2 — " - "the distinct_dimension_values field requires 0.7.2+." + assert version >= (0, 8, 1), ( + f"motley-slayer floor {raw!r} is below 0.8.1 — " + "this repository now requires 0.8.1+." )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_dev1546_distinct_dim_values.py` around lines 602 - 629, The test function test_motley_slayer_floor_is_at_least_0_7_2 is currently checking that the motley-slayer version is at least 0.7.2, but the PR has introduced a new floor requirement of 0.8.1. Update the version tuple in the assertion statement from (0, 7, 2) to (0, 8, 1), and update the error message string to reference 0.8.1 instead of 0.7.2. Additionally, update the function docstring and any inline comments that mention 0.7.2 to reflect the new 0.8.1 floor requirement to keep the test documentation accurate.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@tests/test_dev1546_distinct_dim_values.py`:
- Around line 602-629: The test function
test_motley_slayer_floor_is_at_least_0_7_2 is currently checking that the
motley-slayer version is at least 0.7.2, but the PR has introduced a new floor
requirement of 0.8.1. Update the version tuple in the assertion statement from
(0, 7, 2) to (0, 8, 1), and update the error message string to reference 0.8.1
instead of 0.7.2. Additionally, update the function docstring and any inline
comments that mention 0.7.2 to reflect the new 0.8.1 floor requirement to keep
the test documentation accurate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dab4437f-4cb3-4836-9285-e27ed7dad53f
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.mcp.jsonpyproject.tomlsrc/bird_interact_agents/harness.pytests/test_dev1546_distinct_dim_values.py
What
motley-slayerfloor 0.7.4 → 0.8.1 in both theslayerandallextras.embedding-searchtoadvanced-search(the current name;embedding-searchis now only a graph-less legacy alias). Updated in every reference: bothpyproject.tomlextras,.mcp.json, theharness.pyinstall-hint message, and the floor-guard regex intests/test_dev1546_distinct_dim_values.py.uv.lock(0.8.0 → 0.8.1;advanced-searchaddsladybug).Why
The query-syntax-rejection analysis (PR #50) found that the dominant
filter_constructionfailure mode in slayer-mode runs (~77% of that bucket) was the agent filtering on aggregates / query-declared measures / inline aggregations — which 0.7.3 rejected. These were fixed in 0.8.x (DEV-1443, DEV-1568); on 0.8.1 they now compile toHAVING. Verified directly:num_customers >= 5(query-declared named measure)HAVING COUNT(DISTINCT customer_id) >= 5customer_id:count_distinct >= 5(inline aggregation)HAVING …amount:sum > 100(bare aggregate)HAVING SUM(amount) > 100Test plan
env -u SSH_AUTH_SOCK uv run --extra all --extra dev --extra pydantic-ai pytest→ 3220 passed, 94 skipped, 50 deselected on 0.8.1 + advanced-search.test_motley_slayer_floor_is_at_least_0_7_2) passes against the renamed extra (0.8.1 ≥ 0.7.2).Note: the 50 integration tests (real
slayer ingestsubprocesses) are excluded by default; a cloud smoke on 0.8.1 is the real end-to-end confirmation and is not part of this PR.🤖 Generated with Claude Code
Summary by CodeRabbit