Skip to content

Treat a Non-Blob ls-tree Entry as Absent in _git_revisions - #1019

Merged
ptr727 merged 1 commit into
developfrom
worktree-git-revisions-tree-type-fix
Aug 26, 2026
Merged

Treat a Non-Blob ls-tree Entry as Absent in _git_revisions#1019
ptr727 merged 1 commit into
developfrom
worktree-git-revisions-tree-type-fix

Conversation

@ptr727

@ptr727 ptr727 commented Aug 26, 2026

Copy link
Copy Markdown
Owner

Summary

git ls-tree only proved rel_path had some entry at a revision, not that the entry was a regular file: a file-to-directory transition (a canonical path later replaced by a directory of the same name) still returns a non-empty ls-tree line, so the code proceeded to git show, which succeeds on a tree path and returns a directory listing, not file content. That listing would then be hashed and compared as if it were the file's real text.

Fix

Parse the ls-tree entry's type field and only treat rel_path as present when it is blob. Anything else (a tree from a file-to-directory transition, a commit gitlink from a submodule) reads the same as absent. Added a self-test covering the file-to-directory transition.

Validation

  • python3 spec/audit.py --selftest
  • uvx ruff check / uvx ruff format --check spec/audit.py
  • uvx mypy spec/audit.py
  • python3 scripts/prose_lint.py (full check set)
  • python3 scripts/repo_gate.py

Raised by CodeRabbit on PR #1016 (develop -> main promotion).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved audit handling when a tracked file is replaced by a directory or submodule.
    • Such transitions are now processed safely without treating them as file content.
    • Added coverage for file-to-directory changes in the self-test suite.

git ls-tree only proved rel_path had *some* entry at a revision, not
that the entry was a regular file: a file-to-directory transition (a
canonical path later replaced by a directory of the same name) still
returns a non-empty ls-tree line, so the code proceeded to git show,
which succeeds on a tree path and returns a directory listing, not
file content. That listing would then be hashed and compared as if it
were the file's real text.

Parse the ls-tree entry's type field and only treat rel_path as
present when it is 'blob'. Anything else (a tree from a
file-to-directory transition, a commit gitlink from a submodule) reads
the same as absent. Added a self-test covering the file-to-directory
transition.

## Validation

- python3 spec/audit.py --selftest
- uvx ruff check / uvx ruff format --check spec/audit.py
- uvx mypy spec/audit.py
- python3 scripts/prose_lint.py (full check set)
- python3 scripts/repo_gate.py

Raised by CodeRabbit on PR #1016 (develop -> main promotion).
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Treat non-blob Git revisions as absent file content

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Treat trees and gitlinks as absent file content during revision history reads.
• Prevent directory listings from contaminating fidelity and staleness comparisons.
• Cover file-to-directory transitions with an isolated Git repository self-test.
Diagram

graph TD
    A["_git_revisions"] --> B["git log"] --> C["git ls-tree"] --> D{"Blob entry?"} -->|Yes| E["git show"] --> G["Revision history"]
    D -->|No| F["None content"] --> G
Loading
High-Level Assessment

The object-type check is the appropriate fix because git ls-tree already provides the authoritative type and git show does not fail for tree paths. Using git cat-file would add another subprocess without improving correctness, while relying on git show errors cannot distinguish trees because tree display succeeds.

Files changed (1) +58 / -8

Bug fix (1) +58 / -8
audit.pyIgnore non-blob entries in Git revision history +58/-8

Ignore non-blob entries in Git revision history

• Parses each 'git ls-tree' result and reads content only when the path resolves to a blob, representing trees, gitlinks, and absent paths as 'None'. Adds a self-test that replaces a tracked file with a directory and verifies the directory revision is treated as absent file content.

spec/audit.py

@coderabbitai

coderabbitai Bot commented Aug 26, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 43cc5ed6-2637-4d1e-a691-3e5973b934f6

📥 Commits

Reviewing files that changed from the base of the PR and between fb23136 and c0114cd.

📒 Files selected for processing (1)
  • spec/audit.py

Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.


📝 Walkthrough

Walkthrough

_git_revisions now returns None for paths represented by directories or submodules. The self-test suite verifies a file-to-directory transition and confirms that earlier file content remains available.

Changes

Git revision path handling

Layer / File(s) Summary
Non-regular path classification and validation
spec/audit.py
_git_revisions treats absent paths, directories, and gitlinks as contentless revisions while retaining unrelated git show failures. An offline self-test covers a file-to-directory transition.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to c0114

This localized change makes non-file tree entries behave as absent during revision checks, preventing directory listings from being treated as file content; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: treating non-blob ls-tree entries as absent in _git_revisions.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch worktree-git-revisions-tree-type-fix

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

qodo-code-review Bot commented Aug 26, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (1)

Grey Divider


Remediation recommended

