fix(P1+P2): canonicalize project names at every trust boundary
Three findings from codex's review of the previous P1+P2 fix. The earlier commit (f2372ef) only fixed alias resolution at the context builder. Codex correctly pointed out that the same fragmentation applies at every other place a project name crosses a boundary — project_state writes/reads, interaction capture/listing/filtering, memory create/queries, and reinforcement's downstream queries. Plus a real bug in the interaction `since` filter where the storage format and the documented ISO format don't compare cleanly. The fix is one helper used at every boundary instead of duplicating the resolution inline. New helper: src/atocore/projects/registry.py::resolve_project_name --------------------------------------------------------------- - Single canonicalization boundary for project names - Returns the canonical project_id when the input matches any registered id or alias - Returns the input unchanged for empty/None and for unregistered names (preserves backwards compat with hand-curated state that predates the registry) - Documented as the contract that every read/write at the trust boundary should pass through P1 — Trusted Project State endpoints ------------------------------------ src/atocore/context/project_state.py: set_state, get_state, and invalidate_state now all canonicalize project_name through resolve_project_name BEFORE looking up or creating the project row. Before this fix: - POST /project/state with project="p05" called ensure_project("p05") which created a separate row in the projects table - The state row was attached to that alias project_id - Later context builds canonicalized "p05" -> "p05-interferometer" via the builder fix fromf2372efand never found the state - Result: trusted state silently fragmented across alias rows After this fix: - The alias is resolved to the canonical id at every entry point - Two captures (one via "p05", one via "p05-interferometer") write to the same row - get_state via either alias or the canonical id finds the same row Fixes the highest-priority gap codex flagged because Trusted Project State is supposed to be the most dependable layer in the AtoCore trust hierarchy. P2.a — Interaction capture project canonicalization ---------------------------------------------------- src/atocore/interactions/service.py: record_interaction now canonicalizes project before storing, so interaction.project is always the canonical id regardless of what the client passed. Downstream effects: - reinforce_from_interaction queries memories by interaction.project -> previously missed memories stored under canonical id -> now consistent because interaction.project IS the canonical id - the extractor stamps candidates with interaction.project -> previously created candidates in alias buckets -> now creates candidates in the canonical bucket - list_interactions(project=alias) was already broken, now fixed by canonicalizing the filter input on the read side too Memory service applied the same fix: - src/atocore/memory/service.py: create_memory and get_memories both canonicalize project through resolve_project_name - This keeps stored memory.project consistent with the reinforcement query path P2.b — Interaction `since` filter format normalization ------------------------------------------------------ src/atocore/interactions/service.py: new _normalize_since helper. The bug: - created_at is stored as 'YYYY-MM-DD HH:MM:SS' (no timezone, UTC by convention) so it sorts lexically and compares cleanly with the SQLite CURRENT_TIMESTAMP default - The `since` parameter was documented as ISO 8601 but compared as a raw string against the storage format - The lexically-greater 'T' separator means an ISO timestamp like '2026-04-07T12:00:00Z' is GREATER than the storage form '2026-04-07 12:00:00' for the same instant - Result: a client passing ISO `since` got an empty result for any row from the same day, even though those rows existed and were technically "after" the cutoff in real-world time The fix: - _normalize_since accepts ISO 8601 with T, optional Z suffix, optional fractional seconds, optional +HH:MM offsets - Uses datetime.fromisoformat for parsing (Python 3.11+) - Converts to UTC and reformats as the storage format before the SQL comparison - The bare storage format still works (backwards compat path is a regex match that returns the input unchanged) - Unparseable input is returned as-is so the comparison degrades gracefully (rows just don't match) instead of raising and breaking the listing endpoint builder.py refactor ------------------- The previous P1 fix had inline canonicalization. Now it uses the shared helper for consistency: - import changed from get_registered_project to resolve_project_name - the inline lookup is replaced with a single helper call - the comment block now points at representation-authority.md for the canonicalization contract New shared test fixture: tests/conftest.py::project_registry ------------------------------------------------------------ - Standardizes the registry-setup pattern that was duplicated across test_context_builder.py, test_project_state.py, test_interactions.py, and test_reinforcement.py - Returns a callable that takes (project_id, [aliases]) tuples and writes them into a temp registry file with the env var pointed at it and config.settings reloaded - Used by all 12 new regression tests in this commit Tests (12 new, all green on first run) -------------------------------------- test_project_state.py: - test_set_state_canonicalizes_alias: write via alias, read via every alias and the canonical id, verify same row id - test_get_state_canonicalizes_alias_after_canonical_write - test_invalidate_state_canonicalizes_alias - test_unregistered_project_state_still_works (backwards compat) test_interactions.py: - test_record_interaction_canonicalizes_project - test_list_interactions_canonicalizes_project_filter - test_list_interactions_since_accepts_iso_with_t_separator - test_list_interactions_since_accepts_z_suffix - test_list_interactions_since_accepts_offset - test_list_interactions_since_storage_format_still_works test_reinforcement.py: - test_reinforcement_works_when_capture_uses_alias (end-to-end: capture under alias, seed memory under canonical, verify reinforcement matches) - test_get_memories_filter_by_alias Full suite: 174 passing (was 162), 1 warning. The +12 is the new regression tests, no existing tests regressed. What's still NOT canonicalized (and why) ---------------------------------------- - _rank_chunks's secondary substring boost in builder.py — the retriever already does the right thing via its own _project_match_boost which calls get_registered_project. The redundant secondary boost still uses the raw hint but it's a multiplicative factor on top of correct retrieval, not a filter, so it can't drop relevant chunks. Tracked as a future cleanup but not a P1. - update_memory's project field (you can't change a memory's project after creation in the API anyway). - The retriever's project_hint parameter on direct /query calls — same reasoning as the builder boost, plus the retriever's own get_registered_project call already handles aliases there.
This commit is contained in:
@@ -14,7 +14,7 @@ import atocore.config as _config
|
||||
from atocore.context.project_state import format_project_state, get_state
|
||||
from atocore.memory.service import get_memories_for_context
|
||||
from atocore.observability.logger import get_logger
|
||||
from atocore.projects.registry import get_registered_project
|
||||
from atocore.projects.registry import resolve_project_name
|
||||
from atocore.retrieval.retriever import ChunkResult, retrieve
|
||||
|
||||
log = get_logger("context_builder")
|
||||
@@ -85,20 +85,15 @@ def build_context(
|
||||
max(0, int(budget * PROJECT_STATE_BUDGET_RATIO)),
|
||||
)
|
||||
|
||||
# Resolve the project hint through the registry so callers can pass
|
||||
# an alias (`p05`, `gigabit`) and still find trusted state stored
|
||||
# under the canonical project id (`p05-interferometer`,
|
||||
# `p04-gigabit`). The retriever already does this for the
|
||||
# project-match boost — the project_state lookup needs the same
|
||||
# courtesy. If the registry has no entry for the hint, fall back to
|
||||
# the raw hint so a hand-curated project_state entry that predates
|
||||
# the registry still works.
|
||||
canonical_project = project_hint
|
||||
if project_hint:
|
||||
registered = get_registered_project(project_hint)
|
||||
if registered is not None:
|
||||
canonical_project = registered.project_id
|
||||
|
||||
# Canonicalize the project hint through the registry so callers
|
||||
# can pass an alias (`p05`, `gigabit`) and still find trusted
|
||||
# state stored under the canonical project id. The same helper
|
||||
# is used everywhere a project name crosses a trust boundary
|
||||
# (project_state, memories, interactions). When the registry has
|
||||
# no entry the helper returns the input unchanged so hand-curated
|
||||
# state that predates the registry still works.
|
||||
canonical_project = resolve_project_name(project_hint) if project_hint else ""
|
||||
if canonical_project:
|
||||
state_entries = get_state(canonical_project)
|
||||
if state_entries:
|
||||
project_state_text = format_project_state(state_entries)
|
||||
|
||||
@@ -18,6 +18,7 @@ from datetime import datetime, timezone
|
||||
|
||||
from atocore.models.database import get_connection
|
||||
from atocore.observability.logger import get_logger
|
||||
from atocore.projects.registry import resolve_project_name
|
||||
|
||||
log = get_logger("project_state")
|
||||
|
||||
@@ -101,11 +102,19 @@ def set_state(
|
||||
source: str = "",
|
||||
confidence: float = 1.0,
|
||||
) -> ProjectStateEntry:
|
||||
"""Set or update a project state entry. Upsert semantics."""
|
||||
"""Set or update a project state entry. Upsert semantics.
|
||||
|
||||
The ``project_name`` is canonicalized through the registry so a
|
||||
caller passing an alias (``p05``) ends up writing into the same
|
||||
row as the canonical id (``p05-interferometer``). Without this
|
||||
step, alias and canonical names would create two parallel
|
||||
project rows and fragmented state.
|
||||
"""
|
||||
if category not in CATEGORIES:
|
||||
raise ValueError(f"Invalid category '{category}'. Must be one of: {CATEGORIES}")
|
||||
_validate_confidence(confidence)
|
||||
|
||||
project_name = resolve_project_name(project_name)
|
||||
project_id = ensure_project(project_name)
|
||||
entry_id = str(uuid.uuid4())
|
||||
now = datetime.now(timezone.utc).isoformat()
|
||||
@@ -153,7 +162,12 @@ def get_state(
|
||||
category: str | None = None,
|
||||
active_only: bool = True,
|
||||
) -> list[ProjectStateEntry]:
|
||||
"""Get project state entries, optionally filtered by category."""
|
||||
"""Get project state entries, optionally filtered by category.
|
||||
|
||||
The lookup is canonicalized through the registry so an alias hint
|
||||
finds the same rows as the canonical id.
|
||||
"""
|
||||
project_name = resolve_project_name(project_name)
|
||||
with get_connection() as conn:
|
||||
project = conn.execute(
|
||||
"SELECT id FROM projects WHERE lower(name) = lower(?)", (project_name,)
|
||||
@@ -191,7 +205,12 @@ def get_state(
|
||||
|
||||
|
||||
def invalidate_state(project_name: str, category: str, key: str) -> bool:
|
||||
"""Mark a project state entry as superseded."""
|
||||
"""Mark a project state entry as superseded.
|
||||
|
||||
The lookup is canonicalized through the registry so an alias is
|
||||
treated as the canonical project for the invalidation lookup.
|
||||
"""
|
||||
project_name = resolve_project_name(project_name)
|
||||
with get_connection() as conn:
|
||||
project = conn.execute(
|
||||
"SELECT id FROM projects WHERE lower(name) = lower(?)", (project_name,)
|
||||
|
||||
@@ -18,15 +18,24 @@ violating the AtoCore trust hierarchy.
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import re
|
||||
import uuid
|
||||
from dataclasses import dataclass, field
|
||||
from datetime import datetime, timezone
|
||||
|
||||
from atocore.models.database import get_connection
|
||||
from atocore.observability.logger import get_logger
|
||||
from atocore.projects.registry import resolve_project_name
|
||||
|
||||
log = get_logger("interactions")
|
||||
|
||||
# Stored timestamps use 'YYYY-MM-DD HH:MM:SS' (no timezone offset, UTC by
|
||||
# convention) so they sort lexically and compare cleanly with the SQLite
|
||||
# CURRENT_TIMESTAMP default. The since filter accepts ISO 8601 strings
|
||||
# (with 'T', optional 'Z' or +offset, optional fractional seconds) and
|
||||
# normalizes them to the storage format before the SQL comparison.
|
||||
_STORAGE_TIMESTAMP_FORMAT = "%Y-%m-%d %H:%M:%S"
|
||||
|
||||
|
||||
@dataclass
|
||||
class Interaction:
|
||||
@@ -72,6 +81,13 @@ def record_interaction(
|
||||
if not prompt or not prompt.strip():
|
||||
raise ValueError("Interaction prompt must be non-empty")
|
||||
|
||||
# Canonicalize the project through the registry so an alias and
|
||||
# the canonical id store under the same bucket. Without this,
|
||||
# reinforcement and extraction (which both query by raw
|
||||
# interaction.project) would silently miss memories and create
|
||||
# candidates in the wrong project.
|
||||
project = resolve_project_name(project)
|
||||
|
||||
interaction_id = str(uuid.uuid4())
|
||||
# Store created_at explicitly so the same string lives in both the DB
|
||||
# column and the returned dataclass. SQLite's CURRENT_TIMESTAMP uses
|
||||
@@ -159,9 +175,14 @@ def list_interactions(
|
||||
) -> list[Interaction]:
|
||||
"""List captured interactions, optionally filtered.
|
||||
|
||||
``since`` is an ISO timestamp string; only interactions created at or
|
||||
after that time are returned. ``limit`` is hard-capped at 500 to keep
|
||||
casual API listings cheap.
|
||||
``since`` accepts an ISO 8601 timestamp string (with ``T``, an
|
||||
optional ``Z`` or numeric offset, optional fractional seconds).
|
||||
The value is normalized to the storage format (UTC,
|
||||
``YYYY-MM-DD HH:MM:SS``) before the SQL comparison so external
|
||||
callers can pass any of the common ISO shapes without filter
|
||||
drift. ``project`` is canonicalized through the registry so an
|
||||
alias finds rows stored under the canonical project id.
|
||||
``limit`` is hard-capped at 500 to keep casual API listings cheap.
|
||||
"""
|
||||
if limit <= 0:
|
||||
return []
|
||||
@@ -172,7 +193,7 @@ def list_interactions(
|
||||
|
||||
if project:
|
||||
query += " AND project = ?"
|
||||
params.append(project)
|
||||
params.append(resolve_project_name(project))
|
||||
if session_id:
|
||||
query += " AND session_id = ?"
|
||||
params.append(session_id)
|
||||
@@ -181,7 +202,7 @@ def list_interactions(
|
||||
params.append(client)
|
||||
if since:
|
||||
query += " AND created_at >= ?"
|
||||
params.append(since)
|
||||
params.append(_normalize_since(since))
|
||||
|
||||
query += " ORDER BY created_at DESC LIMIT ?"
|
||||
params.append(limit)
|
||||
@@ -243,3 +264,41 @@ def _safe_json_dict(raw: str | None) -> dict:
|
||||
if not isinstance(value, dict):
|
||||
return {}
|
||||
return value
|
||||
|
||||
|
||||
def _normalize_since(since: str) -> str:
|
||||
"""Normalize an ISO 8601 ``since`` filter to the storage format.
|
||||
|
||||
Stored ``created_at`` values are ``YYYY-MM-DD HH:MM:SS`` (no
|
||||
timezone, UTC by convention). External callers naturally pass
|
||||
ISO 8601 with ``T`` separator, optional ``Z`` suffix, optional
|
||||
fractional seconds, and optional ``+HH:MM`` offsets. A naive
|
||||
string comparison between the two formats fails on the same
|
||||
day because the lexically-greater ``T`` makes any ISO value
|
||||
sort after any space-separated value.
|
||||
|
||||
This helper accepts the common ISO shapes plus the bare
|
||||
storage format and returns the storage format. On a parse
|
||||
failure it returns the input unchanged so the SQL comparison
|
||||
fails open (no rows match) instead of raising and breaking
|
||||
the listing endpoint.
|
||||
"""
|
||||
if not since:
|
||||
return since
|
||||
candidate = since.strip()
|
||||
# Python's fromisoformat understands trailing 'Z' from 3.11+ but
|
||||
# we replace it explicitly for safety against earlier shapes.
|
||||
if candidate.endswith("Z"):
|
||||
candidate = candidate[:-1] + "+00:00"
|
||||
try:
|
||||
dt = datetime.fromisoformat(candidate)
|
||||
except ValueError:
|
||||
# Already in storage format, or unparseable: best-effort
|
||||
# match the storage format with a regex; if that fails too,
|
||||
# return the raw input.
|
||||
if re.fullmatch(r"\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}", since):
|
||||
return since
|
||||
return since
|
||||
if dt.tzinfo is not None:
|
||||
dt = dt.astimezone(timezone.utc).replace(tzinfo=None)
|
||||
return dt.strftime(_STORAGE_TIMESTAMP_FORMAT)
|
||||
|
||||
@@ -29,6 +29,7 @@ from datetime import datetime, timezone
|
||||
|
||||
from atocore.models.database import get_connection
|
||||
from atocore.observability.logger import get_logger
|
||||
from atocore.projects.registry import resolve_project_name
|
||||
|
||||
log = get_logger("memory")
|
||||
|
||||
@@ -84,6 +85,13 @@ def create_memory(
|
||||
raise ValueError(f"Invalid status '{status}'. Must be one of: {MEMORY_STATUSES}")
|
||||
_validate_confidence(confidence)
|
||||
|
||||
# Canonicalize the project through the registry so an alias and
|
||||
# the canonical id store under the same bucket. This keeps
|
||||
# reinforcement queries (which use the interaction's project) and
|
||||
# context retrieval (which uses the registry-canonicalized hint)
|
||||
# consistent with how memories are created.
|
||||
project = resolve_project_name(project)
|
||||
|
||||
memory_id = str(uuid.uuid4())
|
||||
now = datetime.now(timezone.utc).isoformat()
|
||||
|
||||
@@ -162,8 +170,13 @@ def get_memories(
|
||||
query += " AND memory_type = ?"
|
||||
params.append(memory_type)
|
||||
if project is not None:
|
||||
# Canonicalize on the read side so a caller passing an alias
|
||||
# finds rows that were stored under the canonical id (and
|
||||
# vice versa). resolve_project_name returns the input
|
||||
# unchanged for unregistered names so empty-string queries
|
||||
# for "no project scope" still work.
|
||||
query += " AND project = ?"
|
||||
params.append(project)
|
||||
params.append(resolve_project_name(project))
|
||||
if status is not None:
|
||||
query += " AND status = ?"
|
||||
params.append(status)
|
||||
|
||||
@@ -254,6 +254,30 @@ def get_registered_project(project_name: str) -> RegisteredProject | None:
|
||||
return None
|
||||
|
||||
|
||||
def resolve_project_name(name: str | None) -> str:
|
||||
"""Canonicalize a project name through the registry.
|
||||
|
||||
Returns the canonical ``project_id`` if the input matches any
|
||||
registered project's id or alias. Returns the input unchanged
|
||||
when it's empty or not in the registry — the second case keeps
|
||||
backwards compatibility with hand-curated state, memories, and
|
||||
interactions that predate the registry, or for projects that
|
||||
are intentionally not registered.
|
||||
|
||||
This helper is the single canonicalization boundary for project
|
||||
names across the trust hierarchy. Every read/write that takes a
|
||||
project name should pass it through ``resolve_project_name``
|
||||
before storing or querying. The contract is documented in
|
||||
``docs/architecture/representation-authority.md``.
|
||||
"""
|
||||
if not name:
|
||||
return name or ""
|
||||
project = get_registered_project(name)
|
||||
if project is not None:
|
||||
return project.project_id
|
||||
return name
|
||||
|
||||
|
||||
def refresh_registered_project(project_name: str, purge_deleted: bool = False) -> dict:
|
||||
"""Ingest all configured source roots for a registered project.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user