Skip to content

Build graph metadata from analyzed revisions#19

Merged
rs545837 merged 1 commit into
Ataraxy-Labs:mainfrom
Iron-Ham:Iron-Ham/build-graph-from-target-tree
May 22, 2026
Merged

Build graph metadata from analyzed revisions#19
rs545837 merged 1 commit into
Ataraxy-Labs:mainfrom
Iron-Ham:Iron-Ham/build-graph-from-target-tree

Conversation

@Iron-Ham

Copy link
Copy Markdown
Contributor

Summary

  • Build graph metadata from the analyzed tree for commit and range scopes, and from the index for staged scopes
  • Keep the selected source tree alive for dependent snippets and predict output
  • Add regressions for commit, range, staged, dependent-code, and predict historical-tree behavior

Fixes #12

Test plan

  • cargo test -p inspect-core analyze::tests:: -- --nocapture
  • cargo test --workspace
  • cargo build -p inspect-cli
  • inspect diff --format json -C
  • inspect diff .. --format json -C
  • inspect diff --format json --dependents -C
  • inspect predict --format json -C
  • inspect bench --repo --limit 3
  • git diff --no-ext-diff --check

@vercel

vercel Bot commented May 20, 2026

Copy link
Copy Markdown

@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.

@inspect-review inspect-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inspect review

Triage: 28 entities analyzed | 0 critical, 0 high, 16 medium, 12 low
Verdict: standard_review

Findings (5)

  1. [low] In materialize_tree_source, the function calls object.peel_to_tree() which can fail if the object is not a commit or tree. For a commit SHA, this should first peel to commit then get the tree. The current code will fail for commit SHAs.
  2. [low] In analyze_with_options, the destructuring of AnalysisContext includes _source_tree which is prefixed with underscore to indicate it's unused, but this field must be kept alive for the lifetime of source_root since it's a TempDir. If _source_tree is dropped early, the temporary directory will be deleted while source_root still references it, causing file read failures.
  3. [low] In predict_with_options, the same issue exists where _source_tree from AnalysisContext is destructured but the code doesn't show it being kept alive. The ctx.source_root is used later to read files, but if _source_tree (the TempDir) is dropped, those file reads will fail.
  4. [low] In analyze_with_options, the _source_tree field is destructured but never used, and the TempDir will be dropped immediately after destructuring, causing the temporary directory to be deleted while source_root still points to it. This will cause file read failures when collect_dependent_code tries to read from source_root.
  5. [low] In predict_with_options, the _source_tree field is destructured from ctx but the destructuring is not shown in the diff. If it follows the same pattern as analyze_with_options, the TempDir will be dropped immediately, causing the temporary directory to be deleted while ctx.source_root is still being used to read file contents.

Reviewed by inspect | Entity-level triage found 0 high-risk changes

@Iron-Ham Iron-Ham marked this pull request as ready for review May 20, 2026 23:31
@Iron-Ham Iron-Ham force-pushed the Iron-Ham/build-graph-from-target-tree branch from 059e851 to 8cd0da7 Compare May 21, 2026 05:31

@inspect-review inspect-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inspect review

Triage: 28 entities analyzed | 0 critical, 0 high, 16 medium, 12 low
Verdict: standard_review

Findings (6)

  1. [low] Resource leak: TempDir in AnalysisContext is stored but never explicitly cleaned up. The _source_tree field is prefixed with underscore suggesting it's intentionally unused, but TempDir needs to be kept alive for the duration of the analysis. If AnalysisContext is dropped before all file reads complete, the temporary directory will be deleted while still in use.
  2. [low] Destructuring pattern doesn't use _source_tree field, causing premature TempDir cleanup. In analyze_with_options, the AnalysisContext is destructured but _source_tree is extracted and immediately dropped. This will delete the temporary directory before dependent_code collection tries to read files from source_root.
  3. [low] Same TempDir premature cleanup issue in predict_with_options. The ctx.source_root is used to read files but the TempDir that owns that directory is not kept alive, leading to use-after-free of the temporary directory.
  4. [low] Destructuring pattern doesn't bind _source_tree, causing premature TempDir cleanup. In analyze_with_options, the AnalysisContext is destructured but _source_tree is extracted and immediately dropped. This will delete the temporary directory before dependent_code collection tries to read files from source_root.
  5. [low] Same TempDir premature cleanup issue in predict_with_options. The ctx is moved and _source_tree is extracted but not kept alive, causing the temporary directory to be deleted before file reads in the loop that processes dependents.
  6. [low] Use-after-move error in predict_with_options: ctx is moved in the destructuring pattern, but then ctx.source_root is accessed later in the loop. This will cause a compilation error.

Reviewed by inspect | Entity-level triage found 0 high-risk changes

@rs545837 rs545837 merged commit a6cda9a into Ataraxy-Labs:main May 22, 2026
1 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Build graph metadata from the analyzed revision, not the current checkout

2 participants