Skip to content

fix(warehouse): include tracked deleted files in summary + tracked-paths expansion (PER-186)#153

Merged
Shadowsong27 merged 4 commits into
mainfrom
feat/per-186-include-deleted-files
May 20, 2026
Merged

fix(warehouse): include tracked deleted files in summary + tracked-paths expansion (PER-186)#153
Shadowsong27 merged 4 commits into
mainfrom
feat/per-186-include-deleted-files

Conversation

@Shadowsong27
Copy link
Copy Markdown
Owner

Summary

  • summarize_changes.py:_expand_pattern and _tracked_paths.py:_expand_pattern filtered glob matches by Path.is_file(), silently excluding tracked-but-deleted files from /contribute-warehouse triage.
  • Supplement the glob branch with git ls-files -- <pattern> so tracked deletions are included.
  • Applied the fix to BOTH copies (skill + domain) to keep them in sync — consolidation deferred to PER-183.
  • 4 new tests in test_summarize_changes.py (glob+explicit deletion, is_dirty for D/D ) and 2 new tests in test_tracked_paths.py (glob+explicit deletion).

Closes PER-186.

Test plan

  • pytest test_summarize_changes.py test_tracked_paths.py → 37 passed
  • ruff check on touched files → clean
  • CI green
  • opencode-review bot clean

🤖 Generated by diligent-supervisor / delegate-to-oc (kimi-for-coding/k2p6)

The _expand_pattern helper in both summarize_changes.py and
_tracked_paths.py previously used glob.glob + Path.is_file() to resolve
beacon.yaml glob patterns. This silently excluded tracked-but-deleted
files because is_file() returns False for paths that no longer exist on
disk.

Fix: supplement the glob results with git ls-files for glob patterns.
git ls-files lists tracked paths regardless of working-tree existence,
so deleted files matching the pattern are now included. Explicit
(non-glob) patterns are left unchanged as before.

Also adds tests verifying:
- deleted glob match appears with D status
- deleted explicit path appears with D status
- is_dirty() correctly classifies ' D' and 'D ' deletion codes

Closes PER-186.
@github-actions
Copy link
Copy Markdown
Contributor

OpenCode Review

Model: openai/gpt-5.5
Reviewed commit: 93f2b755e00f4bd4cf6fee1692e85935d8887683
Workflow run: https://github.com/Shadowsong27/agentic-beacon/actions/runs/26093266821

I’ll check the repository review context instructions, then evaluate only the attached diff for correctness and project-rule violations.

