Repository navigation
fix(query): count only derived facts against max_results (#528) - #534
Merged
Merged
Conversation
RecursiveEvaluator seeded seen_facts with every base fact and compared its size with max_results, so on a database with max_results (default 1,000,000) live facts every rule query failed with INT-020 at its first derived fact, whatever the size of its answer. The limit now counts only the facts the rules derive. At 1M facts, recursive_ancestors in the performance envelope now returns its 6 rows (40.1 s p50) instead of failing; BENCHMARKS.md and the README are updated. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012wXxZguv7PgKQjMMH4yJ5h
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
Closes #528.
Problem
RecursiveEvaluator::evaluate_recursive_rulesseededseen_factswith every base fact and returnedINT-020onceseen_facts.len() >= max_results. The limit counted the database, not what the rules derive: with 1,000,000 or more live facts (the default limit), every rule query failed at its first derived fact. Affects every release since v0.18.0.Fix
Count only facts added after the base facts (
seen_facts.len() - base_count). A runaway rule still fails withINT-020.Tests
test_max_results_counts_derived_facts_not_base_facts(tests/recursive_rules_test.rs):max_results(10)on 53 live facts; a bound-argument (magic sets) and an all-free rule query both answer;:max-results 5on a query deriving 6 facts is stillINT-020. Failed before the fix.rule_limits_apply_while_the_engine_materialises,test_stratified_max_results_limit) still pass.Measured
perf_envelope --scales 1m:recursive_ancestorsreturns its 6 rows in 40.1 s p50 / 40.8 s p99, 3.2 GiB RSS (wasINT-020). Still slow because rule evaluation reads the whole database (#529).Docs
OpenOptions::max_resultsdocs,ERROR_REFERENCE.md(INT-020 cause),CHANGELOG.md(Fixed),BENCHMARKS.md, README, test counts. Docs site: project-minigraf/minigraf-docs PR to follow (@until/@since v3.0.0text for:max-results).Not backported to v2.x: not a data-integrity fix. #421 should mark #528 as fixed in v3.0.0.
🤖 Generated with Claude Code
https://claude.ai/code/session_012wXxZguv7PgKQjMMH4yJ5h