Repository navigation
#551 PR 2: delete the executor's query path, test-hooks, size gate - #716
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0169pNQsLcCEn48te9X9WuZW
src/query/engine/tests.rs, recursive_engine_test and plan_independence_test relied on the executor oracle to check their answers. Each answer is now also compared with one recorded in tests/answers/<suite>.txt, blessed on this commit with the oracle on, so every recorded answer agreed with the executor when it was written. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0169pNQsLcCEn48te9X9WuZW
…551) The two answer-change tests keep the engine's answer; the executor's is a comment. Steps 16-18 delete the code only the oracle reached. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0169pNQsLcCEn48te9X9WuZW
…aluators go (#551) Deleted with the code they tested: executor unit tests of its internals (apply_post_processing, filter_facts_for_query, selective_fact_fetch, apply_expr, evaluate_branch, branch_outer_reads, apply_window_functions, execute_query_with_rules), PatternMatcher's tests and the evaluator tests; each shape is covered on the engine by aggregation_test, bitemporal and retraction tests, join_rules_test, disjunction_test, window_functions_test, recursive_engine_test and recursive_rules_test. Ported where the engine had no test: the derived-fact limit through a not rule (recursive_engine_test) and count/sum over nil (aggregation_test). Tests that query through DatalogExecutor::execute stay: they run on the engine. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0169pNQsLcCEn48te9X9WuZW
…pers go (#551) Items the oracle shared with unit tests are cfg(test); FindSpec::var and DatalogQuery::get_top_level_rule_invocations had no other user and go with their tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0169pNQsLcCEn48te9X9WuZW
It gates the test-only switches (__reorder, __subplans, __tx_chunk_facts, __statement_facts_high_water) and nothing else. rust.yml's oracle job is the test-hooks job; smoke.yml and crash-kill.yml pass the new name; the integrity test that skipped under the oracle runs everywhere. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0169pNQsLcCEn48te9X9WuZW
With the executor's query path gone the release binary is 1,173 KiB (local stable build). The report-only gate and the temporary note in PHILOSOPHY.md §4 go (#680). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0169pNQsLcCEn48te9X9WuZW
live_facts' unknown entity and attribute reads were reached only through the executor; a unit test covers them. rule_invocation_to_pattern matches on the argument slice, without unreachable INT-022 arms; evaluator.rs has no branches left, so its branches floor goes. Also the docs and counts of PR 2 (CLAUDE.md, TEST_COVERAGE.md, plan decision 18 and deviations 7-8), and the ported test's recorded answer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0169pNQsLcCEn48te9X9WuZW
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
5 tasks
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.
Part 2 of 3 for #551 (streaming engine 9/9, part of #473). Plan:
docs/superpowers/plans/2026-10-23-551-executor-removal.md, steps 14–21. After PR 1 (#715) every query runs on the engine. This PR deletes the code that only the oracle reached. No answer changes.What goes
engine/oracle.rsand the oracle checks inopen_query/open_rule_query. The two answer-change tests (spec §4) keep the engine's answer; the executor's answer is now a comment.execute_query_on_executor,execute_query_with_rules,filter_facts_for_query,selective_fact_fetch,evaluate_branch,apply_*and their helpers),PatternMatcher,RecursiveEvaluator,StratifiedEvaluatorandevaluate_not_join. 6,104 lines deleted in that commit.FindSpec::varandDatalogQuery::get_top_level_rule_invocations, which had no other users. Fact-level storage reads that unit tests still use are nowcfg(test).Tests
query::engine::tests(itsrows()returns a count),recursive_engine_testandplan_independence_test. Their answers were recorded intests/answers/<suite>.txton the commit before the oracle goes (038f53b), with the oracle on, so every recorded answer had just matched the executor. Each line holds a test, a query digest, a row count and a digest of the rows as a bag, about 400 KB in total. Integers within a week of now (wall-clocktx-ids) compare as "now".MINIGRAF_BLESS_ANSWERS=1re-records. I checked that changing one recorded digest fails exactly one test. One answer was recorded after the freeze: the ported limit test below, which also asserts its own rows.apply_post_processingaggregation,filter_facts_for_query,selective_fact_fetch,apply_exprunification,evaluate_branch,branch_outer_reads,apply_window_functions, and thePatternMatcherand evaluator tests. Engine tests that cover the same shapes:aggregation_test,window_functions_test, the bitemporal/retraction/multi-value tests,join_rules_test(Expression binding a variable a pattern also binds depends on plan order #596 unification, keyword/ref, ±0.0),disjunction_test,recursive_engine_test,recursive_rules_test(QRY-013, INT-022, INT-020).notrule, exactly 7 versus 6 (Recursion through a rule with not / not-join / or returns too few rows #633);count/sumskippingnil; andlive_factsof unknown ids, which only the executor reached (keepsreader.rsat its coverage floor).DatalogExecutor::execute. They already run on the engine.--features test-hooks1,713 / 13. Was 1,767 / 12 (oracle builds: 1,782 / 16). Run on the bench machine. The coverage gates pass there too: total 95.32% lines / 85.53% branches.evaluator.rshas no branches left, so its branches floor is removed.test-oracle→test-hooksThe feature now gates only
__reorder,__subplans,__tx_chunk_factsand__statement_facts_high_water.rust.yml'soraclejob becomestest-hooks, andsmoke.ymlandcrash-kill.ymluse the new name. The matchingcrash-kill.ymlchange formainis a separate PR, so the file stays identical on both branches (#687).integrity_test's internal-node test was ignored under the oracle; it now runs in every build.Binary size
The release binary is 1,173 KiB (local stable build).
binary-size.ymlenforces 1,200 KiB again, and the report-only gate and the PHILOSOPHY.md §4 note are gone (#680). The gate no longer has aSIZE_GATEswitch at all (deviation 8), so the file differs frommain's only in its branch filters.Left for PR 3
CHANGELOG.md's per-PR engine entries still describe the steps in between ("still run on the executor"). PR 3 rewrites them along with the migration notes (spec §7). This PR only renamestest-oraclethere.Codex review: no findings in the code. It raised two weaknesses in the digest: the "now" mask would also cover data integers near the wall clock, and the separators are ambiguous if a string contained U+001E/U+001F. No test data does either, and the helper's doc now says so. It also flagged the CHANGELOG text above.
🤖 Generated with Claude Code
https://claude.ai/code/session_0169pNQsLcCEn48te9X9WuZW