CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 14s
CI & Build / TypeScript typecheck (push) Successful in 48s
CI & Build / integration (push) Successful in 38s
CI & Build / Python tests (push) Failing after 57s
CI & Build / Build & push image (push) Skipped
Step 1 found the reason the standard names never held, and it is sharper than "prose doesn't fire": the list WAS real and it WAS seeded — but only on the inception path, for a project with zero Systems. Ad-hoc create_system never consulted it, which is how Forge minted "CI and Release" and Portal minted "CI & release" after the constant already existed. This wires the vocabulary to the moment that mints a name. - services/systems.assess_system_name: the local duplicate gate AND the catalog lookup, in ONE service function both doors call. The gate lived only in the MCP tool, which is exactly how the web UI shipped without a check the agent surface enforced (#2482). REST now answers 409 with the System that already covers the area. - An `exact` catalog hit is APPLIED (mechanical — the names differ only in spelling). An `overlap` is only OFFERED, on both doors: applying a judgment call silently is how a cross-project rule surfaces in the wrong project. - canonical_systems.best_overlap is the ONE scorer behind the create-time offer and the review sweep, so the two surfaces can never name different areas for one System. It also takes the catalog the caller already holds, so the review is not an N+1. UI (folded in from step 1 — rule 27, that step shipped with no human surface): - SystemsSection: a Shared area picker on create and edit, the area on each card, and a collapsed review of proposals that appears only when there is something to decide. `exact` and `overlap` never share a style — one is mechanical, the other is the reviewer's judgment, and presenting them alike is how a wrong mapping gets waved through. - Settings → Admin → Areas: the catalog itself, showing each entry's slug, because the slug is what decides whether two names are the same area and a rename moves it. - A picker rather than a live matcher: reproducing the slug rule in TypeScript would give this feature two matchers to keep in step — the exact drift the catalog exists to end. The server stays authoritative. tests/helpers.fake_system gains canonical_id=None: an unnamed attribute is an auto-MagicMock and therefore truthy, which is the trap that helper exists for (note 2109) and a nullable FK walks straight into it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
291 lines
11 KiB
Python
291 lines
11 KiB
Python
"""The global canonical area vocabulary, and the mapping from a project's
|
|
Systems onto it (milestone 307 step 1, decision note 3026).
|
|
|
|
A `System` is per-project. Nothing outside a project can reference one, so a
|
|
rule that spans projects has no way to say "this is about CI" without chaining
|
|
itself to one project's row. `CanonicalSystem` is that join key, and it is
|
|
GLOBAL — no `user_id`, so a shared project inherits the vocabulary instead of
|
|
re-earning it.
|
|
|
|
Two rules govern everything here:
|
|
|
|
- **Associate, never rewrite.** Mapping a System sets `systems.canonical_id`
|
|
and nothing else. The local name stays whatever the project calls the area,
|
|
and `record_systems` is never touched — no record's tags move.
|
|
- **Propose, never decide.** An exact slug hit is mechanical and maps on
|
|
request; anything short of that is a PROPOSAL a human confirms. "CI &
|
|
Release" vs "CI & runners" is a judgment call, and the cost of guessing it
|
|
wrong silently is a rule surfacing in the wrong project.
|
|
|
|
Reads are open to any authenticated caller (the catalog is shared vocabulary,
|
|
not user data). Writes to the catalog itself are admin-only: a global table
|
|
that anyone can extend is how a shared vocabulary stops being shared.
|
|
"""
|
|
import logging
|
|
import re
|
|
from datetime import datetime, timezone
|
|
|
|
from sqlalchemy import select
|
|
|
|
from scribe.models import async_session
|
|
from scribe.models.canonical_system import CanonicalSystem
|
|
from scribe.models.system import System
|
|
from scribe.models.user import User
|
|
from scribe.services import access
|
|
|
|
logger = logging.getLogger(__name__)
|
|
|
|
# Tokens that carry no meaning for matching — "&" becomes "and" before the
|
|
# split, so it would otherwise dominate the overlap score of every pair.
|
|
_NOISE_TOKENS = frozenset({"and", "the", "a", "of"})
|
|
|
|
_NON_ALNUM = re.compile(r"[^a-z0-9]+")
|
|
|
|
|
|
def canonical_slug(name: str) -> str:
|
|
"""The match key for an area name — NOT a display value.
|
|
|
|
Folds exactly the spelling differences that produced three names for one
|
|
area on the author's instance: `CI & Release`, `CI and Release` and
|
|
`CI & release` all slug to `ci-and-release`, so they map mechanically.
|
|
A real difference survives: `CI & runners` slugs to `ci-and-runners` and
|
|
goes through the proposal path where a human decides.
|
|
"""
|
|
lowered = name.strip().lower().replace("&", " and ")
|
|
return "-".join(_NON_ALNUM.sub(" ", lowered).split())
|
|
|
|
|
|
def _tokens(slug: str) -> frozenset[str]:
|
|
return frozenset(slug.split("-")) - _NOISE_TOKENS
|
|
|
|
|
|
async def _is_admin(user_id: int) -> bool:
|
|
async with async_session() as session:
|
|
role = await session.scalar(select(User.role).where(User.id == user_id))
|
|
return role == "admin"
|
|
|
|
|
|
async def list_canonical_systems() -> list[CanonicalSystem]:
|
|
"""The whole catalog, in display order. Global — no ownership filter."""
|
|
async with async_session() as session:
|
|
result = await session.execute(
|
|
select(CanonicalSystem)
|
|
.where(CanonicalSystem.deleted_at.is_(None))
|
|
.order_by(CanonicalSystem.order_index.asc(), CanonicalSystem.name.asc())
|
|
)
|
|
return list(result.scalars().all())
|
|
|
|
|
|
async def get_canonical_system(canonical_id: int) -> CanonicalSystem | None:
|
|
async with async_session() as session:
|
|
entry = await session.get(CanonicalSystem, canonical_id)
|
|
return entry if entry is not None and entry.deleted_at is None else None
|
|
|
|
|
|
async def find_by_name(name: str) -> CanonicalSystem | None:
|
|
"""The exact-slug lookup — the mechanical half of matching."""
|
|
slug = canonical_slug(name)
|
|
if not slug:
|
|
return None
|
|
async with async_session() as session:
|
|
return await session.scalar(
|
|
select(CanonicalSystem).where(
|
|
CanonicalSystem.slug == slug,
|
|
CanonicalSystem.deleted_at.is_(None),
|
|
)
|
|
)
|
|
|
|
|
|
def _overlap(local: frozenset[str], other: frozenset[str]) -> float:
|
|
return len(local & other) / max(len(local | other), 1)
|
|
|
|
|
|
async def best_overlap(name: str, catalog: list | None = None) -> dict | None:
|
|
"""The closest catalog entry that shares a meaningful word, or None.
|
|
|
|
The ONE scorer behind both offers: the create-time suggestion and the
|
|
review surface. Two scorers would eventually disagree about which area a
|
|
name resembles, and the operator would be asked one question at create
|
|
time and a different one at review.
|
|
|
|
The threshold is any shared meaningful word, deliberately generous: a
|
|
wrong offer costs one dismissal, a missing one costs a mapping nobody
|
|
thinks to make again. Nothing here ever applies — `overlap` is always an
|
|
offer (see propose_mappings).
|
|
"""
|
|
slug = canonical_slug(name)
|
|
if not slug:
|
|
return None
|
|
local = _tokens(slug)
|
|
if not local:
|
|
return None
|
|
# A caller already holding the catalog passes it: this runs once per
|
|
# unmapped System in the review sweep, and re-reading the table each time
|
|
# would make an N+1 out of a report.
|
|
if catalog is None:
|
|
catalog = await list_canonical_systems()
|
|
best, best_score = None, 0.0
|
|
for entry in catalog:
|
|
score = _overlap(local, _tokens(entry.slug))
|
|
if score > best_score:
|
|
best, best_score = entry, score
|
|
if best is None or best_score <= 0:
|
|
return None
|
|
return {
|
|
"id": best.id, "name": best.name,
|
|
"basis": "overlap", "score": round(best_score, 3),
|
|
}
|
|
|
|
|
|
async def create_canonical_system(
|
|
user_id: int, name: str, description: str | None = None,
|
|
) -> CanonicalSystem | dict | None:
|
|
"""Add an area to the global catalog. Admin only.
|
|
|
|
Duplicate-gated on the SLUG, not the raw name, so "CI and Release" cannot
|
|
be added alongside "CI & Release" — that is the drift this table exists to
|
|
end. Returns the existing entry's id instead of creating a second one.
|
|
"""
|
|
if not await _is_admin(user_id):
|
|
return None
|
|
slug = canonical_slug(name)
|
|
if not slug:
|
|
return None
|
|
existing = await find_by_name(name)
|
|
if existing is not None:
|
|
return {
|
|
"duplicate": True,
|
|
"existing_id": existing.id,
|
|
"message": (
|
|
f"'{existing.name}' (#{existing.id}) already covers this area — "
|
|
f"both names reduce to '{slug}'. Map Systems to it, or "
|
|
"update_canonical_system if the charter needs revising."
|
|
),
|
|
}
|
|
async with async_session() as session:
|
|
highest = await session.scalar(
|
|
select(CanonicalSystem.order_index)
|
|
.order_by(CanonicalSystem.order_index.desc())
|
|
.limit(1)
|
|
)
|
|
entry = CanonicalSystem(
|
|
name=" ".join(name.split()),
|
|
slug=slug,
|
|
description=description,
|
|
order_index=(highest or 0) + 1,
|
|
)
|
|
session.add(entry)
|
|
await session.commit()
|
|
await session.refresh(entry)
|
|
return entry
|
|
|
|
|
|
async def update_canonical_system(
|
|
user_id: int, canonical_id: int, **fields: object,
|
|
) -> CanonicalSystem | None:
|
|
"""Rename or re-charter a catalog entry. Admin only.
|
|
|
|
A rename recomputes the slug — the display name and the match key must not
|
|
be allowed to disagree, or the exact-match path silently stops finding it.
|
|
"""
|
|
if not await _is_admin(user_id):
|
|
return None
|
|
allowed = {"name", "description", "order_index"}
|
|
async with async_session() as session:
|
|
entry = await session.get(CanonicalSystem, canonical_id)
|
|
if entry is None or entry.deleted_at is not None:
|
|
return None
|
|
for key, value in fields.items():
|
|
if key in allowed and value is not None:
|
|
setattr(entry, key, value)
|
|
if "name" in fields and fields["name"]:
|
|
entry.name = " ".join(str(fields["name"]).split())
|
|
entry.slug = canonical_slug(entry.name)
|
|
entry.updated_at = datetime.now(timezone.utc)
|
|
await session.commit()
|
|
await session.refresh(entry)
|
|
return entry
|
|
|
|
|
|
async def set_system_canonical(
|
|
user_id: int, system_id: int, canonical_id: int | None,
|
|
) -> System | None:
|
|
"""Map (or unmap) one project System onto a catalog entry.
|
|
|
|
Authorised by the PROJECT, not the catalog: mapping changes the project's
|
|
row, so project write access is the right gate (rule 78 — never a bare
|
|
owner filter). Passing None clears the mapping.
|
|
|
|
Touches `canonical_id` and nothing else — the System's own name, charter
|
|
and record associations are left exactly as they are.
|
|
"""
|
|
if canonical_id is not None and await get_canonical_system(canonical_id) is None:
|
|
return None
|
|
async with async_session() as session:
|
|
system = await session.get(System, system_id)
|
|
if system is None or system.deleted_at is not None:
|
|
return None
|
|
if not await access.can_write_project(user_id, system.project_id):
|
|
return None
|
|
system.canonical_id = canonical_id
|
|
system.updated_at = datetime.now(timezone.utc)
|
|
await session.commit()
|
|
await session.refresh(system)
|
|
return system
|
|
|
|
|
|
async def propose_mappings(user_id: int, project_id: int) -> list[dict]:
|
|
"""Suggest a catalog entry for each of a project's UNMAPPED Systems.
|
|
|
|
Returns proposals, never applied changes — `set_system_canonical` is the
|
|
only thing that writes. Each carries a `basis` so the reviewer knows what
|
|
they are approving:
|
|
|
|
- `exact` — the two names reduce to the same slug. Mechanical.
|
|
- `overlap` — they share a meaningful word ("CI & runners" / "CI &
|
|
Release"). A judgment call, and the reason this is a proposal at all.
|
|
|
|
A System with no plausible match simply gets no proposal: unmapped is a
|
|
perfectly good resting state, so silence here is an answer, not a gap.
|
|
"""
|
|
if not await access.can_read_project(user_id, project_id):
|
|
return []
|
|
catalog = await list_canonical_systems()
|
|
if not catalog:
|
|
return []
|
|
async with async_session() as session:
|
|
result = await session.execute(
|
|
select(System).where(
|
|
System.project_id == project_id,
|
|
System.canonical_id.is_(None),
|
|
System.deleted_at.is_(None),
|
|
).order_by(System.order_index.asc(), System.created_at.asc())
|
|
)
|
|
systems = list(result.scalars().all())
|
|
|
|
by_slug = {entry.slug: entry for entry in catalog}
|
|
proposals: list[dict] = []
|
|
for system in systems:
|
|
slug = canonical_slug(system.name)
|
|
if not slug:
|
|
continue
|
|
exact = by_slug.get(slug)
|
|
if exact is not None:
|
|
match = {"id": exact.id, "name": exact.name, "basis": "exact", "score": 1.0}
|
|
else:
|
|
# Same scorer the create-time offer uses, so the two surfaces can
|
|
# never name different areas for one System.
|
|
match = await best_overlap(system.name, catalog)
|
|
if match is None:
|
|
continue
|
|
proposals.append({
|
|
"system_id": system.id,
|
|
"system_name": system.name,
|
|
"canonical_id": match["id"],
|
|
"canonical_name": match["name"],
|
|
"basis": match["basis"],
|
|
"score": match["score"],
|
|
})
|
|
proposals.sort(key=lambda p: (-p["score"], p["system_name"]))
|
|
return proposals
|