-
Notifications
You must be signed in to change notification settings - Fork 236
fix: determinism in file order #103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -65,7 +65,7 @@ def walk_files(root: Path, extensions: Sequence[str], ignore: Sequence[str] | No | |
| ignored = [] | ||
| extensions_as_patterns = [f"!*{ext}" for ext in extensions] | ||
| ignored.extend(extensions_as_patterns) | ||
| ignored.extend(_DEFAULT_IGNORED_DIRS) | ||
| ignored.extend(sorted(_DEFAULT_IGNORED_DIRS)) | ||
| # Always give user patterns preference | ||
| ignored.extend(ignore or []) | ||
| base_spec = GitIgnoreSpec.from_lines(ignored, backend="simple") | ||
|
|
@@ -109,23 +109,21 @@ def _walk( | |
| inherited_specs: list[IgnoreSpec], | ||
| ) -> Iterator[Path]: | ||
| """Recursive function for walking files under a directory.""" | ||
| active_specs = inherited_specs | ||
|
|
||
| spec = _load_ignore_for_dir(directory) | ||
| if spec is not None: | ||
| active_specs = [ | ||
| inherited_specs = [ | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is done like this to avoid an in-place edit of inherited_specs. |
||
| *inherited_specs, | ||
| IgnoreSpec(base=directory, spec=spec), | ||
| ] | ||
|
|
||
| for item in directory.iterdir(): | ||
| for item in sorted(directory.iterdir()): | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. idem, this could result in issues, so just to rule it out, we sort here. |
||
| # Don't follow symlinks | ||
| if item.is_symlink(): | ||
| continue | ||
| if _is_ignored(item, active_specs): | ||
| if _is_ignored(item, inherited_specs): | ||
| continue | ||
|
|
||
| if item.is_dir(): | ||
| yield from _walk(item, active_specs) | ||
| yield from _walk(item, inherited_specs) | ||
| elif item.is_file(): | ||
| yield item | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -107,10 +107,16 @@ def search_hybrid( | |
| normalized_semantic = _rrf_scores(semantic_scores) | ||
| normalized_bm25 = _rrf_scores(bm25_scores) | ||
|
|
||
| # Sort by the file path and start line to | ||
| # counteract randomness introduces by hashing. | ||
| all_candidates = sorted( | ||
| {*normalized_semantic, *normalized_bm25}, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is the actual bugfix |
||
| key=lambda c: c.start_line, | ||
| ) | ||
| combined_scores: dict[Chunk, float] = { | ||
| chunk: alpha_weight * normalized_semantic.get(chunk, 0.0) | ||
| + (1.0 - alpha_weight) * normalized_bm25.get(chunk, 0.0) | ||
| for chunk in set(normalized_semantic) | set(normalized_bm25) | ||
| for chunk in all_candidates | ||
| } | ||
|
|
||
| # Boost files with multiple relevant chunks. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was also a possible source of errors, although I think it actually wasn't causing an issue.