Reuse entity graph during bench runs#21
Closed
Iron-Ham wants to merge 1 commit into
Closed
Conversation
|
@Iron-Ham is attempting to deploy a commit to the rs545837's projects Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
inspect review
Triage: 22 entities analyzed | 0 critical, 0 high, 16 medium, 6 low
Verdict: standard_review
Findings (3)
- [low] In
analyze_with_options_and_graph, the function usesgraph.file_countfor timing but this represents the file count from when the graph was built, not the current analysis. This creates inconsistent timing data wherefile_countdoesn't match the actual files processed in the current diff analysis. - [low] In
analyze_added_function, the code attempts to readdep.start_line.saturating_sub(1)from file lines, but ifstart_lineis 0,saturating_sub(1)returns 0, which would incorrectly access the first line instead of returning None for an invalid line number. - [low] In
analyze_added_function, the code attempts to get line content withlines.get(dep.start_line.saturating_sub(1))butstart_lineis 1-indexed while array access is 0-indexed. Ifstart_lineis 0 (which shouldn't happen but isn't validated),saturating_sub(1)returns 0, accessing line 1 instead of erroring, causing off-by-one access.
Reviewed by inspect | Entity-level triage found 0 high-risk changes
7a0edc1 to
fd22ee7
Compare
There was a problem hiding this comment.
inspect review
Triage: 22 entities analyzed | 0 critical, 0 high, 16 medium, 6 low
Verdict: standard_review
Findings (3)
- [low] In
analyze_with_options_and_graph, the function usesgraph.file_countfor timing but this represents the file count from when the graph was built, not the current analysis. This creates inconsistent timing data wherefile_countdoesn't match the actual files processed in the current diff analysis. - [low] In
analyze_added_function, the code attempts to read a line at indexdep.start_line.saturating_sub(1)but this can still be out of bounds ifstart_lineis 0 (after saturation) or if the file has fewer lines thanstart_line. Theget()will return None but there's no validation thatstart_lineis valid before attempting access. - [low] In
analyze_added_function, the code attempts to readdep.start_line.saturating_sub(1)from file lines, but ifdep.start_lineis 0, this will try to access line index -1 (wraps to usize::MAX with saturating_sub), causing an out-of-bounds access or returning None incorrectly.
Reviewed by inspect | Entity-level triage found 0 high-risk changes
fd22ee7 to
2c9841f
Compare
There was a problem hiding this comment.
inspect review
Triage: 23 entities analyzed | 0 critical, 0 high, 17 medium, 6 low
Verdict: standard_review
Findings (3)
- [low] In
analyze_with_options_and_graph, the function passesrepo_pathassource_roottoanalyze_changes_with_graph, butrepo_pathis the repository root while the graph was built from a potentially different source tree (e.g., a specific commit's tree). This mismatch will causecollect_dependent_codeto read files from the wrong location wheninclude_dependent_codeis enabled. - [low] The
build_analysis_graphfunction builds a graph from the current working tree of the repository, but when analyzing historical commits withanalyze_with_graph, the graph entities may not match the entities at that commit. This will cause incorrect dependency analysis and risk scoring for historical commits. - [low] In
build_analysis_graph_from_files, the function signature takessource_root: &Pathbut the function returnsAnalysisGraphwhich doesn't store thesource_root. However,analyze_changes_with_graphneeds the correctsource_rootto callcollect_dependent_code. When using a prebuilt graph withanalyze_with_graph, the wrong path (repo_path instead of the commit-specific checkout) will be used.
Reviewed by inspect | Entity-level triage found 0 high-risk changes
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.
Summary
Closes #17
Test plan