Findings

  • [Medium] Staged deletions matched by globs are still omitted - libs/beacon/src/beacon/data/skills/contribute-warehouse/scripts/summarize_changes.py:56
    git ls-files -- <pattern> does not return files that have already been staged for deletion, so a D path matching contexts/*.md will not be summarized. Add staged deleted paths via git diff --cached --name-only --diff-filter=D -- <pattern> in both this script and the mirrored warehouse helper.

  • [Medium] Directory patterns still miss deleted tracked files - libs/beacon/src/beacon/domains/warehouse/_tracked_paths.py:64
    The fix only supplements wildcard patterns; beacon.yaml entries that name a directory still expand through filesystem globbing only, so deleted files under that directory disappear from tracked paths. Apply the same git-backed deleted-path supplementation to directory expansion, and mirror it in summarize_changes.py.

Open Questions

  • None.

Notes

  • PR title and commits were not included in the attached diff, so Conventional Commits could not be assessed.

Token Usage

Input tokens: 16464
Output tokens: 720
Reasoning tokens: 1741
Cache read tokens: 70144
Cache write tokens: 0
Total tokens: 89069

@Shadowsong27
Copy link
Copy Markdown
Owner Author

Supervisor triage — bot round 1

Bot reviewed commit: 93f2b755. CI green.

Findings

  • Medium — Staged deletions matched by globs are still omitted (summarize_changes.py:56 + mirror).
    • Decision: fix in round 2. Verified: git ls-files -- <pattern> excludes index-removed paths, so a D (staged-deleted) match is dropped.
  • Medium — Directory patterns still miss deleted tracked files (_tracked_paths.py:64 + mirror).
    • Decision: fix in round 2. Verified: _expand_pattern's directory branch (if p.is_dir()) uses glob only — no git supplementation.

Notes / residual risks

  • "PR title and commits were not included in the attached diff, so Conventional Commits could not be assessed."
    • Decision: acknowledged, no action — title fix(warehouse): ... follows convention; bot sees diff-only.

Dispatching round 2 fix-up.

🤖 diligent-supervisor

…ked files

- Add _git_tracked_or_staged_deleted helper in both _expand_pattern copies
  to supplement git ls-files with git diff --cached --diff-filter=D, catching
  staged deletions that ls-files alone misses.

- Fix directory-pattern branch to also use the git-backed helper, so deletions
  under a tracked directory are surfaced even when the working-tree directory
  has been removed by git rm.

- Add 6 new tests covering staged-deletion glob match, directory-pattern
  unstaged deletion, and directory-pattern staged deletion.
@github-actions
Copy link
Copy Markdown
Contributor

OpenCode Review

Model: openai/gpt-5.5
Reviewed commit: 0bab6ab7de9648eadf2b7aabb564f5b875fe24b9
Workflow run: https://github.com/Shadowsong27/agentic-beacon/actions/runs/26093694134

I’ll check the repository review context rules, then assess only the attached diff for concrete regressions and test gaps.

Findings

  • [High] Deleted explicit paths are still omitted - libs/beacon/src/beacon/domains/warehouse/_tracked_paths.py:154
    The new git-backed expansion only runs for glob and directory patterns. A tracked file listed explicitly in beacon.yaml and then deleted will make p.is_file() false and still return nothing, so the added test_deleted_explicit_path_included should fail. Fix by checking git-tracked/staged-deleted matches for non-glob explicit patterns before returning [].

  • [High] Summarizer has the same explicit-deletion gap - libs/beacon/src/beacon/data/skills/contribute-warehouse/scripts/summarize_changes.py:65
    summarize() uses the copied _expand_pattern, so explicitly configured deleted files are omitted from the contribution summary despite the new test expecting them. Apply the same explicit-path git lookup there, or share the expansion logic to avoid this duplicate bug.

Open Questions

  • None.

Notes

  • Review was limited to the attached diff.

Token Usage

Input tokens: 7801
Output tokens: 306
Reasoning tokens: 550
Cache read tokens: 27136
Cache write tokens: 0
Total tokens: 35793

@Shadowsong27
Copy link
Copy Markdown
Owner Author

Supervisor triage — bot round 2

Bot reviewed commit: 0bab6ab7. CI green across all jobs.

Findings

  • High"Deleted explicit paths are still omitted (_tracked_paths.py:154)"

    • Decision: false alarm. The bot missed the final return [pattern] fallthrough on line 99 of _tracked_paths.py (and the same in summarize_changes.py). Explicit-path patterns that don't exist on disk fall through both the glob branch (no *) and the dir branch (no trailing /, not a dir) and return [pattern] as-is. Verified by running the exact tests the bot claimed should fail:
      $ pytest -k deleted_explicit -v
      test_deleted_explicit_path_included PASSED
      test_deleted_explicit_path_appears_with_deletion_status PASSED
      
  • High"Summarizer has the same explicit-deletion gap (summarize_changes.py:65)"

    • Decision: false alarm, same root cause. Same fallthrough applies to summarize_changes.py. Same test confirms.

Notes / residual risks

  • "Review was limited to the attached diff." — acknowledged; the fallthrough is in the unchanged tail of _expand_pattern so it wasn't in the diff. That's why the bot missed it.
  • "PR title and commits were not included in the attached diff." — same as round 1; title fix(warehouse): ... follows convention.

PR is READY. CI green; bot findings verified-false. No round 3 needed.

🤖 diligent-supervisor

@Shadowsong27 Shadowsong27 merged commit 5765915 into main May 20, 2026
9 checks passed
@Shadowsong27 Shadowsong27 deleted the feat/per-186-include-deleted-files branch May 20, 2026 05:41
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