Refactor repositories to support caller-owned AsyncSession flows

[phernandez]

Summary

Basic Memory's repositories still own their own SQLAlchemy sessions for most high-level methods. That makes it awkward to orchestrate one logical write transaction across multiple repositories.

This is showing up clearly in Basic Memory Cloud's accepted-write path for note materialization, where the service needs one shared tenant DB transaction to:

• resolve Project
• load or create Entity
• load or upsert NoteContent
• flush related ORM changes together
• commit once

Because the current repository methods open their own sessions, cloud has to keep several transaction-scoped query helpers local in the service layer instead of reusing the repository classes.

Current pain

Examples:

• ProjectRepository.get_by_external_id() opens its own session
• EntityRepository.get_by_external_id() opens its own session
• EntityRepository.get_by_file_path() opens its own session
• NoteContentRepository.get_by_entity_id() opens its own session
• NoteContentRepository.upsert() opens its own session

That makes it hard to compose these operations safely inside an existing caller-owned transaction.

Proposed direction

Add transaction-friendly repository methods that accept an existing AsyncSession.

For example:

• ProjectRepository.get_by_external_id(session, external_id)
• EntityRepository.get_by_external_id(session, external_id, *, load_relations=True)
• EntityRepository.get_by_file_path(session, file_path, *, load_relations=True)
• NoteContentRepository.get_by_entity_id(session, entity_id)
• NoteContentRepository.upsert(..., session=...) or a dedicated upsert_accepted_state(session, ...)

The service layer should own transaction boundaries. Repositories should provide query/persistence helpers that can either:

1. work with a caller-owned session, or
2. provide a convenience wrapper that opens a session when the caller does not care.

Why this matters

This lets orchestration-heavy flows stay simple and correct:

• accepted note writes in cloud
• directory deletes / other multi-step content mutations
• any future flow that needs multiple repositories to participate in one commit

It also reduces duplicated query helpers in cloud code.

Scope

Start small. We do not need to redesign every repository at once.

Suggested first pass:

• ProjectRepository.get_by_external_id(session, ...)
• EntityRepository.get_by_external_id(session, ...)
• EntityRepository.get_by_file_path(session, ...)
• NoteContentRepository.get_by_entity_id(session, ...)
• one note-content upsert/update method that can participate in an existing session

Then update callers to use the shared-session variants where they are already orchestrating a transaction.

Out of scope

• changing user-facing API behavior
• changing note materialization semantics
• broad repository cleanup unrelated to transaction ownership

Acceptance criteria

• caller-owned transaction flows can use repository methods without opening nested sessions
• existing convenience behavior can remain for simple call sites if useful
• cloud can delete its local transaction-scoped lookup/upsert helpers once the new repository API is available

[claude]

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

---

Triage Analysis

• Gather context on issue
• Explore repository code to understand current implementation
• Classify issue type, priority, and complexity
• Check for duplicate issues (none found)
• Add labels (enhancement, cloud)
• Provide triage summary

---

Issue Triage: #750 — Refactor repositories to support caller-owned AsyncSession flows

Type: Enhancement (architectural improvement)
Priority: Medium-High — not blocking existing local functionality, but actively impeding cloud service layer correctness
Component: Database/Repository layer + Cloud
Complexity: Complex — multi-file refactor with test coverage required

---

Code Analysis

I reviewed the current repository implementations to confirm the pain described in the issue.

Current session ownership patterns:

Method	Session ownership
ProjectRepository.get_by_external_id()	Opens its own session via find_one() → execute_query()
EntityRepository.get_by_external_id()	Opens its own session via _find_one_by_query() → execute_query()
EntityRepository.get_by_file_path()	Same as above
NoteContentRepository.get_by_entity_id()	Delegates to find_by_id() which calls scoped_session
NoteContentRepository.upsert()	Opens its own session (async with db.scoped_session(...))

The pattern already exists for low-level methods: select_by_id(session, ...), select_by_ids(session, ...), and get_by_file_paths(session, ...) already accept a caller-provided AsyncSession. The gap is that the higher-level lookup methods (by external_id, by file_path, etc.) don't offer this variant.

---

Assessment

No duplicates found. This is a new issue.

Why this is medium-high priority:
The cloud note materialization path needs a shared transaction across ProjectRepository, EntityRepository, and NoteContentRepository. Without caller-owned session support, the cloud service must duplicate query helpers locally — a code smell that will diverge from the tested repository code over time.

Why this is complex:

1. Need to add session-accepting overloads or separate methods on multiple repositories
2. Need to design the API carefully — either overloaded signatures (session: AsyncSession | None = None) or dedicated _with_session(session, ...) variants
3. Must update all relevant callers to use shared sessions where they are already within a transaction
4. Must maintain 100% test coverage (project requirement)
5. Existing convenience wrappers should remain working for simple call sites

Suggested API design (from the issue):

# Accept an AsyncSession, return the query result using that session
async def get_by_external_id(self, session: AsyncSession, external_id: str) -> Optional[Project]: ...

# OR: optional session parameter with fallback to self-managed session
async def get_by_external_id(self, external_id: str, *, session: AsyncSession | None = None) -> Optional[Project]: ...

The second pattern (optional session kwarg) is less disruptive to existing call sites.

Recommended approach:

• Start with the 5 methods listed in the issue's "Suggested first pass"
• Use optional session keyword argument — keeps backward compatibility and avoids combinatorial overloads
• Once repositories expose session-aware methods, cloud service can collapse its local lookup helpers
• Add integration tests to verify the multi-repository shared-transaction flow works end to end

---

Labels applied: enhancement, cloud