Repository navigation
[FEATURE] Declare the restart parameters on extract() and document what a restart costs - #672
Open
noel-improv wants to merge 1 commit into
Open
noel-improv wants to merge 1 commit into
noel-improv wants to merge 1 commit into
Conversation
…publish what a restart costs extract() took run_id and run_plan_store only through **kwargs, so they were missing from its signature, its docstring and help(), and they also reached BuildPipeline.create, which has no use for them. They are now named parameters, documented, and passed to the extraction pipeline only. indexing.mdx gains the measured cost of a restart at 100, 500 and 1,000 documents, with the configuration behind each figure and what the figures leave out.
noel-improv
marked this pull request as ready for review
November 5, 2026 22:19
This branch has not been deployed
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.
Description
extract()acceptedrun_idandrun_plan_storeonly through**kwargs, so a restartable run was missing from its signature, its docstring andhelp(). This declares both as named parameters and documents what a restart costs, using the restart benchmark from #590.Changes
LexicalGraphIndex.extract()declaresrun_idandrun_plan_store(both defaultNone), documents them, and passes them to the extraction pipeline only. Before this they also reachedBuildPipeline.create, which does not use them.indexing.mdxadds "What a restart costs" to the restart section: per-restart time at 100, 500 and 1,000 documents, the configuration behind each figure, and what the figures leave out.Nonewhen not passed.No feature flag gates restart, so nothing was removed. I searched
lexical-graph/src,docs-site, the examples and everyenv.templateand README for one.Problem
Restart shipped in 3.20, but its parameters were undocumented on the method a caller uses, and there was no published number for what a restart costs.
Related issue (if any): #
Testing
pytest)tests/unit/test_lexical_graph_index.pyandtests/unit/indexing/extract: 409 passed. Withlexical_graph_index.pyreverted tomain, both new tests fail. The docs page was not built locally.Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.