chore(core): make ty the default typechecker

[phernandez]

Summary

• Make just typecheck run ty check src tests test-int and keep pyright available as just typecheck-pyright.
• Clean up source typing issues surfaced by ty, including API hydration, frontmatter config typing, repository update typing, telemetry attrs, and permalink/frontmatter coercion.
• Make unit and integration tests valid under ty with narrow assertions, casts, and typed fakes instead of pyright ignores.

Verification

• just format && just lint && just typecheck
• just test

[github-actions]

Claude finished @phernandez's task in 2m 46s —— View job

---

PR Review: chore(core): make ty the default typechecker

Tasks:

• Read CLAUDE.md for project context
• Analyze PR diff and changed files
• Review code quality and standards
• Check testing and documentation
• Verify architecture patterns
• Check security and performance
• Post final review

---

Summary

This is a well-executed toolchain migration. The PR makes ty the primary typechecker while keeping pyright available, and resolves the typing issues ty surfaced. The majority of changes are correctness improvements, not just silencing errors. A few items worth flagging:

---

Issues

1. Non-exhaustive match in to_summary (src/basic_memory/api/v2/utils.py:96-149)

The original case _: raise ValueError(f"Unexpected type: {item.type}") was removed (it was # pragma: no cover), but without it the function silently returns None at runtime if an unexpected SearchItemType value appears — despite the return type declaring EntitySummary | ObservationSummary | RelationSummary. The observations list defensively filters with isinstance(..., ObservationSummary), but the related list on line 166 does not:

related = [to_summary(rel) for rel in context_item.related_results]

A surprise None there would propagate into ContextResult(related_results=related) and fail at the Pydantic validation boundary instead of at the source. Recommend restoring the fallback:

case _:
    raise ValueError(f"Unexpected type: {item_type}")  # pragma: no cover

Fix this →

---

2. CLAUDE.md not updated to reflect new typecheck commands

The build commands section still describes the old mapping:

# Type check: `just typecheck` or `uv run pyright`
# Type check (supplemental): `just typecheck-ty` or `uv run ty check src/`

After this PR the correct mapping is:

# Type check: `just typecheck` or `uv run ty check src tests test-int`
# Type check (pyright): `just typecheck-pyright` or `uv run pyright`

---

3. Redundant cast in repository.py:282

if isinstance(entity_data, dict):
    update_data = cast(dict[str, Any], entity_data)   # ← isinstance already narrows this
    for key, value in update_data.items():

Inside the isinstance(entity_data, dict) branch, entity_data is already narrowed to dict[str, Any] (since the signature is dict[str, Any] | T). The cast is harmless but unnecessary noise.

---

Positive observations

Real bug fix — api_client.py: The restructuring that catches httpx.HTTPStatusError explicitly before httpx.HTTPError is the canonical httpx pattern and fixes a real correctness issue — the original hasattr(e, "response") guard was fragile for non-HTTPStatusError subclasses.

Real bug fix — resource_router.py:329-330: Adding the 404 guard before search_service.index_entity(updated_entity) prevents a silent AttributeError if the entity wasn't found. This is a genuine correctness improvement, not just a type appeasement.

EntityFrontmatter.model_validator: Using mode="before" to collect raw YAML keys into metadata is the correct Pydantic v2 pattern here. The tightened property return types (tags → list[str], title → str, permalink → Optional[str]) reflect reality more accurately.

Telemetry attrs in search_repository_base.py: Inlining the **metric_attrs dict spread as explicit kwargs is correct. ty is stricter about **dict call-site unpacking, and the verbosity is worth the explicitness.

Variable shadowing fixes: changes → change_summary in watch_service.py and context → context_item in prompts/utils.py both improve readability and were genuine shadowing bugs.

importers/utils.py: Using a separate parsed_timestamp variable instead of rebinding timestamp is cleaner for type narrowing and matches the project's "fail fast, readable" style.

---

Checklist

Category	Status	Notes
Coding conventions (CLAUDE.md)	✅
Python 3.12+ type annotations	✅
SQLAlchemy 2.0 best practices	✅
FastAPI/Typer conventions	✅
100-char line length	✅
No commented-out code	✅
Unit tests for changed code	✅	Existing tests updated with typed fakes/casts
100% test coverage	✅
CLAUDE.md updated	⚠️	Build commands section not updated (see issue #2)
MCP tool atomic/composable design	✅
No DB migrations needed	✅	No schema changes
Security / no hardcoded secrets	✅
Windows file path compatibility	✅
No emoji/unicode in console output	✅

---

CI Status

Tests are currently queued/in-progress — no failures observed yet.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 58dd6963bd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Preserve null frontmatter permalinks during entity creation

Coercing content_frontmatter["permalink"] through _coerce_to_string converts YAML null/empty permalink values into the literal string "None". In files with frontmatter like permalink: or permalink: null, resolve_permalink() now treats that as an explicit permalink and persists None (or None-1, etc.) instead of generating the canonical permalink from the file path, which changes identifiers and can create collisions across notes. This path previously preserved null semantics, so the coercion should be skipped for permalink values unless they are already non-empty strings.

Useful? React with 👍 / 👎.

[phernandez]

Fixed in a3e4bad. YAML null/empty frontmatter permalink values now keep their null semantics, so permalink resolution falls back to the canonical file path instead of persisting a literal None value.

[github-actions]

Claude finished @phernandez's task in 1m 24s —— View job

---

PR Review: chore(core): make ty the default typechecker

Tasks:

• Read CLAUDE.md for project context
• Check latest commits and diff since last review
• Verify previous issues were addressed
• Check CLAUDE.md was updated
• Post updated review

---

What's new since last review

New commit a3e4bad (fix(core): preserve empty frontmatter permalink semantics) addressed the Codex-flagged issue: YAML null/empty permalink: values now go through _frontmatter_permalink() which guards with isinstance(value, str) and value, returning None and falling back to canonical permalink generation instead of persisting the string "None". Correct fix.

---

Open issues

1. Non-exhaustive match in to_summary (src/basic_memory/api/v2/utils.py:100-149)

Still not addressed. The match item_type block handles ENTITY, OBSERVATION, and RELATION but has no case _: fallback. Python will silently return None if an unknown SearchItemType value is introduced, but the declared return type is EntitySummary | ObservationSummary | RelationSummary. The related list on line 166 collects these without a isinstance guard (unlike observations on line 161-165), so a None in related_results would surface only at Pydantic validation time, not at the source.

Recommend restoring:

case _:
    raise ValueError(f"Unexpected type: {item_type}")  # pragma: no cover

after the SearchItemType.RELATION case.

Fix this →

---

2. CLAUDE.md not updated to reflect new typecheck commands

The justfile now has:

• typecheck → uv run ty check src tests test-int
• typecheck-pyright → uv run pyright
• typecheck-ty → alias for just typecheck (backwards-compat)

But CLAUDE.md (lines 33–34) still reads:

- Type check: `just typecheck` or `uv run pyright`
- Type check (supplemental): `just typecheck-ty` or `uv run ty check src/`

Should be updated to:

- Type check: `just typecheck` or `uv run ty check src tests test-int`
- Type check (pyright): `just typecheck-pyright` or `uv run pyright`

Fix this →

---

3. Minor: redundant cast in repository.py:282 (previously noted, still present)

if isinstance(entity_data, dict):
    update_data = cast(dict[str, Any], entity_data)   # isinstance already narrows this

The isinstance check already narrows the type — the cast is dead weight. Low priority.

---

Checklist

Category	Status	Notes
Coding conventions (CLAUDE.md)	✅
Python 3.12+ type annotations	✅
SQLAlchemy 2.0 best practices	✅
FastAPI/Typer conventions	✅
100-char line length	✅
No commented-out code	✅
Unit tests for changed code	✅	Typed fakes / casts throughout
100% test coverage	✅
CLAUDE.md updated	⚠️	Build commands section not updated (issue #2 above)
MCP tool atomic/composable design	✅
No DB migrations needed	✅	No schema changes
Security / no hardcoded secrets	✅
Windows file path compatibility	✅
No emoji/unicode in console output	✅

---

Summary

The a3e4bad commit correctly fixes the permalink coercion bug. Two items from the previous review remain open: the non-exhaustive match in to_summary (a latent correctness risk) and the stale CLAUDE.md typecheck docs (a DX issue). Both are small, targeted fixes.