1. _git_revisions Comment Too Long ✗ Dismissed 📜 Skill insight ⚙ Maintainability
Description
The added comments at the non-blob branch and new self-test wrap individual sentences across
multiple lines, including a three-line explanation that largely repeats the adjacent condition.
Comments should default to one line, use a second only for a genuine constraint the code cannot
express, and keep each sentence complete on a single comment line.
Code

spec/audit.py[R1762-1764]

+            # Absent (empty stdout), or present as something other than a regular file (a
+            # directory from a file-to-directory transition, a submodule gitlink): neither has
+            # file content to compare, so both read the same as a confirmed deletion.
Relevance

●●● Strong

Recent same-file precedents accept multiline prose/comment formatting fixes, including
one-sentence-per-line compliance.

PR-#978
PR-#901

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2826677 requires comments to be one line by default and allows a second line only
for genuine constraints, while PR Compliance ID 2826725 prohibits wrapping a sentence across comment
lines. Lines 1762-1764 split a condition-and-result explanation over three lines, and lines
3318-3319 split another sentence after per.

spec/audit.py[1762-1764]
spec/audit.py[3318-3319]
Skill: comment-and-doc-style

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new comments split sentences across multiple comment lines, and the non-blob explanation spans three lines despite largely elaborating behavior already expressed by the adjacent code.

## Issue Context
Rewrite each comment so that every sentence begins and ends on the same comment line. Comments should default to one concise line, with a second line used only for a genuine constraint the code cannot express; for the non-blob branch, retain only a brief explanation of why non-blobs are treated as absent.

## Fix Focus Areas
- spec/audit.py[1762-1764]
- spec/audit.py[3318-3319]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

2. Docstring Exposes Git Internals ✗ Dismissed 📜 Skill insight ✧ Quality
Description
The revised docstring details git show stderr behavior, tree transitions, gitlinks,
command-failure modes, and caching rather than limiting itself to _git_revisions' behavioral
contract. These implementation details make the contract harder to identify and become stale when
internals change.
Code

spec/audit.py[R1722-1725]

+    `text` is None where rel_path has no file content at this revision: absent (deleted, checked
+    against the revision's own tree rather than `git show`'s stderr wording, which reads
+    differently once rel_path exists again in a later commit), or present as something other than
+    a regular file (a directory from a file-to-directory transition, a submodule gitlink). A
Relevance

●●● Strong

Recent same-file precedents accept contract-focused docstring rewrites and implementation-detail
reductions.

PR-#978
PR-#901

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2827096 requires docstrings to focus on behavior contracts instead of
implementation details. The modified docstring names internal git show behavior and specific
repository object forms, then continues with command-failure and cache mechanics.

spec/audit.py[1722-1729]
Skill: python-codestyle

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The `_git_revisions` docstring describes internal Git commands, failure mechanisms, and caching implementation rather than only caller-observable behavior.

## Issue Context
State the returned tuple order, that entries without regular-file content use `None`, and which failures propagate. Move any essential implementation rationale to a concise inline comment near the relevant code.

## Fix Focus Areas
- spec/audit.py[1720-1729]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Test Comment References PR 📜 Skill insight ✧ Quality
Description
The new self-test comment cites ptr727/ProjectTemplate#1016, tying committed code to the review
task that prompted it. That task-specific provenance belongs in the PR description, not an inline
comment.
Code

spec/audit.py[R3318-3319]

+    # _git_revisions: a path that becomes a directory reads as None too, per
+    # ptr727/ProjectTemplate#1016 (git show on a tree path returns a listing, not file content).
Relevance

● Weak

A near-identical recent finding removing task-specific references from docstrings and test comments
was rejected.

PR-#1004

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2827092 excludes task- and PR-specific references from code comments. The added
comment explicitly names ptr727/ProjectTemplate#1016, which the PR description identifies as the
review that raised this change.

spec/audit.py[3318-3319]
Skill: python-codestyle

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The self-test comment references the pull request context through `ptr727/ProjectTemplate#1016`.

## Issue Context
Retain only the durable behavioral rationale, such as the fact that `git show` returns a listing for a tree path. The PR description already records where the issue originated.

## Fix Focus Areas
- spec/audit.py[3318-3319]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 70 rules
✅ Skills: 5 invoked
  comment-and-doc-style
  dotnet-codestyle
  python-codestyle
  shell-codestyle
  workflow-ci-contract
Review mode: ⚖️ Balanced

Grey Divider

Tip of the day
💡 Did you know, you can start a comment with 'qodo' or '@qodo' to chat about any finding

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread spec/audit.py
Comment thread spec/audit.py
@ptr727
ptr727 merged commit 02e57ed into develop Aug 26, 2026
8 checks passed
@ptr727
ptr727 deleted the worktree-git-revisions-tree-type-fix branch August 26, 2026 02:32
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.

1 participant