Skip to content

fix(hooks): preserve dashed project names + route through normalize_wing_name#10

Merged
jphein merged 1 commit intomainfrom
fix/wing-derivation-dashes
May 6, 2026
Merged

fix(hooks): preserve dashed project names + route through normalize_wing_name#10
jphein merged 1 commit intomainfrom
fix/wing-derivation-dashes

Conversation

@jphein
Copy link
Copy Markdown
Owner

@jphein jphein commented May 6, 2026

Addresses two Copilot findings on the merged #9:

  • L527 (docstring): said spaces are 'collapsed' but only did .replace(' ', '_'); didn't normalize hyphens like normalize_wing_name does.
  • L543 (real bug): encoded.rsplit('-', 1)[-1] collapses realm-watchwatch, losing hyphenated project names AND diverging from mempalace mine wing normalization.

Fix:

  • Reorder resolution: try -Projects-<name> FIRST (preserves dashes), fall back to last-token only for non-Projects layouts (~/dev, ~/code).
  • Route the result through normalize_wing_name so hook-derived wings match operator-mined wing names exactly. Same project mined two ways now produces one wing.

4 new tests cover dashed-project (realm-watchrealm_watch), uppercase-mixed, and an explicit assertion that the hook output equals normalize_wing_name(basename) over the operator-mine input.

Tests: 1551 pass, 1 skipped, ruff clean.

🤖 Generated with Claude Code

…ing_name

Two findings from Copilot review on #9 (already merged):

1. **Last-dash-token rule lost hyphenated project names.** A project
   directory named ``realm-watch`` is encoded by Claude Code as
   ``-home-jp-Projects-realm-watch``, and the previous primary
   regex's ``rsplit('-', 1)[-1]`` returned ``watch`` — collapsing
   the project name. Reorder the resolution: try the explicit
   ``-Projects-<name>`` segment FIRST (preserves dashes), fall back
   to the last-dash-token only when the path is in a non-Projects
   layout (``~/dev/<parent>/<project>``).

2. **Inconsistent normalization vs operator mines.** Hook used
   ``.lower().replace(' ', '_')`` (no hyphen handling) while
   ``mempalace mine`` runs the dirname through
   ``normalize_wing_name`` (lowercases, dashes/spaces → underscores).
   Same project mined two ways produced two different wings.
   Route the hook through ``normalize_wing_name`` for parity.

Net behavior: ``-Projects-realm-watch`` → ``realm_watch`` (matches
operator-mine output). Existing tests for non-dashed projects still
pass; 4 new tests cover dashed-project + uppercase-mixed cases +
explicit operator-mine convergence assertion.

Tests: 1551 pass, 1 skipped. Ruff lint + format clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 6, 2026 02:05
@jphein jphein merged commit d76134d into main May 6, 2026
7 checks passed
@jphein jphein deleted the fix/wing-derivation-dashes branch May 6, 2026 02:05
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates Claude Code hook wing derivation so transcript-based ingestion aligns more closely with the codebase’s normalized wing naming behavior, especially for ~/Projects/... paths. It modifies the hook-side parser in mempalace/hooks_cli.py and adds regression tests in tests/test_hooks_cli.py.

Changes:

  • Route _wing_from_transcript_path() results through normalize_wing_name.
  • Prefer extracting names from -Projects-<name> segments before falling back to the last dash-delimited token.
  • Add regression tests for dashed and mixed-case project names plus parity with operator-side normalization.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
mempalace/hooks_cli.py Changes transcript-path wing derivation to normalize names and prioritize -Projects-... extraction.
tests/test_hooks_cli.py Adds regression coverage for dashed/cased project-name handling and normalization parity.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread mempalace/hooks_cli.py
Comment on lines +549 to +556
# Secondary: last dash-separated token of ``/.claude/projects/-...``.
# Used for projects under non-Projects parents (e.g. ~/dev, ~/code).
match = re.search(r"/\.claude/projects/-([^/]+)", normalized)
if match:
encoded = match.group(1)
project = encoded.rsplit("-", 1)[-1]
if project:
return project.lower().replace(" ", "_")
# Legacy fallback: explicit ``-Projects-<name>`` segment, useful for
# transcripts not under the standard Claude Code projects dir.
match = re.search(r"-Projects-([^/]+?)(?:/|$)", normalized)
if match:
return match.group(1).lower().replace(" ", "_")
return normalize_wing_name(project)
jphein added a commit that referenced this pull request May 6, 2026
Four findings, batched into one cleanup PR.

PR #6 — hooks_cli.hook_precompact docstring lie. Said "mine the
transcript synchronously" but the local-mode _ingest_transcript
uses subprocess.Popen (background) and daemon-strict mode is
fire-and-forget HTTP. Rewrite docstring to describe the actual
two-step flow: (1) transcript ingest (best-effort, may still be
running when compaction starts), (2) _mine_sync of MEMPAL_DIR
(real synchronous subprocess.run).

PR #7 — cmd_mined unpaginated col.get + silent truncation. Old
flow called col.get(where=..., include=[]) once to size the loop,
then paginated with the same where clause. ChromaDB's get() has
a silent ~10K id truncation, so the upfront sizing call would
undercount on a wing >10K drawers. Drop the upfront sizing
entirely; loop on empty-batch instead. Plus an early-exit when
a batch is shorter than batch_size (saves one trailing empty
get on exact-multiple wing counts).

PR #8 — cmd_repair docstring stale. Said "rebuild palace vector
index" but the function still dispatches max-seq-id mode. Rewrite
to describe both modes + acknowledge the deleted reorganize mode.

PR #10 — non-Projects dashed-project divergence. Documented
limitation. Tried adding -dev-/-code-/etc. markers; reverted
because those are also ambiguous (`~/dev/<project>` and
`~/dev/<parent>/<project>` encode identically — a -dev-<rest>
match would catch parent+project, breaking the existing
test_wing_from_transcript_path_non_projects_layout test that
expects the last-token fallback for `-dev-MemPalace-mempalace`
→ `mempalace`). Instead document the limitation in the docstring
and add a regression test pinning the current behavior so future
"fixes" don't break the layouts that DO want the fallback.

Tests: +1 (regression test for the documented limitation). Full
suite: 1552 passed, 1 skipped, ruff clean.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

2 participants