Interesting approach — the decision-signal regex filter is smart. Mining all commits would be noisy, but filtering for "decided", "because", "instead of" etc. catches the architectural reasoning that usually lives only in commit messages and PR descriptions.

Does it handle squash-merged PRs where the commit message is the PR body? That's where most of the decision context lives in GitHub-flow repos.

@jphein — squash-merged PRs are actually the best case for this feature. When a PR is squash-merged, git log sees a single commit whose message is the PR title + description (GitHubs default squash format). The decision-signal regex will catch the architectural intent from the PR description, and the collect_prs() path would also have fetched the full PR body and review threads independently.

The gap would be if the PR description is empty (just "closes #X") and all the decision context lived in review comments — in that case, the review thread content from gh pr view --json reviews is the only source. The implementation already handles this via the --include-reviews flag.

One thing worth noting: GitHubs squash commit format includes Co-authored-by lines but not the individual commits that were squashed. If the squashed commits had good messages, theyre gone. For repos that squash everything, mining PR reviews directly is often more valuable than commit history.

The room naming collision I flagged in my review (git-decisions collides across repos) becomes more important here — for a multi-repo palace, include the repo name in the default room or the squash commits from different repos will land in the same room with no way to filter them.

[MemPalace-AGI integration — 540+ discoveries, 5,251 KG triples]

Makes sense — squash commits carrying PR bodies are ideal input. Good point on the room naming collision for multi-repo palaces. Per-repo room naming (e.g., git-decisions-mempalace) would solve it cleanly.

Thanks for the thorough review — really useful feedback. Here's what was addressed in the latest commit (37c26d3):

Comment 1:

• NUL separator — _LOG_SEP switched from ">>MP<<" to "\x00" with %x00 in the git log format string. NUL can't appear in commit messages so collision is impossible.
• Full SHA in metadata — GitEntry gained a git_sha slot carrying the full 40-char SHA. The ChromaDB upsert now stores it as a git_sha metadata field alongside the existing short ref. You can now look up entries by exact SHA.
• Warning on silent skip — the except ValueError: continue block now prints to stderr with the entry source, ref, and the exception message before skipping.
• collect_prs JSON keys — confirmed: the real implementation calls gh pr list --json number,title,body,author,createdAt which maps exactly to the FAKE_PR_LIST fixture keys.

Comment 2:

• gh unauthenticated stdin hang — both gh pr list and gh pr view calls now pass stdin=subprocess.DEVNULL. An unauthenticated gh will fail with a non-zero exit code rather than prompting interactively.
• hashlib.sha256 stability — confirmed, hashlib.sha256 is used throughout drawer_id(). Python's built-in hash() is never used.
• since ISO date parsing — no Python-side parsing needed: --since is passed directly to git log which accepts all ISO 8601 variants natively (date-only, with time, with timezone offset). Verified locally.
• Room naming collision — resolved by the default-wing change: each repo now gets its own wing (e.g. zendesk_halon) so git-decisions is scoped per repo. Multiple repos no longer collide.

Additionally, two larger design changes were made beyond the reviewer items:

• PR-first storage model — one drawer per merged PR with review threads folded in, plus one drawer per standalone commit not associated with any fetched PR. Commit SHAs belonging to a PR are collected from gh pr view --json commits and excluded from the commit scan to avoid duplication.
• Structured diff summary — --diff-summary=always|fallback|never appends a --- Code changes --- block to PR drawers listing changed files and touched function names extracted from unified diff hunk headers (via gh api /pulls/{n}/files). No raw diff content stored.

Test count is up from 13 to 58.

The v2 revision addresses everything from both rounds of feedback cleanly.

The NUL separator is the right call — unambiguous, collision-proof. The full SHA in metadata is a nice addition beyond what I asked for; exact-SHA lookup is genuinely useful if cross-referencing a specific commit across repos in the same palace.

The PR-first storage model (one drawer per merged PR with folded-in review threads, separate drawers for standalone commits) is a good architectural improvement. The --diff-summary flag is well-designed — fallback as default degrades gracefully when gh is unauthenticated or the API rate limit is hit.

Per-repo wing scoping for room collision is the correct fix. git-decisions being wing-scoped means multi-repo palaces work without namespace collisions.

Going from 13 to 58 tests is the right jump for a miner that makes external subprocess calls and does content-based routing.

One remaining note: the gh api /pulls/{n}/files call for --diff-summary is per-PR, so on repos with 500+ PRs this becomes 500+ API calls in a single run. Worth a rate-limit note in the --diff-summary=always help text, or a --max-prs ceiling defaulting to ~200. Not a blocker, but could surprise users on large repos.

LGTM for the core implementation.

[MemPalace-AGI — git history mining is one of the more useful MemPalace surfaces for long-lived projects]

Good catch on the rate-limit concern. Changed in 089d0b2 — --diff-summary now defaults to fallback instead of always. Files are only fetched for PRs with no description (the case where the diff summary provides the most value). Users who want full diff coverage on every PR can opt in explicitly with --diff-summary=always.