diff --git a/alembic/versions/0072_design_systems.py b/alembic/versions/0072_design_systems.py new file mode 100644 index 0000000..80e467c --- /dev/null +++ b/alembic/versions/0072_design_systems.py @@ -0,0 +1,138 @@ +"""design systems + tokens, and the project pointer + +Revision ID: 0072 +Revises: 0071 +Create Date: 2026-07-30 + +Makes the design system a first-class record instead of prose in a rulebook: a +named set of tokens with an optional parent, so a family system holds the house +style and an app system holds only what it changes. + +`design_systems.parent_id` is the whole model. It replaces both an `always_on` +flag (a family system is one with no parent) and a subscription join table (a +project points at ONE system, and the chain supplies the rest), which is less +schema than the rulebook shape it mirrors. + +Two deliberate choices worth stating here rather than leaving to be re-derived: + + - **`design_tokens.value_by_mode` is JSONB keyed by mode**, not `value_light` + + `value_dark` columns. In a child system an unset mode means "inherit"; in a + root it would mean "not mode-dependent", and as columns both are NULL and + indistinguishable. As a map, resolution is a dict merge at every level with + no special case for roots — and a third mode (high-contrast, print) is data + rather than a schema change. The cost is that a typo'd mode key is not + rejected by the database. Nothing filters tokens by value in SQL, so the + queryability the columns would have bought is for a query no caller makes. + (Named `value_by_mode` rather than `values`, which is reserved in SQL.) + - **`group_name` is free text, not a CHECK enum.** Groupings are each design + system's own vocabulary; a whitelist would bake one install's kit into the + schema. No CHECK is introduced anywhere in this migration. + +`parent_id` and `projects.design_system_id` are both ON DELETE SET NULL. Deleting +a family system must orphan its children into roots that still hold their own +overrides, not cascade away every app system that inherited from it; deleting a +system a project points at must unstyle that project, not delete it. + +Downgrade drops both tables and the column. Any design system defined this way +is lost — this is the migration that introduces the concept, so there is no +earlier representation to fall back to. +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects.postgresql import JSONB + + +revision = "0072" +down_revision = "0071" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.create_table( + "design_systems", + sa.Column("id", sa.BigInteger(), primary_key=True), + sa.Column( + "owner_user_id", sa.BigInteger(), + sa.ForeignKey("users.id", ondelete="CASCADE"), nullable=False, + ), + sa.Column("title", sa.Text(), nullable=False), + sa.Column("description", sa.Text(), nullable=True), + sa.Column( + "parent_id", sa.BigInteger(), + sa.ForeignKey("design_systems.id", ondelete="SET NULL"), nullable=True, + ), + sa.Column( + "created_at", sa.DateTime(timezone=True), nullable=False, + server_default=sa.text("now()"), + ), + sa.Column( + "updated_at", sa.DateTime(timezone=True), nullable=False, + server_default=sa.text("now()"), + ), + sa.Column("deleted_at", sa.DateTime(timezone=True), nullable=True), + sa.Column("deleted_batch_id", sa.Text(), nullable=True), + ) + op.create_index( + "ix_design_systems_owner_user_id", "design_systems", ["owner_user_id"] + ) + op.create_index("ix_design_systems_parent_id", "design_systems", ["parent_id"]) + + op.create_table( + "design_tokens", + sa.Column("id", sa.BigInteger(), primary_key=True), + sa.Column( + "design_system_id", sa.BigInteger(), + sa.ForeignKey("design_systems.id", ondelete="CASCADE"), nullable=False, + ), + sa.Column("name", sa.Text(), nullable=False), + # NOT NULL with a '{}' default: a nullable JSONB column has two empty + # states (SQL NULL and JSON null) and every reader has to test for both. + sa.Column( + "value_by_mode", JSONB, + nullable=False, server_default=sa.text("'{}'::jsonb"), + ), + sa.Column("group_name", sa.Text(), nullable=True), + sa.Column("purpose", sa.Text(), nullable=True), + sa.Column("order_index", sa.Integer(), nullable=False, server_default="0"), + sa.Column( + "created_at", sa.DateTime(timezone=True), nullable=False, + server_default=sa.text("now()"), + ), + sa.Column( + "updated_at", sa.DateTime(timezone=True), nullable=False, + server_default=sa.text("now()"), + ), + sa.Column("deleted_at", sa.DateTime(timezone=True), nullable=True), + sa.Column("deleted_batch_id", sa.Text(), nullable=True), + ) + op.create_index( + "ix_design_tokens_design_system_id", "design_tokens", ["design_system_id"] + ) + # Partial unique: a name is unique among LIVE tokens in a system. Two live + # rows with the same name are a duplicate definition and the cascade would + # pick between them arbitrarily; a trashed row must not block reusing its + # name. + op.create_index( + "uq_token_per_design_system", "design_tokens", ["design_system_id", "name"], + unique=True, postgresql_where=sa.text("deleted_at IS NULL"), + ) + + op.add_column( + "projects", sa.Column("design_system_id", sa.BigInteger(), nullable=True) + ) + op.create_foreign_key( + "fk_projects_design_system_id", "projects", "design_systems", + ["design_system_id"], ["id"], ondelete="SET NULL", + ) + + +def downgrade() -> None: + op.drop_constraint("fk_projects_design_system_id", "projects", type_="foreignkey") + op.drop_column("projects", "design_system_id") + op.drop_index("uq_token_per_design_system", table_name="design_tokens") + op.drop_index("ix_design_tokens_design_system_id", table_name="design_tokens") + op.drop_table("design_tokens") + op.drop_index("ix_design_systems_parent_id", table_name="design_systems") + op.drop_index("ix_design_systems_owner_user_id", table_name="design_systems") + op.drop_table("design_systems") diff --git a/alembic/versions/0073_design_token_supersedes.py b/alembic/versions/0073_design_token_supersedes.py new file mode 100644 index 0000000..5596bb2 --- /dev/null +++ b/alembic/versions/0073_design_token_supersedes.py @@ -0,0 +1,55 @@ +"""design_tokens.supersedes — the literals a token should be used instead of + +Revision ID: 0073 +Revises: 0072 +Create Date: 2026-07-30 + +Records what a prohibition was actually trying to say. + +A design rulebook writes "pure white #FFFFFF is NEVER used as text color". That +sentence has no row in a table of tokens, because a design system stores what +things ARE — which looked like a gap in the model and was really a sentence +written backwards. The positive fact is "text is Parchment", and the useful +record is the mapping from the literal someone would otherwise write to the +token they should write instead. + +So `supersedes` is a JSONB array of literal values, e.g. `["#fff", "#ffffff"]` +on a text-on-action token. A finding built from it can say what to write, not +merely what not to. + +It must be DECLARED rather than derived. `#fff` and Parchment `#E8E4D8` are +different colours, so no value-matching rule could ever have connected them — +which is exactly why the prohibition felt unrepresentable until it was turned +around. + +Consumed by the source lint that reads component CSS, not by the drift panel: +these literals are in the components, which the panel cannot see. + +NOT NULL with a `'[]'` default, matching `value_by_mode` — a nullable JSONB +column has two empty states and every reader has to test for both. + +Downgrade drops the column; the declarations are lost, which costs the lint its +input and nothing else. +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects.postgresql import JSONB + + +revision = "0073" +down_revision = "0072" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.add_column( + "design_tokens", + sa.Column( + "supersedes", JSONB, nullable=False, server_default=sa.text("'[]'::jsonb") + ), + ) + + +def downgrade() -> None: + op.drop_column("design_tokens", "supersedes") diff --git a/alembic/versions/0074_design_prose.py b/alembic/versions/0074_design_prose.py new file mode 100644 index 0000000..b48971c --- /dev/null +++ b/alembic/versions/0074_design_prose.py @@ -0,0 +1,44 @@ +"""central prose on a design system, and rationale on a token + +Revision ID: 0074 +Revises: 0073 +Create Date: 2026-07-31 + +The operator's shape for #254: prose doesn't live as one-offs, it lives centrally +on the system. Two fields rather than one per category: + + design_systems.guidance the narrative a token table cannot hold — aesthetic, + voice and tone, what is deliberately out of scope. + Markdown, free-form. + design_tokens.rationale WHY this token is this value, which is a different + question from `purpose` (what it is FOR). "Success + equals Moss, aligned by design" is a rationale; + "page bg, deepest surface" is a purpose. + +Free-form rather than a column per category on purpose. A schema with `voice`, +`aesthetic` and `scope` columns would bake one rulebook's table of contents into +every install (rule #115), and the next install's design system would have three +empty columns and nowhere to put what it actually cares about. + +Both nullable: a design system with no prose at all is complete, not a draft. + +Downgrade drops both columns and the prose with them. +""" +from alembic import op +import sqlalchemy as sa + + +revision = "0074" +down_revision = "0073" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.add_column("design_systems", sa.Column("guidance", sa.Text(), nullable=True)) + op.add_column("design_tokens", sa.Column("rationale", sa.Text(), nullable=True)) + + +def downgrade() -> None: + op.drop_column("design_tokens", "rationale") + op.drop_column("design_systems", "guidance") diff --git a/frontend/src/api/design.ts b/frontend/src/api/design.ts new file mode 100644 index 0000000..cbdfe09 --- /dev/null +++ b/frontend/src/api/design.ts @@ -0,0 +1,9 @@ +import { apiGet } from "@/api/client"; +import type { ExpectationResponse } from "@/utils/designDrift"; + +/** Checkable claims from the rulebook this install designated as its design system. + * + * `rulebook_id: null` means none has been designated — the normal state for a + * fresh install, not an error. The caller shows an explanatory empty state. */ +export const fetchDesignExpectations = () => + apiGet("/api/design/expectations"); diff --git a/frontend/src/api/designSystems.ts b/frontend/src/api/designSystems.ts new file mode 100644 index 0000000..f9333fe --- /dev/null +++ b/frontend/src/api/designSystems.ts @@ -0,0 +1,216 @@ +/** + * Design systems — the stylesheet held as records (milestone #254). + * + * A design system is a named set of tokens with an optional parent. A system + * with no parent is a "family"; one with a parent holds ONLY what it changes, + * so "what does this app alter?" is a plain list rather than a diff. + */ +import { apiDelete, apiGet, apiPatch, apiPost, apiPut } from "@/api/client"; + +export interface DesignSystem { + id: number; + owner_user_id: number; + title: string; + description: string; + /** Narrative a token table cannot hold: aesthetic, voice, what's out of scope. */ + guidance: string; + parent_id: number | null; + created_at: string | null; + updated_at: string | null; +} + +/** A token as STORED — one system's own row for it. */ +export interface DesignToken { + id: number; + design_system_id: number; + name: string; + /** Values keyed by mode. `base` applies when no mode is more specific. */ + value_by_mode: Record; + group_name: string | null; + purpose: string | null; + /** WHY it is this value — distinct from `purpose`, which is what it is FOR. */ + rationale: string | null; + /** Literal values this token should be used INSTEAD OF, e.g. ["#fff"]. + * + * How a design system records what a prohibition was trying to say: not + * "white is banned" but "write this token instead". Declared rather than + * inferred, because a superseded literal and the token's own value are + * usually different values and nothing could connect them by matching. */ + supersedes: string[]; + order_index: number; +} + +export interface Contribution { + system_id: number; + value: string; +} + +/** + * A token after the cascade. + * + * `contributions` is every system that offered a value, per mode, DEEPEST + * FIRST — entry 0 won and the rest were shadowed. `value_by_mode` and + * `origin_by_mode` are the winners, provided so the client never has to derive + * them (and so it cannot derive them differently). + * + * Provenance is per MODE because overriding is: a system can own `base` and + * inherit `dark` at the same time. + */ +export interface ResolvedToken { + name: string; + group_name: string | null; + purpose: string | null; + rationale: string | null; + supersedes: string[]; + order_index: number; + value_by_mode: Record; + origin_by_mode: Record; + contributions: Record; +} + +export const fetchDesignSystems = () => + apiGet<{ design_systems: DesignSystem[] }>("/api/design-systems"); + +export const fetchDesignSystem = (id: number) => + apiGet(`/api/design-systems/${id}`); + +export const createDesignSystem = (body: { + title: string; + description?: string; + guidance?: string; + parent_id?: number | null; +}) => apiPost("/api/design-systems", body); + +/** Omit `parent_id` to leave it alone; send `null` to make the system a family. */ +export const updateDesignSystem = ( + id: number, + body: { + title?: string; + description?: string; + guidance?: string; + parent_id?: number | null; + }, +) => apiPatch(`/api/design-systems/${id}`, body); + +export const deleteDesignSystem = (id: number) => + apiDelete(`/api/design-systems/${id}`); + +/** The EFFECTIVE set: everything inherited, with this system's on top. */ +export const fetchResolvedTokens = (id: number) => + apiGet<{ design_system_id: number; tokens: ResolvedToken[] }>( + `/api/design-systems/${id}/resolved`, + ); + +/** This system's OWN tokens — its override set. */ +export const fetchDesignTokens = (id: number) => + apiGet<{ tokens: DesignToken[] }>(`/api/design-systems/${id}/tokens`); + +export const createDesignToken = ( + designSystemId: number, + body: { + name: string; + value_by_mode?: Record; + group_name?: string | null; + purpose?: string | null; + rationale?: string | null; + supersedes?: string[]; + order_index?: number; + }, +) => apiPost(`/api/design-systems/${designSystemId}/tokens`, body); + +export const updateDesignToken = ( + tokenId: number, + body: Partial>, +) => apiPatch(`/api/design-tokens/${tokenId}`, body); + +export const deleteDesignToken = (tokenId: number) => + apiDelete(`/api/design-tokens/${tokenId}`); + +/** Point a project at a design system. `null` clears it. */ +export const setProjectDesignSystem = ( + projectId: number, + designSystemId: number | null, +) => + apiPut<{ project_id: number; design_system_id: number | null }>( + `/api/projects/${projectId}/design-system`, + { design_system_id: designSystemId }, + ); + +/** One token an import proposes, with the evidence for it. */ +export interface ProposedToken { + name: string; + value_by_mode: Record; + group_name: string | null; + purpose: string | null; + supersedes: string[]; + source_rule_id: number | null; + source_rule_title: string; + source_context: string; +} + +export interface ImportReport { + rulebook_id: number; + proposed: ProposedToken[]; + created: DesignToken[]; + /** Proposals whose name the system already defines. Never overwritten. */ + skipped: string[]; +} + +/** Seed a design system from a rulebook that describes one in prose. + * + * Defaults to a PREVIEW: an import is a proposal, since rulebooks are written + * aspirationally and some of what they describe was never built. Pass + * `apply: true` to write. Existing token names are never overwritten, so a + * second run fills gaps and reports the rest. */ +export const importFromRulebook = ( + designSystemId: number, + rulebookId: number, + apply = false, +) => + apiPost(`/api/design-systems/${designSystemId}/import`, { + rulebook_id: rulebookId, + apply, + }); + +export interface StylesheetResult { + design_system_id: number; + /** The master sheet: purpose tokens only, no element or class rules. */ + css: string; + token_count: number; + /** Tokens the system names but has no value for yet. */ + valueless: string[]; + /** Values declared under more than one name — alias, or one idea twice. */ + duplicates: Record; +} + +/** The master CSS sheet a design system generates. + * + * Purpose tokens only. Components (buttons, tables, input schemes) are + * snippets that reference these names, so a value is stated once and reused + * rather than restated per element. */ +export const fetchStylesheet = (id: number) => + apiGet(`/api/design-systems/${id}/stylesheet`); + +export interface SnippetFinding { + snippet_id: number; + title: string; + /** References that resolve against the sheet. */ + used: string[]; + /** `var(--x)` where the system has no `--x` — renders as nothing at all. */ + unknown: string[]; + /** Literals the sheet says to stop writing, paired with what to write. */ + superseded_literals: { literal: string; use_instead: string }[]; + /** Custom properties the snippet mints for itself instead of reusing. */ + local_definitions: string[]; +} + +export interface SnippetCheck { + design_system_id: number; + checked: number; + /** Only snippets with something to act on; clean ones are omitted. */ + findings: SnippetFinding[]; +} + +/** Which recorded snippets disagree with this design system's sheet. */ +export const checkSnippets = (id: number) => + apiGet(`/api/design-systems/${id}/snippet-check`); diff --git a/frontend/src/components/AppHeader.vue b/frontend/src/components/AppHeader.vue index 37c567b..9afa991 100644 --- a/frontend/src/components/AppHeader.vue +++ b/frontend/src/components/AppHeader.vue @@ -6,7 +6,7 @@ import { useShortcuts } from "@/composables/useShortcuts"; import { useAuthStore } from "@/stores/auth"; import AppLogo from "@/components/AppLogo.vue"; import NotificationBell from "@/components/NotificationBell.vue"; -import { Sun, Moon, Settings, Trash2 } from "lucide-vue-next"; +import { Sun, Moon, Palette, Settings, Trash2 } from "lucide-vue-next"; const { theme, toggleTheme } = useTheme(); const { toggleShortcuts } = useShortcuts(); @@ -64,6 +64,13 @@ router.afterEach(() => { + + + + + @@ -98,6 +105,8 @@ router.afterEach(() => { Rulebooks Shared
+ Design + Design systems Trash Settings
diff --git a/frontend/src/router/index.ts b/frontend/src/router/index.ts index 43de4dd..5e9bf81 100644 --- a/frontend/src/router/index.ts +++ b/frontend/src/router/index.ts @@ -109,6 +109,20 @@ const router = createRouter({ name: "rules", component: () => import("@/views/RulesView.vue"), }, + { + // Meta-surface, same family as /rules: it describes the app rather than + // holding the operator's records. + path: "/design", + name: "design", + component: () => import("@/views/DesignView.vue"), + }, + { + // The editable half of the same surface: /design is what the browser + // renders, /design-systems is the record that ought to decide it. + path: "/design-systems", + name: "design-systems", + component: () => import("@/views/DesignSystemsView.vue"), + }, { path: "/tasks", redirect: "/", diff --git a/frontend/src/utils/designDrift.ts b/frontend/src/utils/designDrift.ts new file mode 100644 index 0000000..453e84f --- /dev/null +++ b/frontend/src/utils/designDrift.ts @@ -0,0 +1,169 @@ +/** + * Drift comparison — what the rulebook claims vs what the stylesheet does. + * + * Milestone #251 step 5. Deliberately thin: the hard half (turning rulebook + * prose into claims) is server-side in `services/design_system.py`, where pytest + * can assert on it. What's left here is set arithmetic over live token values, + * which is the one thing the browser knows and the server doesn't. + * + * SCOPE, and it is a real limit rather than an omission. This compares the + * rulebook against the TOKENS. It cannot see the third category of drift — a + * literal hardcoded in a component where a token should be referenced (#2275, + * 67 occurrences of `color: #fff` against a rule that forbids pure white). That + * drift isn't in the tokens at all, so no amount of inspecting them finds it. + * + * Catching it needs the component sources, which would mean bundling every SFC + * into the app to read at runtime — a large cost for a panel. It belongs in CI, + * as a lint-shaped check, and is tracked there (#2277). Saying so in the panel + * matters: a drift report that silently omits a category invites the reader to + * conclude the category is clean. + */ +import type { DesignToken } from "@/utils/designTokens"; + +export type ExpectationKind = "token" | "color" | "prohibited_color"; + +export interface Expectation { + kind: ExpectationKind; + value: string; + rule_id: number; + rule_title: string; + context: string; +} + +export interface ExpectationResponse { + rulebook_id: number | null; + expectations: Expectation[]; +} + +export type FindingStatus = "ok" | "missing" | "violated"; + +export interface Finding { + expectation: Expectation; + status: FindingStatus; + /** Tokens that satisfy (or, for a prohibition, breach) the expectation. */ + matches: string[]; +} + +/** + * Normalise a colour for comparison — the client-side twin of + * `normalize_hex` in services/design_system.py. + * + * These two MUST agree. The rulebook writes `#FFFFFF`, `theme.css` writes + * `#fff`, and getComputedStyle hands back `rgb(255, 255, 255)` — three + * spellings of one colour, and a comparison that misses any of them under-reports + * rather than erroring. The rgb() case is browser-specific and therefore has no + * server-side counterpart, which is exactly why it is handled here. + */ +export function normalizeColour(value: string): string | null { + const raw = value.trim().toLowerCase(); + + const hex = /^#([0-9a-f]{3,8})$/.exec(raw); + if (hex) { + let digits = hex[1]; + if (digits.length === 3 || digits.length === 4) { + digits = digits.split("").map((c) => c + c).join(""); + } + return digits.length === 6 || digits.length === 8 ? `#${digits}` : null; + } + + // getComputedStyle always reports colours as rgb()/rgba(), never as authored. + const rgb = /^rgba?\(([^)]+)\)$/.exec(raw); + if (rgb) { + const parts = rgb[1].split(/[,\s/]+/).filter(Boolean); + if (parts.length < 3) return null; + const channels = parts.slice(0, 3).map((p) => Number(p)); + if (channels.some((n) => !Number.isFinite(n))) return null; + const hexOf = (n: number) => Math.round(n).toString(16).padStart(2, "0"); + const base = `#${channels.map(hexOf).join("")}`; + if (parts.length === 3) return base; + const alpha = Number(parts[3]); + if (!Number.isFinite(alpha) || alpha >= 1) return base; + return `${base}${hexOf(alpha * 255)}`; + } + + return null; +} + +/** Every distinct colour the stylesheet actually resolves to, mapped to its tokens. */ +export function colourIndex(tokens: DesignToken[]): Map { + const index = new Map(); + for (const token of tokens) { + const colour = normalizeColour(token.value); + if (!colour) continue; + const names = index.get(colour); + if (names) names.push(token.name); + else index.set(colour, [token.name]); + } + return index; +} + +/** + * Compare claims against the live tokens. + * + * A `token` claim asks whether a custom property of that name exists. + * A `color` claim asks whether any token resolves to that value. + * A `prohibited_color` claim INVERTS the test — present is the failure. + */ +export function compareToTokens( + expectations: Expectation[], + tokens: DesignToken[], +): Finding[] { + const names = new Set(tokens.map((t) => t.name)); + const colours = colourIndex(tokens); + + return expectations.map((expectation) => { + if (expectation.kind === "token") { + const present = names.has(expectation.value); + return { + expectation, + status: present ? "ok" : "missing", + matches: present ? [expectation.value] : [], + }; + } + + const matches = colours.get(expectation.value) ?? []; + if (expectation.kind === "prohibited_color") { + return { + expectation, + status: matches.length ? "violated" : "ok", + matches, + }; + } + return { + expectation, + status: matches.length ? "ok" : "missing", + matches, + }; + }); +} + +export interface DriftSummary { + ok: number; + missing: number; + violated: number; + total: number; +} + +export function summarise(findings: Finding[]): DriftSummary { + const summary: DriftSummary = { ok: 0, missing: 0, violated: 0, total: findings.length }; + for (const finding of findings) summary[finding.status] += 1; + return summary; +} + +/** + * Findings worth leading with. + * + * A panel that opens with every row gets closed and never reopened — the same + * principle the auto-inject menu is built on: a short list that gets read beats + * a complete one that doesn't. Violations first (something is actively wrong), + * then missing (something was never built), and `ok` rows are not "findings" at + * all — they belong behind an expansion. + */ +export function rankFindings(findings: Finding[]): Finding[] { + const order: Record = { violated: 0, missing: 1, ok: 2 }; + return [...findings].sort((a, b) => { + const byStatus = order[a.status] - order[b.status]; + if (byStatus !== 0) return byStatus; + return a.expectation.rule_id - b.expectation.rule_id; + }); +} diff --git a/frontend/src/utils/designTokens.ts b/frontend/src/utils/designTokens.ts new file mode 100644 index 0000000..029d475 --- /dev/null +++ b/frontend/src/utils/designTokens.ts @@ -0,0 +1,181 @@ +/** + * Design-token inventory — what tokens exist, and what they actually resolve to. + * + * Foundation for the design explorer (milestone #251): the gallery renders + * against these, and the drift panel compares them to the design rulebook. + * + * DESIGN NOTE — why this parses NAMES but never VALUES. + * Extracting `--foo` from a stylesheet is a trivial, robust regex. Extracting + * its VALUE is not: values contain nested parens, commas inside rgba(), + * `var()` references to other tokens, multi-part shadows, and gradients — and + * `theme.css` has all of those today. So we take the names from the source and + * ask the BROWSER for every value. + * + * That is not just easier, it is more correct. getComputedStyle reports what + * actually won the cascade, resolves `var()` chains, and — critically for this + * milestone — reflects live overrides set on a container, which is exactly what + * the preview surface needs (see #2261). Parsing the source would report what + * the file says rather than what the user is looking at. + * + * It also means this module needs no unit tests to be trustworthy: the only + * logic here is a name regex and a group lookup. The frontend has no test + * runner today (`vue-tsc --noEmit` is the whole check), so keeping the + * error-prone half in the browser rather than in our code is deliberate. + */ +import themeCss from "@/assets/theme.css?raw"; + +export type TokenGroup = + | "color" + | "radius" + | "gradient" + | "glow" + | "focus" + | "layout" + | "other"; + +export type ThemeMode = "light" | "dark"; + +export interface DesignToken { + /** Full custom-property name, including the leading `--`. */ + name: string; + /** Coarse family, derived from the name prefix. */ + group: TokenGroup; + /** Resolved value in the requested context, straight from the browser. */ + value: string; + /** True when the declaration appears inside the dark block in source. */ + overriddenInDark: boolean; +} + +/** + * Matches a custom-property DECLARATION, and never a `var(--name)` use. + * + * The discriminator is the COLON, not the preceding character. A declaration is + * `--name:`; a reference is `var(--name)` or `var(--name, fallback)` — followed + * by `)` or `,`, never by `:`. So no anchor is needed, and adding one is + * actively wrong: an earlier version required the match to follow `{` or `;`, + * which silently dropped every declaration that came after a comment — + * including `--color-bg`, the first and most-used token in the file. + */ +const DECLARATION = /(--[A-Za-z0-9_-]+)\s*:/g; + +/** Comments are stripped first so a commented-out declaration isn't counted. */ +const COMMENT = /\/\*[\s\S]*?\*\//g; + +/** The dark block's selector, as written in theme.css. */ +const DARK_SELECTOR = '[data-theme="dark"]'; + +const GROUP_PREFIXES: ReadonlyArray<[string, TokenGroup]> = [ + ["--color-", "color"], + ["--radius-", "radius"], + ["--gradient-", "gradient"], + ["--glow-", "glow"], + ["--focus-", "focus"], + ["--page-", "layout"], + ["--sidebar-", "layout"], + ["--chat-", "layout"], +]; + +export function groupFor(name: string): TokenGroup { + for (const [prefix, group] of GROUP_PREFIXES) { + if (name.startsWith(prefix)) return group; + } + return "other"; +} + +/** Every custom property declared anywhere in the stylesheet, in source order, deduped. */ +export function tokenNames(css: string = themeCss): string[] { + const seen = new Set(); + const out: string[] = []; + for (const match of css.replace(COMMENT, "").matchAll(DECLARATION)) { + const name = match[1]; + if (!seen.has(name)) { + seen.add(name); + out.push(name); + } + } + return out; +} + +/** The subset re-declared inside the dark block — i.e. tokens that change with mode. */ +export function darkOverriddenNames(css: string = themeCss): Set { + const bare = css.replace(COMMENT, ""); + const start = bare.indexOf(DARK_SELECTOR); + if (start === -1) return new Set(); + const open = bare.indexOf("{", start); + const close = bare.indexOf("}", open); + if (open === -1 || close === -1) return new Set(); + return new Set(tokenNames(bare.slice(open, close))); +} + +/** + * Read the resolved value of every token in `host`'s context. + * + * Pass a container to read the tokens as they apply INSIDE it — which is how + * the preview surface reads a scoped override without disturbing the page. + * Defaults to the document root, i.e. the app-wide values. + */ +export function readTokens(host: Element = document.documentElement): DesignToken[] { + const computed = getComputedStyle(host); + const dark = darkOverriddenNames(); + return tokenNames().map((name) => ({ + name, + group: groupFor(name), + value: computed.getPropertyValue(name).trim(), + overriddenInDark: dark.has(name), + })); +} + +/** + * Read tokens as they would resolve in a given mode, without touching the page. + * + * Uses an offscreen probe carrying the mode attribute, so the live UI is never + * mutated to take a reading. + * + * KNOWN LIMITATION, and it is a property of the stylesheet rather than of this + * function: light is declared on `:root` while dark is declared on + * `[data-theme="dark"]`. An attribute selector can ADD the dark values to a + * subtree, but there is no `[data-theme="light"]` block to add the light ones + * back. So reading "light" from inside a dark page returns the dark values — + * the probe has nothing to match. + * + * Concretely: dark-inside-light previews work, light-inside-dark previews do + * not. Introducing a `[data-theme="light"]` block alongside the dark-first flip + * (milestone #251 step 6) is what makes this symmetric, and until then callers + * should treat a cross-mode read as best-effort. + */ +export function readTokensForMode(mode: ThemeMode): DesignToken[] { + const probe = document.createElement("div"); + probe.setAttribute("data-theme", mode); + probe.style.display = "none"; + document.body.appendChild(probe); + try { + return readTokens(probe); + } finally { + probe.remove(); + } +} + +/** Tokens grouped by family, preserving source order within each group. */ +export function groupTokens(tokens: DesignToken[]): Map { + const out = new Map(); + for (const token of tokens) { + const bucket = out.get(token.group); + if (bucket) bucket.push(token); + else out.set(token.group, [token]); + } + return out; +} + +/** + * Tokens declared in the stylesheet that nothing references with `var()`. + * + * Dead tokens are drift too: `--chat-reading-width` and + * `--chat-context-sidebar-width` outlived the chat subsystem that was deleted + * in the MCP-first pivot, and nothing has referenced them since. Takes the + * corpus of source files to search as an argument so the caller decides what + * "used" means — this module has no opinion about the project layout. + */ +export function unreferencedTokens(tokens: DesignToken[], sources: string[]): DesignToken[] { + const haystack = sources.join("\n"); + return tokens.filter((token) => !haystack.includes(`var(${token.name}`)); +} diff --git a/frontend/src/views/DesignSystemsView.vue b/frontend/src/views/DesignSystemsView.vue new file mode 100644 index 0000000..12725c1 --- /dev/null +++ b/frontend/src/views/DesignSystemsView.vue @@ -0,0 +1,1648 @@ + + + + + diff --git a/frontend/src/views/DesignView.vue b/frontend/src/views/DesignView.vue new file mode 100644 index 0000000..5d137f4 --- /dev/null +++ b/frontend/src/views/DesignView.vue @@ -0,0 +1,519 @@ + + + + + diff --git a/frontend/src/views/ProjectView.vue b/frontend/src/views/ProjectView.vue index 4e2d9ea..14aeca2 100644 --- a/frontend/src/views/ProjectView.vue +++ b/frontend/src/views/ProjectView.vue @@ -9,6 +9,11 @@ import { renderMarkdown } from "@/utils/markdown"; import ShareDialog from "@/components/ShareDialog.vue"; import ProjectRulesTab from "@/components/rules/ProjectRulesTab.vue"; import SystemsSection from "@/components/SystemsSection.vue"; +import { + fetchDesignSystems, + setProjectDesignSystem, + type DesignSystem, +} from "@/api/designSystems"; import { LayoutGrid, Clock, @@ -41,6 +46,7 @@ interface Project { goal: string | null; status: "active" | "paused" | "completed" | "archived"; color: string | null; + design_system_id: number | null; permission?: string; created_at: string; updated_at: string; @@ -69,6 +75,12 @@ const toast = useToastStore(); const tasksStore = useTasksStore(); const project = ref(null); + +// Design system the project is styled from. Loaded separately because an +// install with none is the ordinary case (rule #115) and the picker simply +// doesn't render — a failed fetch must not take the project page with it. +const designSystems = ref([]); +const editDesignSystemId = ref(null); const loading = ref(false); const showStartPlanning = ref(false); @@ -175,6 +187,7 @@ async function loadProject() { editDescription.value = data.description ?? ""; editGoal.value = data.goal ?? ""; editStatus.value = data.status; + editDesignSystemId.value = data.design_system_id ?? null; editDirty.value = false; milestones.value = data.summary?.milestone_summary ?? []; autoCollapseCompleted(milestones.value); @@ -336,8 +349,20 @@ onMounted(async () => { await loadProject(); loadTasks(); loadNotes(); + loadDesignSystems(); }); +/** Populate the design-system picker. Swallows failure on purpose: with no + * design systems the picker doesn't render at all, which is the ordinary state + * for most installs — so this must never be able to break the project page. */ +async function loadDesignSystems() { + try { + designSystems.value = (await fetchDesignSystems()).design_systems; + } catch { + designSystems.value = []; + } +} + watch(projectId, async () => { await loadProject(); loadTasks(); @@ -345,28 +370,39 @@ watch(projectId, async () => { }); watch( - () => [editTitle.value, editDescription.value, editGoal.value, editStatus.value], + () => [editTitle.value, editDescription.value, editGoal.value, editStatus.value, editDesignSystemId.value], () => { if (!project.value) return; editDirty.value = editTitle.value !== project.value.title || editDescription.value !== (project.value.description ?? "") || editGoal.value !== (project.value.goal ?? "") || - editStatus.value !== project.value.status; + editStatus.value !== project.value.status || + editDesignSystemId.value !== (project.value.design_system_id ?? null); } ); async function saveProject() { - if (!project.value || saving.value) return; + // Bound once rather than re-read: the checks below straddle two awaits, and + // `project.value` is a ref whose narrowing doesn't survive them. + const current = project.value; + if (!current || saving.value) return; saving.value = true; try { - const updated = await apiPatch(`/api/projects/${project.value.id}`, { + const updated = await apiPatch(`/api/projects/${current.id}`, { title: editTitle.value.trim(), description: editDescription.value.trim() || null, goal: editGoal.value.trim() || null, status: editStatus.value, }); - project.value = { ...project.value, ...updated }; + // The design-system pointer is its own endpoint (PUT, because clearing it + // is a real outcome rather than an omission), so it saves separately — + // only when it actually changed, to keep the common save at one request. + if (editDesignSystemId.value !== (current.design_system_id ?? null)) { + await setProjectDesignSystem(current.id, editDesignSystemId.value); + updated.design_system_id = editDesignSystemId.value; + } + project.value = { ...current, ...updated }; editDirty.value = false; toast.show("Project saved"); } catch { @@ -505,6 +541,13 @@ async function confirmDelete() { +
+ + +
diff --git a/frontend/src/views/SettingsView.vue b/frontend/src/views/SettingsView.vue index 4c7c8d8..fbcf524 100644 --- a/frontend/src/views/SettingsView.vue +++ b/frontend/src/views/SettingsView.vue @@ -4,6 +4,7 @@ import { useSettingsStore } from "@/stores/settings"; import { useAuthStore } from "@/stores/auth"; import { useToastStore } from "@/stores/toast"; import { apiGet, apiPost, apiPut, apiDelete, listGroups, createGroup, deleteGroup, listGroupMembers, addGroupMember, removeGroupMember, searchUsers, listApiKeys, createApiKey as apiCreateApiKey, revokeApiKey as apiRevokeApiKey, getProfile, updateProfile, type ApiKeyEntry, type GroupEntry, type GroupMember, type UserSearchResult, type UserProfile } from "@/api/client"; +import { listRulebooks } from "@/api/rulebooks"; import type { User } from "@/types/auth"; import PaginationBar from "@/components/PaginationBar.vue"; import TagInput from "@/components/TagInput.vue"; @@ -31,6 +32,11 @@ const kbWritePathThreshold = ref("0.68"); // gate: that one BLOCKS a create and must be unforgiving of noise, this one only // suggests a merge the operator reviews (services/dedup.py). const kbDuplicateThreshold = ref("0.82"); +// Which rulebook describes this install's design system, for the /design drift +// panel. Empty = none designated, which is the normal state for a fresh install +// rather than a misconfiguration — the panel explains itself when unset. +const designRulebookId = ref(""); +const designRulebooks = ref<{ id: number; title: string }[]>([]); const savingKbInject = ref(false); const kbInjectSaved = ref(false); @@ -100,6 +106,9 @@ async function saveKbInject() { kb_writepath_enabled: kbWritePathEnabled.value ? 'true' : 'false', kb_writepath_threshold: String(wpT), kb_duplicate_threshold: String(dupT), + // Empty string DELETES the setting (see routes/settings.py), which is + // exactly right for "no design rulebook" — absent rather than zero. + design_rulebook_id: designRulebookId.value, }); kbInjectSaved.value = true; setTimeout(() => (kbInjectSaved.value = false), 2000); @@ -490,6 +499,14 @@ onMounted(async () => { if (allSettings.kb_duplicate_threshold !== undefined) { kbDuplicateThreshold.value = allSettings.kb_duplicate_threshold; } + designRulebookId.value = allSettings.design_rulebook_id ?? ""; + // Best-effort: the picker degrades to "none available" rather than blocking + // the whole settings page if rulebooks can't be listed. + try { + designRulebooks.value = (await listRulebooks()).map((r) => ({ id: r.id, title: r.title })); + } catch { + designRulebooks.value = []; + } if (allSettings.notify_task_reminders !== undefined) { notifyTaskReminders.value = allSettings.notify_task_reminders !== "false"; } @@ -1260,6 +1277,22 @@ function formatUserDate(iso: string): string { location, not by resemblance.

+
+ + +

+ Which rulebook describes how this app should look. Once set, the + Design page compares every colour + and token your rules name against what the stylesheet actually resolves + to, and reports where they disagree. Leave it as None if your rules + don't describe a design system — nothing else depends on this. +

+
Quart: app.register_blueprint(knowledge_bp) app.register_blueprint(rulebooks_bp) app.register_blueprint(plugin_bp) + app.register_blueprint(design_bp) + app.register_blueprint(design_systems_bp) app.register_blueprint(trash_bp) app.register_blueprint(dashboard_bp) app.register_blueprint(systems_bp) diff --git a/src/scribe/mcp/tools/__init__.py b/src/scribe/mcp/tools/__init__.py index b9b8dc3..f53e73f 100644 --- a/src/scribe/mcp/tools/__init__.py +++ b/src/scribe/mcp/tools/__init__.py @@ -5,7 +5,8 @@ to a FastMCP instance. `register_all(mcp)` is the single entry point called from `mcp.server.build_mcp_server`. """ from scribe.mcp.tools import ( - milestones, notes, processes, projects, recent, repos, rulebooks, search, snippets, systems, tags, tasks, trash, + design_systems, milestones, notes, processes, projects, recent, repos, rulebooks, search, snippets, + systems, tags, tasks, trash, ) @@ -17,6 +18,7 @@ def register_all(mcp) -> None: projects.register(mcp) milestones.register(mcp) systems.register(mcp) + design_systems.register(mcp) tags.register(mcp) recent.register(mcp) repos.register(mcp) diff --git a/src/scribe/mcp/tools/design_systems.py b/src/scribe/mcp/tools/design_systems.py new file mode 100644 index 0000000..f8777bb --- /dev/null +++ b/src/scribe/mcp/tools/design_systems.py @@ -0,0 +1,398 @@ +"""Design system + token MCP tools — wrappers over services/design_systems.py. + +A design system is a stylesheet held as records: a named set of tokens with an +optional parent, so a family system carries the house style and an app system +carries only what it changes. Precedence by name along the parent chain IS the +CSS cascade, which is why "what does this app alter?" is a plain list rather +than a diff. + +Parity with the REST surface is a rule, not a nicety (see +`tests/test_routes_design_systems.py`): an agent and a browser are two callers +of one service. + +Sentinels, matching the milestone/task tool conventions: + - title="" / description="" / etc. -> "leave unchanged" on update + - parent_id / design_system_id: 0 = leave unchanged, -1 = clear, positive = set + (three states, because clearing a parent is a real operation and not the + same as omitting the argument) + - order_index=-1 -> "leave unchanged" (0 is a valid order_index) +""" +from __future__ import annotations + +from scribe.mcp._context import current_user_id +from scribe.services import design_systems as ds_svc +from scribe.services.design_systems import DesignSystemCycle + + +async def create_design_system( + title: str, + description: str = "", + guidance: str = "", + parent_id: int = 0, +) -> dict: + """Create a design system, optionally inheriting from another. + + Args: + title: What this system is, e.g. "FabledSword" or "Scribe" (required). + description: What it covers and when it applies. + guidance: The narrative a token table cannot hold — aesthetic, voice and + tone, what is deliberately out of scope. Markdown, free-form. + parent_id: Inherit from this system — it holds the defaults this one + overrides. Omit (0) for a top-level "family" system, which is what + a first design system usually is. + """ + uid = current_user_id() + system = await ds_svc.create_design_system( + uid, + title=title, + description=description or None, + guidance=guidance or None, + parent_id=parent_id or None, + ) + if system is None: + raise ValueError(f"parent design system {parent_id} not found or not writable") + return system.to_dict() + + +async def list_design_systems() -> dict: + """List your design systems. An empty list is normal — most installs have none.""" + uid = current_user_id() + rows = await ds_svc.list_design_systems(uid) + return {"design_systems": [s.to_dict() for s in rows]} + + +async def get_design_system(design_system_id: int) -> dict: + """Fetch a design system plus its OWN tokens — i.e. what it changes. + + For what it actually resolves to once inheritance is applied, use + `resolve_design_system`. The two answer different questions and a system + that overrides nothing has an empty token list but a full resolved set. + """ + uid = current_user_id() + system = await ds_svc.get_design_system(uid, design_system_id) + if system is None: + raise ValueError(f"design system {design_system_id} not found") + tokens = await ds_svc.list_tokens(uid, design_system_id) + return { + "design_system": system.to_dict(), + "tokens": [t.to_dict() for t in tokens], + } + + +async def resolve_design_system(design_system_id: int) -> dict: + """The EFFECTIVE token set — everything inherited, with this system's on top. + + Each token carries `origin_by_mode` (which system supplied each mode's + value) and `contributions` (every system that offered one, deepest first — + so entry 0 won and the rest were shadowed). Reach for this when you need to + know what a value actually IS; reach for `get_design_system` when you need + to know what this system CHANGES. + """ + uid = current_user_id() + resolved = await ds_svc.resolve_design_system(uid, design_system_id) + if resolved is None: + raise ValueError(f"design system {design_system_id} not found") + return { + "design_system_id": design_system_id, + "tokens": [t.to_dict() for t in resolved], + } + + +async def update_design_system( + design_system_id: int, + title: str = "", + description: str = "", + guidance: str = "", + parent_id: int = 0, +) -> dict: + """Update a design system. + + Args: + design_system_id: The system to update. + title: New title, or "" to leave unchanged. + description: New description, or "" to leave unchanged. + guidance: New guidance prose, or "" to leave unchanged. + parent_id: 0 = leave unchanged, -1 = clear (make this a top-level + family system), positive = inherit from that system. A parent that + already inherits from this system is refused — that would be a loop. + """ + uid = current_user_id() + fields: dict = {} + if title: + fields["title"] = title + if description: + fields["description"] = description + if guidance: + fields["guidance"] = guidance + if parent_id: + fields["parent_id"] = None if parent_id == -1 else parent_id + try: + system = await ds_svc.update_design_system(uid, design_system_id, **fields) + except DesignSystemCycle as exc: + raise ValueError(str(exc)) from exc + if system is None: + raise ValueError(f"design system {design_system_id} not found or not writable") + return system.to_dict() + + +async def delete_design_system(design_system_id: int) -> dict: + """Soft-delete a design system (recoverable). + + Systems that inherited from it become top-level systems keeping their own + tokens — deleting a family does not delete the apps under it. + """ + uid = current_user_id() + if not await ds_svc.delete_design_system(uid, design_system_id): + raise ValueError(f"design system {design_system_id} not found or not writable") + return {"message": f"Design system {design_system_id} deleted."} + + +async def get_design_system_stylesheet( + design_system_id: int, + root_selector: str = ":root", +) -> dict: + """The master CSS sheet a design system generates. + + Purpose tokens only — this sheet declares what values MEAN and styles no + elements. Components (buttons, tables, input schemes) are SNIPPETS that + reference these names, so a value is stated once and reused rather than + restated per element. Reach for this when you need the tokens a snippet is + allowed to use. + + Also returns `valueless` (tokens the system names but has no value for) and + `duplicates` (values declared under more than one name — a deliberate alias, + or one idea recorded twice). + + Args: + design_system_id: The system to render. + root_selector: Selector for the base layer. Defaults to `:root`; pass a + container selector to scope the sheet to a preview region. + """ + uid = current_user_id() + result = await ds_svc.stylesheet_for_system(uid, design_system_id, root_selector) + if result is None: + raise ValueError(f"design system {design_system_id} not found") + return result + + +async def check_snippets_against_design_system( + design_system_id: int, + project_id: int = 0, +) -> dict: + """Which recorded snippets disagree with a design system's sheet. + + Snippets are the component layer — buttons, tables, input schemes — and they + are supposed to use the tags the sheet declares. Three findings per snippet, + each currently silent in the codebase: + + unknown `var(--x)` where the system has no `--x`. Renders as + NOTHING: no error, no failing test, just an element + that quietly isn't styled. + superseded_literals a literal the sheet says to stop writing, paired with + the token to write instead. + local_definitions custom properties the snippet mints for itself rather + than using shared ones — the bloat a shared sheet + exists to prevent. + + Snippets with nothing to report are omitted. Reach for this before writing + or reviewing component CSS. + + Args: + design_system_id: The system whose sheet is authoritative. + project_id: Narrow to one project, or 0 for every project. + """ + uid = current_user_id() + result = await ds_svc.check_snippets_against_system( + uid, design_system_id, project_id + ) + if result is None: + raise ValueError(f"design system {design_system_id} not found") + return result + + +async def import_design_system_from_rulebook( + design_system_id: int, + rulebook_id: int, + apply: bool = False, +) -> dict: + """Seed a design system from a rulebook that describes one in prose. + + Reads the rulebook's colour and token declarations and proposes the tokens + they add up to — joining "Obsidian #14171A" in one rule to `--fs-obsidian` + in another, since neither alone is a token. + + Defaults to a PREVIEW. An import is a proposal: rulebooks are written + aspirationally and some of what they describe was never built, so read + `proposed` before setting apply=True. Every entry carries the rule and + sentence it came from so the claim can be checked rather than trusted. + + Existing token names are never overwritten — a second run fills gaps and + lists the rest under `skipped`, so it is safe to repeat. + + Args: + design_system_id: The system to seed. + rulebook_id: The rulebook to read. + apply: False (default) previews; True writes the tokens. + """ + uid = current_user_id() + report = await ds_svc.import_from_rulebook( + uid, design_system_id, rulebook_id, apply=apply, + ) + if report is None: + raise ValueError( + f"design system {design_system_id} not writable, or rulebook " + f"{rulebook_id} not readable" + ) + return report + + +# ── Tokens ────────────────────────────────────────────────────────────── + +async def create_design_token( + design_system_id: int, + name: str, + value_by_mode: dict | None = None, + group_name: str = "", + purpose: str = "", + rationale: str = "", + supersedes: list | None = None, + order_index: int = 0, +) -> dict: + """Add a token to a design system. + + Args: + design_system_id: The system that owns this token. + name: The custom-property name, e.g. "--fs-obsidian" (required). + value_by_mode: Values keyed by mode, e.g. + {"base": "#f7f5ef", "dark": "#14171a"}. Use "base" for the value + that applies when no mode is more specific; a token that is not + mode-dependent needs only "base". In a system WITH a parent, an + omitted mode is inherited rather than blanked. + group_name: Free-text grouping — "surface", "text", "radius", whatever + this system's own vocabulary is. + purpose: What the token is for, e.g. "page bg, deepest surface". + rationale: WHY it is this value — a different question from purpose. + "Success equals Moss, aligned by design" is a rationale; "page bg, + deepest surface" is a purpose. + supersedes: Literal values this token should be used INSTEAD OF, e.g. + ["#fff", "#ffffff"]. This is how a design system records what a + prohibition was trying to say — not "white is banned" but "write + this token instead". Declare it rather than expecting it to be + inferred: a superseded literal and the token's own value are + usually different values, so nothing can connect them by matching. + order_index: Display position within its group. + """ + uid = current_user_id() + token = await ds_svc.create_token( + uid, + design_system_id=design_system_id, + name=name, + value_by_mode=value_by_mode, + group_name=group_name or None, + purpose=purpose or None, + rationale=rationale or None, + supersedes=supersedes, + order_index=order_index, + ) + if token is None: + raise ValueError(f"design system {design_system_id} not found or not writable") + return token.to_dict() + + +async def list_design_tokens(design_system_id: int) -> dict: + """A design system's OWN tokens — its override set, not its effective set.""" + uid = current_user_id() + rows = await ds_svc.list_tokens(uid, design_system_id) + return {"tokens": [t.to_dict() for t in rows]} + + +async def update_design_token( + token_id: int, + name: str = "", + value_by_mode: dict | None = None, + group_name: str = "", + purpose: str = "", + rationale: str = "", + supersedes: list | None = None, + order_index: int = -1, +) -> dict: + """Update a token. Empty/None args leave a field unchanged. + + `value_by_mode` and `supersedes` REPLACE their whole value rather than + merging into it, so send every entry you want the token to keep. Pass `[]` + to clear `supersedes` entirely. + """ + uid = current_user_id() + fields: dict = {} + if name: + fields["name"] = name + if value_by_mode is not None: + fields["value_by_mode"] = value_by_mode + if group_name: + fields["group_name"] = group_name + if purpose: + fields["purpose"] = purpose + if rationale: + fields["rationale"] = rationale + # `is not None`, not truthiness: `[]` is a meaningful edit (drop every + # superseded literal) and would otherwise be unreachable. + if supersedes is not None: + fields["supersedes"] = supersedes + if order_index >= 0: + fields["order_index"] = order_index + token = await ds_svc.update_token(uid, token_id, **fields) + if token is None: + raise ValueError(f"design token {token_id} not found or not writable") + return token.to_dict() + + +async def delete_design_token(token_id: int) -> dict: + """Soft-delete a token (recoverable). + + In a system with a parent this restores inheritance: the token stops being + overridden here and resolves to the parent's value again. + """ + uid = current_user_id() + if not await ds_svc.delete_token(uid, token_id): + raise ValueError(f"design token {token_id} not found or not writable") + return {"message": f"Design token {token_id} deleted."} + + +async def set_project_design_system(project_id: int, design_system_id: int = 0) -> dict: + """Point a project at a design system. + + Args: + project_id: The project to style. + design_system_id: The system it uses, or -1 to clear it. Pointing at a + system only requires READ access to it — consuming a design system + is not changing it. + """ + uid = current_user_id() + target = None if design_system_id == -1 else design_system_id + ok = await ds_svc.set_project_design_system(uid, project_id, target) + if not ok: + raise ValueError( + f"project {project_id} not writable, or design system " + f"{design_system_id} not found" + ) + return {"project_id": project_id, "design_system_id": target} + + +def register(mcp) -> None: + for fn in ( + create_design_system, + list_design_systems, + get_design_system, + resolve_design_system, + update_design_system, + delete_design_system, + get_design_system_stylesheet, + check_snippets_against_design_system, + import_design_system_from_rulebook, + create_design_token, + list_design_tokens, + update_design_token, + delete_design_token, + set_project_design_system, + ): + mcp.tool(name=fn.__name__)(fn) diff --git a/src/scribe/models/__init__.py b/src/scribe/models/__init__.py index 2297cdd..0135af2 100644 --- a/src/scribe/models/__init__.py +++ b/src/scribe/models/__init__.py @@ -43,3 +43,4 @@ from scribe.models.rulebook import ( # noqa: E402, F401 ) from scribe.models.repo_binding import RepoBinding # noqa: E402, F401 from scribe.models.system import System, RecordSystem # noqa: E402, F401 +from scribe.models.design_system import DesignSystem, DesignToken # noqa: E402, F401 diff --git a/src/scribe/models/design_system.py b/src/scribe/models/design_system.py new file mode 100644 index 0000000..8aa2c6e --- /dev/null +++ b/src/scribe/models/design_system.py @@ -0,0 +1,156 @@ +"""Design systems — a stylesheet held as records, inherited family -> app. + +A DesignSystem is a named set of design tokens with an OPTIONAL parent, and that +single self-FK is the whole model. A system with no parent is a family system; a +system WITH one holds only what it changes. "What does this app alter?" is +therefore `list its tokens` — nothing to compute, nothing to diff — which is why +inheritance won over a flat family-plus-loose-overrides shape. + +Resolution walks the chain and lets the deepest system win by token name. That +is the CSS cascade rather than an analogy to it, which is why the storage model +and the stylesheet model come out the same shape. + +`parent_id` also replaces two things the rulebook model needs to express the same +idea: an `always_on` flag (a family system is simply one with no parent) and a +subscription join table (a project points at ONE system, and the chain supplies +the rest). Less schema for more structure. +""" +from sqlalchemy import BigInteger, ForeignKey, Index, Integer, Text, text +from sqlalchemy.dialects.postgresql import JSONB +from sqlalchemy.orm import Mapped, mapped_column + +from scribe.models import Base +from scribe.models.base import SoftDeleteMixin, TimestampMixin + + +class DesignSystem(Base, TimestampMixin, SoftDeleteMixin): + __tablename__ = "design_systems" + + id: Mapped[int] = mapped_column(BigInteger, primary_key=True) + owner_user_id: Mapped[int] = mapped_column( + BigInteger, ForeignKey("users.id", ondelete="CASCADE"), index=True + ) + title: Mapped[str] = mapped_column(Text) + description: Mapped[str | None] = mapped_column(Text, nullable=True) + # The narrative a token table cannot hold: aesthetic, voice and tone, what + # is deliberately out of scope. Free-form markdown rather than a column per + # category — a schema with `voice`/`aesthetic`/`scope` columns would bake one + # rulebook's table of contents into every install. + guidance: Mapped[str | None] = mapped_column(Text, nullable=True) + # SET NULL, not CASCADE: deleting a family system must not delete every app + # system that inherited from it. Orphaning turns each child into a root that + # still holds its own overrides — recoverable. A cascade would destroy data + # the operator never asked to touch. + parent_id: Mapped[int | None] = mapped_column( + BigInteger, + ForeignKey("design_systems.id", ondelete="SET NULL"), + nullable=True, + index=True, + ) + + def to_dict(self) -> dict: + return { + "id": self.id, + "owner_user_id": self.owner_user_id, + "title": self.title, + "description": self.description or "", + "guidance": self.guidance or "", + "parent_id": self.parent_id, + "created_at": self.created_at.isoformat() if self.created_at else None, + "updated_at": self.updated_at.isoformat() if self.updated_at else None, + } + + +class DesignToken(Base, TimestampMixin, SoftDeleteMixin): + """One custom property in one system: its name, and its value per mode. + + `value_by_mode` is a JSONB map of mode -> value, e.g. + `{"base": "#f7f5ef", "dark": "#14171a"}`. Three reasons it beats a pair of + `value_light` / `value_dark` columns here: + + - **Absence means one thing.** In a child system an unset mode means + "inherit"; in a root it would have to mean "not mode-dependent". With + columns those are both NULL and the resolver cannot tell them apart. With + a map, resolution is `{**parent_map, **child_map}` at every level — + one rule, no special case for roots. + - **Per-mode overrides are already real.** A palette rule in this operator's + own kit deepens one accent on light backgrounds for contrast while + leaving the dark value alone. Mode is a second override axis, not a + second column. + - **Nothing filters tokens by value in SQL.** Drift comparison resolves the + set first and compares in the client; the importer diffs in Python. The + queryability columns would buy is for a query no caller makes. + + The cost is real — a third mode is data rather than schema, so the DB will + not reject a typo'd mode key. That is the trade taken. + + NOT NULL with a `{}` default deliberately: a JSONB column otherwise has two + empty states (SQL NULL and JSON null) and code has to test for both. + """ + __tablename__ = "design_tokens" + __table_args__ = ( + # Partial unique: a name is unique among LIVE tokens in a system, so a + # trashed token doesn't block recreating the same name. Two live rows + # named `--fs-obsidian` in one system is a duplicate definition, and the + # cascade would pick between them arbitrarily. + Index( + "uq_token_per_design_system", "design_system_id", "name", + unique=True, postgresql_where=text("deleted_at IS NULL"), + ), + ) + + id: Mapped[int] = mapped_column(BigInteger, primary_key=True) + design_system_id: Mapped[int] = mapped_column( + BigInteger, ForeignKey("design_systems.id", ondelete="CASCADE"), index=True + ) + name: Mapped[str] = mapped_column(Text) + # Named for what it holds rather than the bare word `values`, which is + # reserved in SQL — the same reason `group_name` is not `group`. + value_by_mode: Mapped[dict] = mapped_column( + JSONB, nullable=False, default=dict, server_default=text("'{}'::jsonb") + ) + # `group` is a reserved word in SQL; `group_name` throughout — model, column + # and payload — so no layer has to remember which spelling it is on. + # Free text, not a CHECK enum: groupings are the design system's own + # vocabulary, and a whitelist would bake one install's kit into the schema. + group_name: Mapped[str | None] = mapped_column(Text, nullable=True) + purpose: Mapped[str | None] = mapped_column(Text, nullable=True) + # WHY this token is this value — a different question from `purpose`, which + # is what it is FOR. "Success equals Moss, aligned by design" is a rationale; + # "page bg, deepest surface" is a purpose. Rules carry the first routinely + # and a token row had nowhere to put it. + rationale: Mapped[str | None] = mapped_column(Text, nullable=True) + # Literal values this token should be used INSTEAD OF, e.g. ["#fff", + # "#ffffff"] on a text-on-action token. + # + # This is how a design system records the thing a prohibition was trying to + # say. "Pure white is never text" is the shadow of a positive fact — text is + # Parchment — and a system that stores what things ARE has no row for a ban. + # Recording the replacement keeps the check and makes it actionable: a + # finding can name what to write instead of merely objecting. + # + # It has to be DECLARED rather than inferred, because the superseded literal + # and the token's own value are usually different colours (#fff is not + # #E8E4D8). No value-matching rule could ever connect them. + # + # Consumed by the source lint (#2277), not by the drift panel: these + # literals live in component CSS, which the panel cannot see and says so. + supersedes: Mapped[list] = mapped_column( + JSONB, nullable=False, default=list, server_default=text("'[]'::jsonb") + ) + order_index: Mapped[int] = mapped_column(Integer, default=0, server_default="0") + + def to_dict(self) -> dict: + return { + "id": self.id, + "design_system_id": self.design_system_id, + "name": self.name, + "value_by_mode": self.value_by_mode or {}, + "group_name": self.group_name, + "purpose": self.purpose, + "rationale": self.rationale, + "supersedes": self.supersedes or [], + "order_index": self.order_index, + "created_at": self.created_at.isoformat() if self.created_at else None, + "updated_at": self.updated_at.isoformat() if self.updated_at else None, + } diff --git a/src/scribe/models/project.py b/src/scribe/models/project.py index 8ccacc5..d05a165 100644 --- a/src/scribe/models/project.py +++ b/src/scribe/models/project.py @@ -1,5 +1,5 @@ import enum -from sqlalchemy import ForeignKey, Integer, Text +from sqlalchemy import BigInteger, ForeignKey, Integer, Text from sqlalchemy.orm import Mapped, mapped_column from scribe.models import Base from scribe.models.base import TimestampMixin, SoftDeleteMixin @@ -21,6 +21,12 @@ class Project(Base, TimestampMixin, SoftDeleteMixin): goal: Mapped[str] = mapped_column(Text, default="") status: Mapped[str] = mapped_column(Text, default="active") color: Mapped[str | None] = mapped_column(Text, nullable=True) # hex color + # The design system this project's UI is built from, or NULL. NULL is the + # ordinary state, not a degraded one — most installs have no design system + # at all and nothing may assume one exists. + design_system_id: Mapped[int | None] = mapped_column( + BigInteger, ForeignKey("design_systems.id", ondelete="SET NULL"), nullable=True + ) def to_dict(self) -> dict: return { @@ -31,6 +37,7 @@ class Project(Base, TimestampMixin, SoftDeleteMixin): "goal": self.goal, "status": self.status, "color": self.color, + "design_system_id": self.design_system_id, "created_at": self.created_at.isoformat(), "updated_at": self.updated_at.isoformat(), } diff --git a/src/scribe/routes/design.py b/src/scribe/routes/design.py new file mode 100644 index 0000000..630e710 --- /dev/null +++ b/src/scribe/routes/design.py @@ -0,0 +1,29 @@ +"""Design-system surface — what the rulebook expects of the stylesheet. + +The client owns the other half of the comparison: it reads live token values from +the browser (see `utils/designTokens.ts`), which is the only place they exist +resolved. This endpoint supplies the claims to check them against. +""" +from quart import Blueprint, jsonify + +from scribe.auth import get_current_user_id, login_required +from scribe.services import design_rulebook_import as design_svc + +design_bp = Blueprint("design", __name__, url_prefix="/api/design") + + +@design_bp.get("/expectations") +@login_required +async def get_expectations(): + """Checkable claims from the rulebook this install designated as its design system. + + Returns `{"rulebook_id": int|null, "expectations": [...]}`. + + `rulebook_id: null` is the NORMAL case, not an error — an install that has + not designated a design rulebook has nothing to compare against, and the + client shows an explanatory empty state (rule #115). Distinguishing it from + "designated but empty" is why the id is returned alongside the list. + """ + uid = get_current_user_id() + result = await design_svc.design_expectations(uid) + return jsonify(result.as_dict()) diff --git a/src/scribe/routes/design_systems.py b/src/scribe/routes/design_systems.py new file mode 100644 index 0000000..4c0e015 --- /dev/null +++ b/src/scribe/routes/design_systems.py @@ -0,0 +1,260 @@ +"""Design system + token REST endpoints (milestone #254 step 4). + +Wraps `services/design_systems.py`, which owns the ACL and the cycle guard. +Design systems are owner-scoped top-level records rather than project-scoped +ones, so these do NOT nest under `/api/projects/` — `routes/rulebooks.py` is the +closer shape. + +Two failures this layer has to keep apart, which is why the service raises for +one and returns None for the other: + + - `DesignSystemCycle` -> 400 with its message. "That parent already inherits + from this system" is a correctable mistake and the caller needs to be told + which one they made. + - None -> 404, covering both "no such system" and "not yours". Conflating + those two IS the intent: distinguishing them would confirm the existence of + records the caller may not see. +""" +from quart import Blueprint, g, jsonify, request + +from scribe.auth import login_required +from scribe.services import design_systems as ds_svc +from scribe.services.design_systems import DesignSystemCycle + +design_systems_bp = Blueprint("design_systems", __name__, url_prefix="/api") + + +def _uid() -> int: + return g.user.id + + +def _not_found(what: str = "design system"): + return jsonify({"error": f"{what} not found"}), 404 + + +# ── Design systems ────────────────────────────────────────────────────── + +@design_systems_bp.get("/design-systems") +@login_required +async def list_design_systems(): + """The caller's design systems. An empty list is the ordinary state for an + install that has never made one, not an error.""" + rows = await ds_svc.list_design_systems(_uid()) + return jsonify({"design_systems": [s.to_dict() for s in rows]}) + + +@design_systems_bp.post("/design-systems") +@login_required +async def create_design_system(): + data = await request.get_json() or {} + title = (data.get("title") or "").strip() + if not title: + return jsonify({"error": "title is required"}), 400 + system = await ds_svc.create_design_system( + user_id=_uid(), + title=title, + description=data.get("description") or None, + guidance=data.get("guidance") or None, + parent_id=data.get("parent_id"), + ) + if system is None: + return jsonify({"error": "parent design system not found"}), 404 + return jsonify(system.to_dict()), 201 + + +@design_systems_bp.get("/design-systems/") +@login_required +async def get_design_system(design_system_id: int): + system = await ds_svc.get_design_system(_uid(), design_system_id) + if system is None: + return _not_found() + return jsonify(system.to_dict()) + + +@design_systems_bp.patch("/design-systems/") +@login_required +async def update_design_system(design_system_id: int): + data = await request.get_json() or {} + fields = { + k: v for k, v in data.items() if k in ("title", "description", "guidance") + } + # Presence, not truthiness: `{"parent_id": null}` means "make this a root", + # which a `if data.get("parent_id")` filter would silently drop. + if "parent_id" in data: + fields["parent_id"] = data["parent_id"] + try: + system = await ds_svc.update_design_system(_uid(), design_system_id, **fields) + except DesignSystemCycle as exc: + return jsonify({"error": str(exc)}), 400 + if system is None: + return _not_found() + return jsonify(system.to_dict()) + + +@design_systems_bp.delete("/design-systems/") +@login_required +async def delete_design_system(design_system_id: int): + if not await ds_svc.delete_design_system(_uid(), design_system_id): + return _not_found() + return "", 204 + + +@design_systems_bp.get("/design-systems//resolved") +@login_required +async def resolve_design_system(design_system_id: int): + """The EFFECTIVE token set — everything inherited, with this system's on top. + + Distinct from `/tokens` on purpose: that returns what this system CHANGES, + this returns what it ends up being. Both are real questions and answering + only one would make the other a client-side computation. + """ + resolved = await ds_svc.resolve_design_system(_uid(), design_system_id) + if resolved is None: + return _not_found() + return jsonify({ + "design_system_id": design_system_id, + "tokens": [t.to_dict() for t in resolved], + }) + + +@design_systems_bp.get("/design-systems//stylesheet") +@login_required +async def get_design_system_stylesheet(design_system_id: int): + """The master CSS sheet this design system generates. + + JSON by default (the UI wants the reuse report alongside the CSS); add + `?format=css` for the raw stylesheet as `text/css`, which is what a build + step or a `curl` wants. + + `?root=` overrides the base selector — a container-scoped preview cannot use + `:root`, so the generator takes it as a parameter. + """ + root = (request.args.get("root") or ":root").strip() or ":root" + result = await ds_svc.stylesheet_for_system(_uid(), design_system_id, root) + if result is None: + return _not_found() + if request.args.get("format") == "css": + return result["css"], 200, {"Content-Type": "text/css; charset=utf-8"} + return jsonify(result) + + +@design_systems_bp.get("/design-systems//snippet-check") +@login_required +async def check_snippets_against_system(design_system_id: int): + """Which recorded snippets disagree with this system's sheet. + + `?project_id=` narrows to one project; omit it to check every project, which + is usually right — a component recorded elsewhere still has to use the same + tags. + """ + project_id = request.args.get("project_id", type=int) or 0 + result = await ds_svc.check_snippets_against_system( + _uid(), design_system_id, project_id + ) + if result is None: + return _not_found() + return jsonify(result) + + +@design_systems_bp.post("/design-systems//import") +@login_required +async def import_design_system(design_system_id: int): + """Seed a design system from a rulebook's prose. + + `{"rulebook_id": N}` previews; add `"apply": true` to write. Preview is the + default because an import is a PROPOSAL — rulebooks are written + aspirationally and some of what they describe was never built. + + Existing token names are never overwritten, so a second run fills gaps and + reports the rest rather than undoing corrections. + """ + data = await request.get_json() or {} + rulebook_id = data.get("rulebook_id") + if not isinstance(rulebook_id, int) or rulebook_id <= 0: + return jsonify({"error": "rulebook_id is required"}), 400 + report = await ds_svc.import_from_rulebook( + _uid(), design_system_id, rulebook_id, apply=bool(data.get("apply")), + ) + if report is None: + return _not_found("design system or rulebook") + return jsonify(report) + + +# ── Tokens ────────────────────────────────────────────────────────────── + +@design_systems_bp.get("/design-systems//tokens") +@login_required +async def list_design_tokens(design_system_id: int): + """This system's OWN tokens — its override set, not its effective set.""" + if await ds_svc.get_design_system(_uid(), design_system_id) is None: + return _not_found() + rows = await ds_svc.list_tokens(_uid(), design_system_id) + return jsonify({"tokens": [t.to_dict() for t in rows]}) + + +@design_systems_bp.post("/design-systems//tokens") +@login_required +async def create_design_token(design_system_id: int): + data = await request.get_json() or {} + name = (data.get("name") or "").strip() + if not name: + return jsonify({"error": "name is required"}), 400 + token = await ds_svc.create_token( + user_id=_uid(), + design_system_id=design_system_id, + name=name, + value_by_mode=data.get("value_by_mode"), + group_name=data.get("group_name") or None, + purpose=data.get("purpose") or None, + rationale=data.get("rationale") or None, + supersedes=data.get("supersedes"), + order_index=data.get("order_index") or 0, + ) + if token is None: + return _not_found() + return jsonify(token.to_dict()), 201 + + +@design_systems_bp.patch("/design-tokens/") +@login_required +async def update_design_token(token_id: int): + data = await request.get_json() or {} + fields = { + k: v for k, v in data.items() + if k in ( + "name", "value_by_mode", "group_name", "purpose", "rationale", + "supersedes", "order_index", + ) + } + token = await ds_svc.update_token(_uid(), token_id, **fields) + if token is None: + return _not_found("design token") + return jsonify(token.to_dict()) + + +@design_systems_bp.delete("/design-tokens/") +@login_required +async def delete_design_token(token_id: int): + if not await ds_svc.delete_token(_uid(), token_id): + return _not_found("design token") + return "", 204 + + +# ── The project pointer ───────────────────────────────────────────────── + +@design_systems_bp.put("/projects//design-system") +@login_required +async def set_project_design_system(project_id: int): + """Point a project at a design system. `{"design_system_id": null}` clears it. + + PUT rather than PATCH: this sets one field to exactly what is sent, and + clearing it is a first-class outcome rather than an omission. + """ + data = await request.get_json() or {} + ok = await ds_svc.set_project_design_system( + _uid(), project_id, data.get("design_system_id") + ) + if not ok: + return _not_found("project or design system") + return jsonify({"project_id": project_id, + "design_system_id": data.get("design_system_id")}) diff --git a/src/scribe/services/access.py b/src/scribe/services/access.py index bb39d61..2a43374 100644 --- a/src/scribe/services/access.py +++ b/src/scribe/services/access.py @@ -12,11 +12,13 @@ import logging from sqlalchemy import or_, select from scribe.models import async_session +from scribe.models.design_system import DesignSystem from scribe.models.group import GroupMembership from scribe.models.note import Note from scribe.models.project import Project from scribe.models.share import NoteShare, ProjectShare from scribe.models.user import User +from scribe.services.design_cascade import ancestry logger = logging.getLogger(__name__) @@ -164,6 +166,88 @@ async def can_write_note(user_id: int, note_id: int) -> bool: return perm in ("editor", "admin", "owner") +# --------------------------------------------------------------------------- +# Design-system permissions +# --------------------------------------------------------------------------- + +async def get_design_system_permission( + user_id: int, design_system_id: int +) -> str | None: + """Effective permission on a design system, or None. + + Two ways in, and the asymmetry between them is the point: + + - **Owning it** grants "owner" — full read and write. + - **Reaching it through a project you can see** grants "viewer", and only + ever "viewer". Being an editor on a shared project must NOT confer the + right to rewrite the family system that project inherits from: one + project's collaborator would be editing tokens every other project in + the family resolves through. Editing a design system stays the owner's + act, and it is the same reasoning that keeps a rulebook owner-scoped. + + Reachability follows the parent chain UPWARD. Rendering a project's UI means + resolving its whole chain, so read access to a system implies read access to + its ancestors — otherwise a shared project would resolve to a truncated + cascade and silently render with the wrong values. + """ + async with async_session() as session: + system = await session.get(DesignSystem, design_system_id) + if system is None or system.deleted_at is not None: + return None + if system.owner_user_id == user_id: + return "owner" + + shared_project_ids = select(ProjectShare.project_id).where( + or_( + ProjectShare.shared_with_user_id == user_id, + ProjectShare.shared_with_group_id.in_(_my_group_ids(user_id)), + ) + ) + entry_points = set( + ( + await session.execute( + select(Project.design_system_id).where( + Project.design_system_id.is_not(None), + Project.deleted_at.is_(None), + or_( + Project.user_id == user_id, + Project.id.in_(shared_project_ids), + ), + ) + ) + ).scalars().all() + ) + if not entry_points: + return None + + # One narrow query for the whole forest's shape. Design systems are a + # handful of rows per install — a family and one per app — so walking + # from each entry point in memory beats a recursive CTE per check. + parents = dict( + ( + await session.execute( + select(DesignSystem.id, DesignSystem.parent_id).where( + DesignSystem.deleted_at.is_(None) + ) + ) + ).all() + ) + + for entry in entry_points: + if design_system_id in ancestry(entry, parents): + return "viewer" + return None + + +async def can_read_design_system(user_id: int, design_system_id: int) -> bool: + return (await get_design_system_permission(user_id, design_system_id)) is not None + + +async def can_write_design_system(user_id: int, design_system_id: int) -> bool: + perm = await get_design_system_permission(user_id, design_system_id) + return perm in ("editor", "admin", "owner") + + # --------------------------------------------------------------------------- # Set-based visibility (for LIST queries) # diff --git a/src/scribe/services/design_cascade.py b/src/scribe/services/design_cascade.py new file mode 100644 index 0000000..89ca254 --- /dev/null +++ b/src/scribe/services/design_cascade.py @@ -0,0 +1,251 @@ +"""The design-system cascade, as pure functions over already-loaded rows. + +Deliberately free of every database and service import — including +`services/access.py`, which needs `ancestry` to answer "can this caller read +this system?" and would otherwise form an import cycle with the service that +needs `access` back. A module that imports nothing can be imported by both. + +Being pure is also what makes the cascade rule testable without a database: +these take a plain `{id: parent_id}` map, so a test states the shape of a +hierarchy in one literal instead of building one. +""" +from collections.abc import Mapping, Sequence +from dataclasses import dataclass + +# The mode key that applies when no more specific one does. Emitted CSS puts it +# on the base selector and every other key on a mode selector, mirroring how a +# stylesheet is actually written: light on `:root`, dark layered over it. +BASE_MODE = "base" + + +def ancestry(system_id: int, parents: Mapping[int, int | None]) -> list[int]: + """The inheritance chain from `system_id` up to its root, nearest first. + + Includes `system_id` itself at index 0, because resolution wants + deepest-to-shallowest and the system being resolved is the deepest link. + + A system missing from `parents` terminates the chain rather than raising: a + parent whose row was soft-deleted or filtered out is a truncated chain, not + a failed request. + + The visited-set is defensive, not the primary guard — writes already refuse + to create a cycle (`would_cycle`). It is here because a loop introduced by a + direct DB edit or a future bug must degrade to a truncated chain instead of + spinning forever. Truncation shows up in the result; a hang shows up as an + outage. + """ + chain: list[int] = [] + seen: set[int] = set() + current: int | None = system_id + while current is not None and current not in seen: + seen.add(current) + chain.append(current) + current = parents.get(current) + return chain + + +def would_cycle( + system_id: int, + proposed_parent_id: int | None, + parents: Mapping[int, int | None], +) -> bool: + """Would making `proposed_parent_id` the parent of `system_id` close a loop? + + True when the proposed parent IS the system, or already inherits from it. + + The walk goes UP from the proposed parent, which is the cheap direction — + each system has at most one parent, so the chain is a line. Asking the + equivalent downward question ("is the proposed parent among my + descendants?") would mean searching a whole forest for the same answer. + + `proposed_parent_id=None` clears the parent and can never cycle. + """ + if proposed_parent_id is None: + return False + if proposed_parent_id == system_id: + return True + return system_id in ancestry(proposed_parent_id, parents) + + +# --------------------------------------------------------------------------- +# Resolution — flattening a chain into an effective token set +# --------------------------------------------------------------------------- + +@dataclass(frozen=True) +class Contribution: + """One system's offer for one token in one mode.""" + system_id: int + value: str + + +@dataclass(frozen=True) +class ResolvedToken: + """A token after the cascade, carrying the whole argument rather than the verdict. + + `contributions` holds every system that supplied a value, per mode, DEEPEST + FIRST — so `[0]` is the winner and `[1:]` are what it shadowed. Storing the + contest rather than a winner plus a separate provenance field means there is + nothing to keep in sync: "which system supplied this?" and "what did it + override?" are both reads of the same list, and they cannot disagree. + + Provenance is per MODE, not per token, because overriding is. A system that + deepens one accent for light backgrounds while leaving dark alone owns the + base value and inherits the dark one, and a token-level "overridden here" + flag would have to lie about one of them. + """ + name: str + contributions: dict[str, tuple[Contribution, ...]] + group_name: str | None + purpose: str | None + rationale: str | None + supersedes: tuple[str, ...] + order_index: int + + @property + def value_by_mode(self) -> dict[str, str]: + """The effective value for each mode — the winner of each contest.""" + return {mode: entries[0].value for mode, entries in self.contributions.items()} + + @property + def origin_by_mode(self) -> dict[str, int]: + """Which system supplied each mode's effective value.""" + return {mode: entries[0].system_id for mode, entries in self.contributions.items()} + + def value_for(self, mode: str) -> str | None: + """The value to render in `mode`, falling back to the base mode. + + This is the read rule the storage shape implies: a token that is not + mode-dependent carries only `base`, and asking it for "dark" must yield + the base value rather than nothing. + """ + entries = self.contributions.get(mode) or self.contributions.get(BASE_MODE) + return entries[0].value if entries else None + + def to_dict(self) -> dict: + """Payload shape for both surfaces — and it carries the SHADOWED entries. + + Serialising only the winner would throw away the provenance at the last + step, which is the one thing this type exists to preserve. `contributions` + is the audit trail; `value_by_mode` / `origin_by_mode` are alongside it so + a client renders without re-deriving anything, and cannot derive it + differently. + """ + return { + "name": self.name, + "group_name": self.group_name, + "purpose": self.purpose, + "rationale": self.rationale, + "supersedes": list(self.supersedes), + "order_index": self.order_index, + "value_by_mode": self.value_by_mode, + "origin_by_mode": self.origin_by_mode, + "contributions": { + mode: [ + {"system_id": c.system_id, "value": c.value} for c in entries + ] + for mode, entries in self.contributions.items() + }, + } + + def is_overridden_in(self, system_id: int) -> bool: + """Does `system_id` win any mode of this token AND shadow something? + + The distinction the UI needs: a token this system introduced is not an + override, and a token it merely inherits is not either. + """ + return any( + len(entries) > 1 and entries[0].system_id == system_id + for entries in self.contributions.values() + ) + + +def _sort_key(token: ResolvedToken) -> tuple: + # Ungrouped tokens sort last rather than first: a design system that has + # started grouping should read as its groups, with the not-yet-filed + # remainder at the end. + return (token.group_name is None, token.group_name or "", token.order_index, token.name) + + +def resolve_tokens( + system_id: int, + parents: Mapping[int, int | None], + tokens_by_system: Mapping[int, Sequence], +) -> list[ResolvedToken]: + """Flatten a system's inheritance chain into its effective token set. + + Walks from `system_id` up to the root and applies tokens by name, deepest + winning. That is the CSS cascade — precedence by name along a parent chain — + rather than an analogy to it, which is why the storage model and the + stylesheet model came out the same shape. + + `tokens_by_system` maps a system id to its own token rows. Any object with + `.name`, `.value_by_mode`, `.group_name`, `.purpose` and `.order_index` will + do, so a test can state a hierarchy in literals and the service can pass ORM + rows to the same function. + + The result includes tokens the system never mentions — inheriting one is + what puts it in the effective set. A system with no tokens of its own + resolves to its parent's set entire, which is the correct answer for an app + that has not departed from the family yet. + + Merging is per (name, MODE): a child that supplies only a dark value + overrides only dark and keeps inheriting base. Metadata (`group_name`, + `purpose`, `order_index`) cascades separately by the same deepest-wins rule, + since a child overriding a value routinely leaves the family's description + of what the token is FOR untouched — and inheriting it beats blanking it. + """ + chain = ancestry(system_id, parents) + + contributions: dict[str, dict[str, list[Contribution]]] = {} + metadata: dict[str, dict[str, object]] = {} + + # Deepest first, so the first contribution seen for a (name, mode) wins and + # every later one is a shadowed ancestor appended behind it. + for depth_system_id in chain: + for token in tokens_by_system.get(depth_system_id) or (): + per_mode = contributions.setdefault(token.name, {}) + for mode, value in (token.value_by_mode or {}).items(): + per_mode.setdefault(mode, []).append( + Contribution(system_id=depth_system_id, value=value) + ) + + meta = metadata.setdefault( + token.name, + { + "group_name": None, "purpose": None, "rationale": None, + "supersedes": None, "order_index": None, + }, + ) + for field in ("group_name", "purpose", "rationale"): + if meta[field] is None: + meta[field] = getattr(token, field, None) + # order_index alone treats 0 as UNSTATED rather than "first", + # because 0 is the column default. Reading it as a real value would + # let any child override drag its token to the top of the group and + # lose the family's ordering — a visible reshuffle in return for a + # change that only touched a colour. + if not meta["order_index"]: + meta["order_index"] = getattr(token, "order_index", 0) or None + # `supersedes` cascades on EMPTINESS, not on None: a child that + # overrides a colour and says nothing about which literals it + # replaces should keep the family's declaration, and an empty list + # is what "said nothing" looks like once the column is NOT NULL. + # A child that states its own list replaces the whole thing. + if not meta["supersedes"]: + meta["supersedes"] = tuple(getattr(token, "supersedes", None) or ()) or None + + resolved = [ + ResolvedToken( + name=name, + contributions={ + mode: tuple(entries) for mode, entries in per_mode.items() + }, + group_name=metadata[name]["group_name"], + purpose=metadata[name]["purpose"], + rationale=metadata[name]["rationale"], + supersedes=metadata[name]["supersedes"] or (), + order_index=metadata[name]["order_index"] or 0, + ) + for name, per_mode in contributions.items() + ] + return sorted(resolved, key=_sort_key) diff --git a/src/scribe/services/design_rulebook_import.py b/src/scribe/services/design_rulebook_import.py new file mode 100644 index 0000000..34573c5 --- /dev/null +++ b/src/scribe/services/design_rulebook_import.py @@ -0,0 +1,404 @@ +"""Design-system expectations — turning rulebook prose into checkable claims. + +Milestone #251 step 2. The drift panel compares what the design rulebook SAYS +against what the stylesheet and components actually DO. This module owns the +first half: reading a rulebook's rules and extracting the claims that can be +mechanically checked. + +WHY THIS LIVES SERVER-SIDE. The frontend has no test runner — `vue-tsc --noEmit` +is the entire check — and this is the one genuinely fiddly piece of the feature. +Extraction happens here where pytest can assert on it; the comparison itself is +set arithmetic and stays in the browser, where the live token values are. + +WHY NOT NLP. Rule statements are prose written for humans, and they should stay +that way — they are read by people far more often than they are parsed. So this +extracts only what is unambiguous in ANY prose: the hex colours and CSS custom +property names a rule mentions. Everything subtler (padding scales, type ramps) +needs a rule author to opt into a structured form, which is deliberately left for +when someone wants it rather than invented up front. + +RULE #115. Nothing here assumes a design rulebook exists, or that it is this +operator's. An install designates one; an install that hasn't gets an empty +result and a panel that explains itself. +""" +from __future__ import annotations + +import logging +import re +from dataclasses import dataclass, field + +from scribe.models.rulebook import Rule +from scribe.services.settings import get_setting + +logger = logging.getLogger(__name__) + +# Which rulebook describes this install's design system. A plain setting rather +# than a column: no migration, discoverable in the Settings UI (rule #25), and +# honest about being a per-install choice rather than a property of the rulebook. +DESIGN_RULEBOOK_SETTING = "design_rulebook_id" + +# `#abc` and `#aabbcc`, plus the 4/8-digit alpha forms. +_HEX = re.compile(r"#([0-9a-fA-F]{3,8})\b") + +# A custom-property name as written in prose, including the slash shorthand the +# rulebook uses: `--fs-radius-sm/md/lg/xl`, `--fs-obsidian/iron/slate/pewter`. +_TOKEN = re.compile(r"(--[a-zA-Z][\w-]*(?:/[\w-]+)*)") + +# Sentence-ish split. Rules use semicolons as hard breaks as often as periods. +_SENTENCE_SPLIT = re.compile(r"(?<=[.;])\s+|\n+") + +# Negation markers. Checked PER SENTENCE, which is the whole trick — see +# _extract_from_sentence. +_NEGATIONS = ("never", "not ", "no ", "avoid", "don't", "must not", "excluded") + + +@dataclass +class Expectation: + """One mechanically-checkable claim a rule makes.""" + + kind: str # "token" | "color" | "prohibited_color" + value: str # "--fs-obsidian" | "#14171a" + rule_id: int + rule_title: str + context: str # the sentence it came from, for showing your work + + def as_dict(self) -> dict: + return { + "kind": self.kind, + "value": self.value, + "rule_id": self.rule_id, + "rule_title": self.rule_title, + "context": self.context, + } + + +@dataclass +class ExpectationSet: + rulebook_id: int | None = None + expectations: list[Expectation] = field(default_factory=list) + + def as_dict(self) -> dict: + return { + "rulebook_id": self.rulebook_id, + "expectations": [e.as_dict() for e in self.expectations], + } + + +def normalize_hex(value: str) -> str | None: + """Fold a hex colour to a comparable form, or None if it isn't one. + + Load-bearing for the whole comparison: the rulebook writes `#FFFFFF` and the + code writes `#fff`, and those must compare equal or the single largest drift + finding (#2275) reads as zero. Expands 3-digit shorthand and lowercases. + + Alpha forms (4 and 8 digit) keep their alpha — `#fff` and `#ffff` are not the + same colour, and silently dropping the alpha would invent equality. + """ + match = _HEX.fullmatch(value.strip()) or _HEX.match(value.strip()) + if not match: + return None + digits = match.group(1).lower() + if len(digits) in (3, 4): + digits = "".join(c * 2 for c in digits) + if len(digits) not in (6, 8): + return None + return f"#{digits}" + + +def expand_token_shorthand(raw: str) -> list[str]: + """`--fs-radius-sm/md/lg/xl` -> the four names it stands for. + + The rulebook writes token families in a slash shorthand, and both forms it + uses expand correctly under one rule: take everything up to and including the + LAST hyphen of the first segment as the prefix, then append each alternative. + + --fs-radius-sm/md/lg/xl prefix `--fs-radius-` -> sm, md, lg, xl + --fs-obsidian/iron/slate prefix `--fs-` -> obsidian, iron, slate + --fs-dur-fast/base/slow prefix `--fs-dur-` -> fast, base, slow + + A name with no slash is returned as-is. + """ + if "/" not in raw: + return [raw] + head, *rest = raw.split("/") + cut = head.rfind("-") + if cut <= 1: # no hyphen beyond the leading `--` + return [head, *rest] + prefix = head[: cut + 1] + return [head, *[f"{prefix}{part}" for part in rest if part]] + + +def _is_negated(sentence: str) -> bool: + return any(marker in sentence.lower() for marker in _NEGATIONS) + + +def _extract_from_sentence(sentence: str, rule: Rule) -> list[Expectation]: + """Claims in ONE sentence, with negation scoped to that sentence. + + Sentence scope is what makes the prohibition detection usable. Rule 52 reads: + + "Text tokens: Parchment #E8E4D8 …, Vellum #C2BFB4 …, Ash #9C9A92 …. + Pure white #FFFFFF is NEVER used as text color." + + Three colours the palette REQUIRES and one it FORBIDS, in one statement. + Detecting negation across the whole statement would mark all four as + forbidden; detecting it per sentence gets all four right. + """ + out: list[Expectation] = [] + negated = _is_negated(sentence) + + for match in _HEX.finditer(sentence): + value = normalize_hex(match.group(0)) + if not value: + continue + out.append(Expectation( + kind="prohibited_color" if negated else "color", + value=value, + rule_id=int(rule.id), + rule_title=rule.title, + context=sentence.strip(), + )) + + # Token names are not negated in practice — a rule says which tokens should + # exist, never which must not — so they are recorded as expectations + # regardless. If that ever changes, it needs its own kind rather than + # borrowing the colour one. + for match in _TOKEN.finditer(sentence): + for name in expand_token_shorthand(match.group(1)): + out.append(Expectation( + kind="token", + value=name, + rule_id=int(rule.id), + rule_title=rule.title, + context=sentence.strip(), + )) + + return out + + +def extract_expectations(rules: list[Rule]) -> list[Expectation]: + """Every checkable claim across a set of rules, deduped on (kind, value). + + First occurrence wins so the reported rule is the one that introduced the + claim, which is usually the most specific place to send a reader. + """ + seen: set[tuple[str, str]] = set() + out: list[Expectation] = [] + for rule in rules: + text = " ".join(filter(None, [rule.statement or "", rule.how_to_apply or ""])) + for sentence in _SENTENCE_SPLIT.split(text): + if not sentence.strip(): + continue + for expectation in _extract_from_sentence(sentence, rule): + key = (expectation.kind, expectation.value) + if key in seen: + continue + seen.add(key) + out.append(expectation) + return out + + +async def get_design_rulebook_id(user_id: int) -> int | None: + """The rulebook this install designated as its design system, if any.""" + raw = (await get_setting(user_id, DESIGN_RULEBOOK_SETTING, "")).strip() + if not raw: + return None + try: + value = int(raw) + except (TypeError, ValueError): + return None + return value if value > 0 else None + + +async def design_expectations(user_id: int) -> ExpectationSet: + """Checkable claims from the designated design rulebook. + + Returns an empty set when no rulebook is designated — the normal case for + any install but the one that set it up (rule #115). The caller shows an + explanatory empty state rather than treating this as an error. + """ + rulebook_id = await get_design_rulebook_id(user_id) + if rulebook_id is None: + return ExpectationSet() + + from scribe.services import rulebooks as rulebooks_svc + + try: + rules = await rulebooks_svc.list_rules(user_id, rulebook_id=rulebook_id) + except Exception: + logger.warning("Design rulebook %s could not be read", rulebook_id, exc_info=True) + return ExpectationSet(rulebook_id=rulebook_id) + + return ExpectationSet(rulebook_id=rulebook_id, expectations=extract_expectations(rules)) + + +# --------------------------------------------------------------------------- +# Import — turning a rulebook into a PROPOSED design system (milestone #254 step 3) +# --------------------------------------------------------------------------- +# +# The extraction above answers "what claims does this rulebook make?", which is +# what a drift panel needs. Seeding a design system needs a different shape: +# tokens with names AND values, which the rulebook states in two separate +# places. Rule 51 names the colours ("Obsidian #14171A (page bg, deepest +# surface)"); rule 72 names the custom properties (`--fs-obsidian/iron/...`). +# Neither alone is a token. +# +# So the import joins them on the WORD: `--fs-obsidian` ends with `obsidian`, +# and a colour called Obsidian was declared elsewhere. That join is mechanical +# and it is the only reason an import produces something usable rather than 70 +# empty names. +# +# AN IMPORT IS A PROPOSAL, NOT A TRUTH. Rulebooks are written aspirationally and +# some of what they describe was never built. Every proposed token therefore +# carries the rule and sentence it came from, so a reviewer can check the claim +# rather than trust it. + +# "Obsidian #14171A (page bg, deepest surface)" — a capitalised name, a hex, and +# an optional parenthetical saying what it is for. +_NAMED_COLOUR = re.compile( + r"\b([A-Z][A-Za-z]*(?:\s+[A-Z][A-Za-z]*)?)\s+(#[0-9a-fA-F]{3,8})\b" + r"(?:\s*\(([^)]{0,80})\))?" +) + + +@dataclass +class ProposedToken: + """One token an import suggests, with the evidence for it. + + `value_by_mode` is empty when the rulebook names the token but states no + value this can read — radius steps, type sizes and durations are prose + (`Small 4px`), not hex, and inventing a parse for each would be guessing. + An empty value is the honest output: the name is real, the value needs a + human. Reporting how many landed that way is part of the result. + """ + + name: str + value_by_mode: dict[str, str] = field(default_factory=dict) + group_name: str | None = None + purpose: str | None = None + supersedes: list[str] = field(default_factory=list) + source_rule_id: int | None = None + source_rule_title: str = "" + source_context: str = "" + + def as_dict(self) -> dict: + return { + "name": self.name, + "value_by_mode": self.value_by_mode, + "group_name": self.group_name, + "purpose": self.purpose, + "supersedes": self.supersedes, + "source_rule_id": self.source_rule_id, + "source_rule_title": self.source_rule_title, + "source_context": self.source_context, + } + + +@dataclass +class _NamedColour: + value: str + purpose: str | None + rule_id: int + rule_title: str + context: str + + +def _group_from_name(name: str) -> str | None: + """`--fs-radius-sm` -> "radius"; `--fs-obsidian` -> None. + + A family name has a middle segment; a flat one does not. Structural rather + than a lookup table, so it works on a naming scheme this code has never + seen — which rule #115 requires, since the prefix is each install's own. + """ + parts = [p for p in name.lstrip("-").split("-") if p] + return parts[1] if len(parts) >= 3 else None + + +def _named_colours(rules: list[Rule]) -> dict[str, _NamedColour]: + """Every `Name #hex (purpose)` a rulebook declares, keyed by lowercased name. + + First declaration wins, matching `extract_expectations` — the rule that + introduces a colour is the one worth citing. + """ + out: dict[str, _NamedColour] = {} + for rule in rules: + text = " ".join(filter(None, [rule.statement or "", rule.how_to_apply or ""])) + for sentence in _SENTENCE_SPLIT.split(text): + if not sentence.strip() or _is_negated(sentence): + continue + for match in _NAMED_COLOUR.finditer(sentence): + label, raw_hex, purpose = match.groups() + value = normalize_hex(raw_hex) + key = label.strip().lower() + if not value or key in out: + continue + out[key] = _NamedColour( + value=value, + purpose=(purpose or "").strip() or None, + rule_id=int(rule.id), + rule_title=rule.title, + context=sentence.strip(), + ) + return out + + +def _prohibitions_by_rule(rules: list[Rule]) -> dict[int, list[str]]: + """Forbidden colours, grouped by the rule that forbids them.""" + out: dict[int, list[str]] = {} + for expectation in extract_expectations(rules): + if expectation.kind == "prohibited_color": + out.setdefault(expectation.rule_id, []).append(expectation.value) + return out + + +def propose_tokens(rules: list[Rule]) -> list[ProposedToken]: + """Turn a rulebook into the design system it is describing. + + One proposal per custom-property NAME the rulebook declares, valued from the + named colour whose word matches the token's last segment. + + Prohibitions attach as `supersedes` on the first token drawn from the SAME + rule that forbids them. Rule 52 declares Parchment/Vellum/Ash and forbids + pure white in one breath, so pure white becomes "write --fs-parchment + instead" — the positive form of what the rule was saying. Guessing which + token inherits the prohibition is acceptable precisely because this is a + proposal a human reviews; guessing silently would not be, which is why every + entry carries its source sentence. + """ + colours = _named_colours(rules) + prohibited = _prohibitions_by_rule(rules) + claimed_prohibitions: set[int] = set() + + proposals: list[ProposedToken] = [] + seen: set[str] = set() + + for expectation in extract_expectations(rules): + if expectation.kind != "token" or expectation.value in seen: + continue + seen.add(expectation.value) + + suffix = expectation.value.rsplit("-", 1)[-1].lower() + colour = colours.get(suffix) + + proposal = ProposedToken( + name=expectation.value, + value_by_mode={"base": colour.value} if colour else {}, + group_name=_group_from_name(expectation.value), + purpose=colour.purpose if colour else None, + source_rule_id=colour.rule_id if colour else expectation.rule_id, + source_rule_title=colour.rule_title if colour else expectation.rule_title, + source_context=colour.context if colour else expectation.context, + ) + + # The prohibition rides on the first token that rule supplied a value + # for — its primary. Attaching it to every token of that rule would + # claim the rulebook said something it didn't. + if colour and colour.rule_id not in claimed_prohibitions: + forbidden = prohibited.get(colour.rule_id) + if forbidden: + proposal.supersedes = list(forbidden) + claimed_prohibitions.add(colour.rule_id) + + proposals.append(proposal) + + return proposals diff --git a/src/scribe/services/design_stylesheet.py b/src/scribe/services/design_stylesheet.py new file mode 100644 index 0000000..6ead872 --- /dev/null +++ b/src/scribe/services/design_stylesheet.py @@ -0,0 +1,301 @@ +"""Render a design system's resolved tokens as its master CSS sheet. + +Pure, like `design_cascade` and for the same reasons: no database import, so the +rendering rule is testable without one and the preview surface can reuse it. + +WHAT THIS SHEET IS, AND DELIBERATELY IS NOT +------------------------------------------- +It declares **purpose tokens only** — custom properties, grouped by what they +mean. It contains no rules for elements or classes: no `.btn-primary`, no +`table`, no `input`. + +That is the point rather than a limitation. A sheet that styled every element +would restate the same handful of values once per element and grow with the UI; +a sheet of purpose-named values states each once and is reused. Components — +buttons, tables, input schemes — live as SNIPPETS that reference these names and +carry prose about the idea, which is a surface that already exists and already +has recall, locations, drift checks and merge. + +So the division is: this sheet says what the values MEAN; snippets say what +things LOOK LIKE, in terms of those values. A token named after an element +(`--fs-button-bg`) is the smell that the two have been mixed — it multiplies +with every new element, where a purpose name (`--fs-action-primary`) is reused. + +SAFETY +------ +Values are interpolated into a stylesheet, and design systems are shareable +records (rule #47). A value of `red; } body { display: none` in a system someone +shared with you would otherwise inject CSS into your page. Everything rendered +here is validated or dropped — never escaped-and-hoped. +""" +from __future__ import annotations + +import re +from collections.abc import Sequence + +BASE_MODE = "base" + +# A custom property name, strictly. Anything else is dropped rather than +# sanitised: a name is an identifier, and a "cleaned up" identifier is a +# different token than the one the operator recorded. +_VALID_NAME = re.compile(r"^--[A-Za-z0-9_-]+$") + +# Characters that would end the declaration, open a block, start an at-rule, or +# begin a tag. A value containing any of them is not a value. +_UNSAFE_VALUE = re.compile(r"[{};@<>]|\*/|/\*|\n|\r") + + +def is_valid_token_name(name: str) -> bool: + return bool(_VALID_NAME.match((name or "").strip())) + + +def safe_value(value: str) -> str | None: + """A CSS value that cannot escape its declaration, or None. + + Rejects rather than strips. A partially-sanitised value is a value the + operator did not write, and silently rendering a different colour than the + record holds is worse than rendering none — the sheet's whole claim is that + it IS the record. + """ + candidate = (value or "").strip() + if not candidate or _UNSAFE_VALUE.search(candidate): + return None + return candidate + + +def safe_comment(text: str) -> str: + """Comment text that cannot close its comment or break the line.""" + return re.sub(r"\*/|/\*|[\n\r]", " ", (text or "")).strip() + + +def selector_for_mode(mode: str, root_selector: str = ":root") -> str: + """Which selector a mode's declarations belong under. + + `base` gets the caller's root selector; every other mode gets a bare + attribute selector, matching the convention already in the codebase. + + The root selector is a PARAMETER because a container-scoped preview cannot + use `:root` — #251 recorded that mode scoping is one-way (light lives on + `:root`, dark layers over it), so a generator that hardcoded `:root` could + not serve a preview at all. + """ + if mode == BASE_MODE: + return root_selector + safe_mode = re.sub(r"[^A-Za-z0-9_-]", "", mode) + return f'[data-theme="{safe_mode}"]' if safe_mode else root_selector + + +def _grouped(tokens: Sequence) -> list[tuple[str | None, list]]: + """Tokens by group, preserving the order they arrive in (already sorted).""" + groups: dict[str | None, list] = {} + for token in tokens: + groups.setdefault(getattr(token, "group_name", None), []).append(token) + return list(groups.items()) + + +def _modes_present(tokens: Sequence) -> list[str]: + """Every mode any token declares, base first then the rest alphabetically.""" + modes = { + mode + for token in tokens + for mode in (getattr(token, "value_by_mode", None) or {}) + } + rest = sorted(modes - {BASE_MODE}) + return ([BASE_MODE] if BASE_MODE in modes else []) + rest + + +def render_stylesheet( + tokens: Sequence, + *, + root_selector: str = ":root", + title: str = "", + design_system_id: int | None = None, +) -> str: + """The master sheet for a resolved token set. + + One block per mode. Within a block, only the tokens that declare a value for + that mode — so a mode block is an override layer, exactly as the storage + model has it. + + Tokens with no value at all are emitted as COMMENTED-OUT declarations in + their group, not dropped. The rulebook named them, so their absence is a + finding, and a commented line puts that finding where the reader is already + looking. + """ + header = [ + "/*", + f" * {safe_comment(title) or 'Design system'} — generated stylesheet", + ] + if design_system_id is not None: + header.append(f" * Source: design system {int(design_system_id)}.") + header += [ + " *", + " * Purpose tokens only. This sheet declares what values MEAN; it styles", + " * no elements. Buttons, tables and input schemes live as snippets that", + " * reference these names, so each value is stated once and reused rather", + " * than restated per element.", + " *", + " * Generated — edit the design system, not this file.", + " */", + "", + ] + + lines = list(header) + valueless = [ + t for t in tokens + if is_valid_token_name(getattr(t, "name", "")) + and not (getattr(t, "value_by_mode", None) or {}) + ] + + for mode in _modes_present(tokens): + block: list[str] = [] + for group, group_tokens in _grouped(tokens): + entries: list[str] = [] + for token in group_tokens: + name = (getattr(token, "name", "") or "").strip() + if not is_valid_token_name(name): + continue + raw = (getattr(token, "value_by_mode", None) or {}).get(mode) + if raw is None: + continue + value = safe_value(str(raw)) + if value is None: + entries.append( + f" /* {name}: value rejected — not a safe CSS value */" + ) + continue + # Purpose first — what the token is FOR is what a reader of the + # stylesheet needs. Rationale is the fallback so a token that + # only carries the why still says something. + purpose = safe_comment( + getattr(token, "purpose", "") + or getattr(token, "rationale", "") + or "" + ) + comment = f" /* {purpose} */" if purpose and mode == BASE_MODE else "" + entries.append(f" {name}: {value};{comment}") + if entries: + if group: + block.append(f" /* {safe_comment(group)} */") + block.extend(entries) + block.append("") + + # Declared-but-valueless tokens belong to the base layer: they have no + # mode to sit under, and repeating them per mode would triple the noise. + if mode == BASE_MODE and valueless: + block.append(" /* Declared by the design system, no value set yet: */") + block.extend(f" /* {t.name}: ; */" for t in valueless) + block.append("") + + if not block: + continue + lines.append(f"{selector_for_mode(mode, root_selector)} {{") + lines.extend(block[:-1] if block[-1] == "" else block) + lines.append("}") + lines.append("") + + return "\n".join(lines).rstrip() + "\n" + + +def duplicate_values(tokens: Sequence) -> dict[str, list[str]]: + """Values declared by more than one token, mapped to the names declaring them. + + Two names for one value are either a deliberate alias or the same idea + recorded twice — the token-level form of the duplicated-definition shape. + Reported rather than refused: a design system legitimately aligns colours on + purpose ("Success = Moss, by design"), and a generator that rejected that + would be wrong about the operator's intent. + + Compares the BASE value only. Two tokens agreeing in one mode and diverging + in another are not duplicates of each other — they are a near-miss, which is + a different and less interesting finding. + """ + by_value: dict[str, list[str]] = {} + for token in tokens: + name = getattr(token, "name", "") + base = (getattr(token, "value_by_mode", None) or {}).get(BASE_MODE) + if not name or not base: + continue + by_value.setdefault(str(base).strip().lower(), []).append(name) + return {value: names for value, names in by_value.items() if len(names) > 1} + + +# --------------------------------------------------------------------------- +# Reading a sheet from the other side: does this code use it correctly? +# --------------------------------------------------------------------------- +# +# "The snippets use the tags from the sheet" is a verifiable relation, and +# nothing checked it before. Three questions, each a different failure: +# +# var(--x) where no --x exists -> renders as NOTHING. No error, no test +# failure, no visual clue beyond the thing +# silently not being styled. +# a superseded literal in code -> the value the sheet said to stop writing, +# and the sheet knows what to write instead. +# --x: declared inside a snippet -> a component minting its own token is the +# bloat a shared sheet exists to prevent. +# +# The first is not hypothetical: `--color-accent` was used throughout a new view +# in this codebase and does not exist. + +_VAR_REFERENCE = re.compile(r"var\(\s*(--[A-Za-z0-9_-]+)") +_LOCAL_DEFINITION = re.compile(r"(? set[str]: + """Every custom property the code reads through `var()`.""" + return set(_VAR_REFERENCE.findall(code or "")) + + +def defined_tokens(code: str) -> set[str]: + """Every custom property the code declares itself. + + Excludes names it also reads: `--x: var(--x, fallback)` is a redeclaration + of something the sheet owns, which the unknown-reference check already + covers more precisely. + """ + return set(_LOCAL_DEFINITION.findall(code or "")) - referenced_tokens(code or "") + + +def _literal_pattern(literal: str) -> re.Pattern: + """Match a literal value without matching a longer one that contains it. + + `#fff` must not match inside `#ffffff` — they are different colours, and a + finding that fired on the wrong one would send someone to change code that + was already correct. + """ + escaped = re.escape(literal) + lead = r"(? dict: + """What this code gets wrong about that token set. + + `tokens` is any sequence with `.name`, `.value_by_mode` and `.supersedes` — + resolved tokens, or stored ones. + + Reports rather than scores. Every finding here has a legitimate exception: + a snippet may target a system it isn't being checked against, and a literal + may be deliberate in a context the token doesn't cover. What it removes is + the SILENCE — all three currently fail with no signal at all. + """ + known = {getattr(t, "name", "") for t in tokens} + referenced = referenced_tokens(code) + + superseded: list[dict] = [] + for token in tokens: + for literal in getattr(token, "supersedes", None) or (): + if _literal_pattern(str(literal)).search(code or ""): + superseded.append({ + "literal": literal, + "use_instead": getattr(token, "name", ""), + }) + + return { + "used": sorted(referenced & known), + "unknown": sorted(referenced - known), + "superseded_literals": superseded, + "local_definitions": sorted(defined_tokens(code)), + } diff --git a/src/scribe/services/design_systems.py b/src/scribe/services/design_systems.py new file mode 100644 index 0000000..de6a1af --- /dev/null +++ b/src/scribe/services/design_systems.py @@ -0,0 +1,501 @@ +"""Design system + token persistence, and the guard on the parent chain. + +Access goes through `services/access.py` (rule #78), where reaching a system via +a project you can see grants READ but never write — see +`get_design_system_permission` for why that asymmetry exists. + +The cascade itself lives in `services/design_cascade.py` as pure functions. This +module is the part that needs a database: loading the shape of the hierarchy, +refusing writes that would break it, and storing tokens. + +Nothing here seeds or implies a default system. An install with no design +systems is an ordinary install, and every caller must handle an empty list as +the normal case rather than a missing prerequisite. +""" +import logging +from datetime import datetime, timezone + +from sqlalchemy import select + +from scribe.models import async_session +from scribe.models.design_system import DesignSystem, DesignToken +from scribe.models.project import Project +from scribe.services import access +from scribe.services.design_stylesheet import ( + check_code_against_tokens, + duplicate_values, + render_stylesheet, +) +from scribe.services.design_cascade import ( + ResolvedToken, + ancestry, + resolve_tokens, + would_cycle, +) + +logger = logging.getLogger(__name__) + + +class DesignSystemCycle(ValueError): + """A parent assignment that would close an inheritance loop. + + Raised rather than returned as None because the two outcomes need different + answers: None already means "not found, or not yours", and a caller that + conflated them would show "no such design system" for what is really "that + parent is one of its own descendants". Routes map this to a 400. + """ + + +async def _parent_map(session, owner_user_id: int) -> dict[int, int | None]: + """`{id: parent_id}` for one owner's live systems — the hierarchy's shape. + + Scoped to the OWNER of the systems, not the caller, and the distinction is + load-bearing on the read path: a caller reading through a shared project + does not own any link in the chain, and a caller-scoped map would hand them + an empty forest and a cascade truncated to one system. They would get a page + that renders with plausible wrong values and no error anywhere. + + Owner-scoping is also the constraint on parenting: a chain may only be built + from systems its owner controls, or someone else's delete or re-parent would + silently restyle your app. + """ + rows = ( + await session.execute( + select(DesignSystem.id, DesignSystem.parent_id).where( + DesignSystem.owner_user_id == owner_user_id, + DesignSystem.deleted_at.is_(None), + ) + ) + ).all() + return dict(rows) + + +# --- design systems --------------------------------------------------------- + +async def create_design_system( + user_id: int, + title: str, + description: str | None = None, + guidance: str | None = None, + parent_id: int | None = None, +) -> DesignSystem | None: + """Create a system, with or without a parent. + + Returns None when `parent_id` names a system the caller may not write — + which, per the ACL, means one they do not own. + """ + if parent_id is not None and not await access.can_write_design_system( + user_id, parent_id + ): + return None + async with async_session() as session: + system = DesignSystem( + owner_user_id=user_id, + title=title.strip(), + description=description, + guidance=guidance, + parent_id=parent_id, + ) + session.add(system) + await session.commit() + await session.refresh(system) + return system + + +async def get_design_system(user_id: int, design_system_id: int) -> DesignSystem | None: + async with async_session() as session: + system = await session.get(DesignSystem, design_system_id) + if system is None or system.deleted_at is not None: + return None + if not await access.can_read_design_system(user_id, design_system_id): + return None + return system + + +async def list_design_systems(user_id: int) -> list[DesignSystem]: + """The caller's own systems, ordered by title. Empty is normal.""" + async with async_session() as session: + rows = await session.execute( + select(DesignSystem) + .where( + DesignSystem.owner_user_id == user_id, + DesignSystem.deleted_at.is_(None), + ) + .order_by(DesignSystem.title) + ) + return list(rows.scalars().all()) + + +async def update_design_system( + user_id: int, design_system_id: int, **fields: object +) -> DesignSystem | None: + """Partial update. Raises DesignSystemCycle if `parent_id` would loop. + + `parent_id` is handled apart from the other fields because None is a + meaningful value for it — "make this a root" — where for every other field + None means "leave alone". Callers signal it by passing the key at all. + """ + if not await access.can_write_design_system(user_id, design_system_id): + return None + async with async_session() as session: + system = await session.get(DesignSystem, design_system_id) + if system is None or system.deleted_at is not None: + return None + + if "parent_id" in fields: + parent_id = fields.pop("parent_id") + if parent_id is not None: + parent_id = int(parent_id) + if not await access.can_write_design_system(user_id, parent_id): + return None + if would_cycle( + design_system_id, + parent_id, + await _parent_map(session, system.owner_user_id), + ): + raise DesignSystemCycle( + f"Design system {design_system_id} cannot inherit from " + f"{parent_id}: that system already inherits from it." + ) + system.parent_id = parent_id + + for key, value in fields.items(): + if key in ("title", "description", "guidance") and value is not None: + setattr(system, key, value) + system.updated_at = datetime.now(timezone.utc) + await session.commit() + await session.refresh(system) + return system + + +async def delete_design_system(user_id: int, design_system_id: int) -> bool: + """Soft-delete a system. Children survive as roots (the FK is SET NULL).""" + if not await access.can_write_design_system(user_id, design_system_id): + return False + async with async_session() as session: + system = await session.get(DesignSystem, design_system_id) + if system is None or system.deleted_at is not None: + return False + system.deleted_at = datetime.now(timezone.utc) + await session.commit() + return True + + +# --- tokens ----------------------------------------------------------------- + +async def create_token( + user_id: int, + design_system_id: int, + name: str, + value_by_mode: dict | None = None, + group_name: str | None = None, + purpose: str | None = None, + rationale: str | None = None, + supersedes: list | None = None, + order_index: int = 0, +) -> DesignToken | None: + if not await access.can_write_design_system(user_id, design_system_id): + return None + async with async_session() as session: + token = DesignToken( + design_system_id=design_system_id, + name=name.strip(), + # `or {}` and not the argument as given: the column is NOT NULL so + # that absence has exactly one spelling. Passing None here would + # otherwise store JSON null and reintroduce the second empty state. + value_by_mode=value_by_mode or {}, + group_name=group_name, + purpose=purpose, + rationale=rationale, + # `or []` for the same reason as value_by_mode above: the column is + # NOT NULL so absence has one spelling, and None would store JSON + # null instead of an empty array. + supersedes=supersedes or [], + order_index=order_index, + ) + session.add(token) + await session.commit() + await session.refresh(token) + return token + + +async def list_tokens(user_id: int, design_system_id: int) -> list[DesignToken]: + """One system's OWN tokens — its override set, not its effective set. + + Resolving the chain is step 2's job; this deliberately answers the narrower + question ("what does this system change?") that the model exists to make + free. + """ + if not await access.can_read_design_system(user_id, design_system_id): + return [] + async with async_session() as session: + rows = await session.execute( + select(DesignToken) + .where( + DesignToken.design_system_id == design_system_id, + DesignToken.deleted_at.is_(None), + ) + .order_by(DesignToken.order_index.asc(), DesignToken.name.asc()) + ) + return list(rows.scalars().all()) + + +async def resolve_design_system( + user_id: int, design_system_id: int +) -> list[ResolvedToken] | None: + """A system's EFFECTIVE token set — everything it inherits, with its own on top. + + None when the caller may not read the system; an empty list when the chain + genuinely holds no tokens, which is an ordinary state for a system that has + just been created. + + Two queries regardless of how deep the chain runs: one for the hierarchy's + shape, one for every token in it. The flattening itself is + `design_cascade.resolve_tokens` — pure, so the cascade rule is tested + without a database and this function is only the loading. + """ + if not await access.can_read_design_system(user_id, design_system_id): + return None + async with async_session() as session: + system = await session.get(DesignSystem, design_system_id) + if system is None or system.deleted_at is not None: + return None + + parents = await _parent_map(session, system.owner_user_id) + chain = ancestry(design_system_id, parents) + + rows = ( + await session.execute( + select(DesignToken) + .where( + DesignToken.design_system_id.in_(chain), + DesignToken.deleted_at.is_(None), + ) + .order_by(DesignToken.order_index.asc(), DesignToken.name.asc()) + ) + ).scalars().all() + + tokens_by_system: dict[int, list[DesignToken]] = {} + for token in rows: + tokens_by_system.setdefault(token.design_system_id, []).append(token) + return resolve_tokens(design_system_id, parents, tokens_by_system) + + +async def update_token( + user_id: int, token_id: int, **fields: object +) -> DesignToken | None: + allowed = { + "name", "value_by_mode", "group_name", "purpose", "rationale", + "supersedes", "order_index", + } + async with async_session() as session: + token = await session.get(DesignToken, token_id) + if token is None or token.deleted_at is not None: + return None + if not await access.can_write_design_system(user_id, token.design_system_id): + return None + for key, value in fields.items(): + if key in allowed and value is not None: + setattr(token, key, value) + token.updated_at = datetime.now(timezone.utc) + await session.commit() + await session.refresh(token) + return token + + +async def delete_token(user_id: int, token_id: int) -> bool: + async with async_session() as session: + token = await session.get(DesignToken, token_id) + if token is None or token.deleted_at is not None: + return False + if not await access.can_write_design_system(user_id, token.design_system_id): + return False + token.deleted_at = datetime.now(timezone.utc) + await session.commit() + return True + + +# --- the project pointer ---------------------------------------------------- + +async def set_project_design_system( + user_id: int, project_id: int, design_system_id: int | None +) -> bool: + """Point a project at a design system, or at nothing (None clears it). + + Needs write on the project and READ on the system: pointing at a system is + consuming it, not changing it, so a system shared with you through another + project is a legitimate choice here. + """ + if not await access.can_write_project(user_id, project_id): + return False + if design_system_id is not None and not await access.can_read_design_system( + user_id, design_system_id + ): + return False + async with async_session() as session: + project = await session.get(Project, project_id) + if project is None or project.deleted_at is not None: + return False + project.design_system_id = design_system_id + project.updated_at = datetime.now(timezone.utc) + await session.commit() + return True + + +# --- import from a rulebook ------------------------------------------------- + +async def import_from_rulebook( + user_id: int, + design_system_id: int, + rulebook_id: int, + apply: bool = False, +) -> dict | None: + """Propose (and optionally create) tokens for a system from a rulebook. + + Returns None when the caller may not write the system or read the rulebook. + Otherwise a report with three lists, and the split between them is the whole + point of running it with `apply=False` first: + + proposed — everything the rulebook describes, each entry carrying the + rule and sentence it came from + created — what was actually written (empty unless `apply`) + skipped — proposals whose name the system already defines + + **Existing tokens are never overwritten.** An import is a proposal built by + reading prose; a value already in the record was put there deliberately, and + a re-run must not undo an operator's correction. That also makes the whole + operation safe to repeat — it fills gaps and reports the rest. + + Tokens with no value are still created when `apply` is set. The rulebook + names them, so their absence from the system is itself a finding, and a + named token with a blank value says "this exists and needs deciding" where + silence says nothing at all. + """ + if not await access.can_write_design_system(user_id, design_system_id): + return None + + from scribe.services import rulebooks as rulebooks_svc + from scribe.services.design_rulebook_import import propose_tokens + + rules = await rulebooks_svc.list_rules(user_id, rulebook_id=rulebook_id) + if not rules: + return {"rulebook_id": rulebook_id, "proposed": [], "created": [], "skipped": []} + + proposals = propose_tokens(rules) + existing = {t.name for t in await list_tokens(user_id, design_system_id)} + + created: list[dict] = [] + skipped: list[str] = [] + for index, proposal in enumerate(proposals): + if proposal.name in existing: + skipped.append(proposal.name) + continue + if not apply: + continue + token = await create_token( + user_id, + design_system_id=design_system_id, + name=proposal.name, + value_by_mode=proposal.value_by_mode, + group_name=proposal.group_name, + purpose=proposal.purpose, + supersedes=proposal.supersedes, + order_index=index, + ) + if token is not None: + created.append(token.to_dict()) + + return { + "rulebook_id": rulebook_id, + "proposed": [p.as_dict() for p in proposals], + "created": created, + "skipped": skipped, + } + + +# --- the master sheet ------------------------------------------------------- + +async def stylesheet_for_system( + user_id: int, design_system_id: int, root_selector: str = ":root" +) -> dict | None: + """The master CSS sheet a design system generates, plus its reuse report. + + Purpose tokens only — the sheet styles no elements. See + `services/design_stylesheet.py` for why that split is the design rather than + a shortcut. + + Returns None if the caller may not read the system. The `duplicates` half is + advisory: two tokens sharing a value are either a deliberate alias or one + idea recorded twice, and only the operator knows which. + """ + resolved = await resolve_design_system(user_id, design_system_id) + if resolved is None: + return None + system = await get_design_system(user_id, design_system_id) + + return { + "design_system_id": design_system_id, + "css": render_stylesheet( + resolved, + root_selector=root_selector, + title=system.title if system else "", + design_system_id=design_system_id, + ), + "token_count": len(resolved), + "valueless": [t.name for t in resolved if not t.value_by_mode], + "duplicates": duplicate_values(resolved), + } + + +async def check_snippets_against_system( + user_id: int, design_system_id: int, project_id: int = 0 +) -> dict | None: + """Which recorded snippets disagree with this design system's sheet. + + The relation the operator named — "the snippets use the tags from the sheet" + — turned into a check. For each snippet: `var(--x)` references with no such + token, literals the sheet says to stop writing, and custom properties the + snippet mints for itself instead of using shared ones. + + Snippets with nothing to report are omitted entirely. A list of everything + that is fine is a list nobody reads twice. + """ + resolved = await resolve_design_system(user_id, design_system_id) + if resolved is None: + return None + + from scribe.services import snippets as snippets_svc + + # `list_snippets` returns (rows, total) and caps limit at 100; `project_id` + # must be None — not 0 — to reach across every project, since 0 would filter + # to a project with that id. + rows, _total = await snippets_svc.list_snippets( + user_id=user_id, project_id=project_id or None, limit=100, + ) + + findings: list[dict] = [] + for row in rows: + snippet_id = row.get("id") + if snippet_id is None: + continue + # The list rows carry a preview, not the code. The check has to read the + # whole body or it would report on a truncation. + note = await snippets_svc.get_snippet(user_id=user_id, snippet_id=int(snippet_id)) + if note is None: + continue + report = check_code_against_tokens(note.body or "", resolved) + if not ( + report["unknown"] or report["superseded_literals"] or report["local_definitions"] + ): + continue + findings.append({ + "snippet_id": int(snippet_id), + "title": note.title or "", + **report, + }) + + return { + "design_system_id": design_system_id, + "checked": len(rows), + "findings": findings, + } diff --git a/src/scribe/services/logging.py b/src/scribe/services/logging.py index 228e2ee..3123920 100644 --- a/src/scribe/services/logging.py +++ b/src/scribe/services/logging.py @@ -89,6 +89,38 @@ async def log_error( await session.commit() +def parse_filter_datetime(value: str | None, *, end_of_day: bool = False) -> datetime | None: + """An ISO date/datetime from a query string as an AWARE UTC datetime. + + The admin log filters arrive as raw `request.args` strings and used to be + compared straight against `AppLog.created_at`. asyncpg binds a str as + VARCHAR and Postgres has no `timestamptz >= text` operator, so supplying + either date filter raised — the same defect as #1727 in notifications, in a + second place (#2254). + + Returns None for anything unparseable: a malformed filter should narrow + nothing rather than 500 the log viewer. + + `end_of_day` matters for the upper bound. A bare "2026-07-30" parses to + midnight, so `created_at <= that` would exclude the whole of the day the + user asked for — the one day they most likely wanted. With this flag a + date-only value is pushed to the last microsecond of that day; a value that + already carries a time is left exactly as given. + """ + raw = (value or "").strip() + if not raw: + return None + try: + parsed = datetime.fromisoformat(raw) + except ValueError: + return None + if end_of_day and len(raw) == 10: # date-only, no time component + parsed = parsed.replace(hour=23, minute=59, second=59, microsecond=999999) + if parsed.tzinfo is None: + parsed = parsed.replace(tzinfo=timezone.utc) + return parsed + + async def get_logs( category: str | None = None, user_id: int | None = None, @@ -118,12 +150,14 @@ async def get_logs( ) query = query.where(search_filter) count_query = count_query.where(search_filter) - if date_from: - query = query.where(AppLog.created_at >= date_from) - count_query = count_query.where(AppLog.created_at >= date_from) - if date_to: - query = query.where(AppLog.created_at <= date_to) - count_query = count_query.where(AppLog.created_at <= date_to) + start = parse_filter_datetime(date_from) + end = parse_filter_datetime(date_to, end_of_day=True) + if start: + query = query.where(AppLog.created_at >= start) + count_query = count_query.where(AppLog.created_at >= start) + if end: + query = query.where(AppLog.created_at <= end) + count_query = count_query.where(AppLog.created_at <= end) total = (await session.execute(count_query)).scalar() or 0 diff --git a/src/scribe/services/notifications.py b/src/scribe/services/notifications.py index ae84c9f..cd171d6 100644 --- a/src/scribe/services/notifications.py +++ b/src/scribe/services/notifications.py @@ -3,7 +3,7 @@ import asyncio import json import logging -from datetime import date, datetime, timezone +from datetime import date, datetime, time, timezone from sqlalchemy import func, select, text @@ -149,13 +149,25 @@ async def send_invitation_email(email: str, invite_url: str, invited_by_username await send_email(email, "Fabled Scribe - You're Invited!", _email_html("You're Invited!", body)) +def utc_day_start(day: date) -> datetime: + """Midnight UTC on `day`, as an AWARE datetime — the reminder dedup window. + + Deliberately a `datetime` and not the bare `date` (#1727). Comparing a + `timestamptz` column against a `date` does work, via an implicit cast — but + Postgres resolves that cast in the SESSION's TimeZone, so the window would + move with a server setting nobody remembers is load-bearing. An aware UTC + datetime says what it means and compares the same way everywhere. + """ + return datetime.combine(day, time.min, tzinfo=timezone.utc) + + async def check_due_tasks() -> None: """Check for tasks due today and send reminder emails.""" if not await is_smtp_configured(): return today = date.today() - today_str = today.isoformat() + window_start = utc_day_start(today) async with async_session() as session: # Find tasks due today or overdue, not done @@ -188,12 +200,18 @@ async def check_due_tasks() -> None: if not email: continue - # Dedup: check if we already sent a reminder today + # Dedup: check if we already sent a reminder today. + # `window_start` is an aware datetime, NOT `today.isoformat()`. + # asyncpg binds a str as VARCHAR and Postgres has no + # `timestamptz >= text` operator, so this raised on every run + # that got this far — swallowed by the per-user `except` below, + # which is why the only symptom was reminders silently never + # sending plus an hourly traceback in the DB log (#1727). dedup_result = await session.execute( select(func.count(AppLog.id)).where( AppLog.action == "task_reminder", AppLog.user_id == user_id, - AppLog.created_at >= today_str, + AppLog.created_at >= window_start, ) ) if (dedup_result.scalar() or 0) > 0: diff --git a/tests/test_design_cascade.py b/tests/test_design_cascade.py new file mode 100644 index 0000000..9083c33 --- /dev/null +++ b/tests/test_design_cascade.py @@ -0,0 +1,420 @@ +"""The parent chain: walking it, and refusing to close it (milestone #254 step 1). + +Pure functions over a `{id: parent_id}` literal, so a whole hierarchy is one line +of setup and no database is involved. That is the reason the cascade lives in its +own import-free module — see services/design_cascade.py. +""" +from types import SimpleNamespace + +from scribe.services.design_cascade import ancestry, resolve_tokens, would_cycle + + +# --- ancestry --------------------------------------------------------------- + +def test_ancestry_returns_the_chain_deepest_first(): + """Deepest first because that is the order resolution consumes it in: the + system being resolved wins over its parent, which wins over the root.""" + parents = {1: None, 2: 1, 3: 2} + assert ancestry(3, parents) == [3, 2, 1] + + +def test_a_root_is_its_own_whole_chain(): + """A family system has no parent, and that is an ordinary state — not an + incomplete one. It must resolve to exactly itself.""" + assert ancestry(1, {1: None}) == [1] + + +def test_a_missing_parent_truncates_rather_than_raising(): + """A parent that was soft-deleted (or filtered out of the caller's scope) is + a shorter chain, not a failed request. The alternative — raising — would make + one deleted system break every descendant's rendering.""" + assert ancestry(3, {3: 2}) == [3, 2] + + +def test_a_system_absent_from_the_map_still_yields_itself(): + assert ancestry(9, {}) == [9] + + +def test_ancestry_terminates_on_a_cycle_instead_of_hanging(): + """LOAD-BEARING, and the reason a visited-set exists even though writes are + guarded. A loop introduced by a direct DB edit or a future bug must degrade + to a truncated chain: truncation shows up in the result, a hang shows up as + an outage. Every id appears exactly once.""" + parents = {1: 3, 2: 1, 3: 2} + chain = ancestry(1, parents) + assert chain == [1, 3, 2] + assert len(chain) == len(set(chain)) + + +def test_ancestry_terminates_on_a_self_parent(): + assert ancestry(1, {1: 1}) == [1] + + +# --- would_cycle ------------------------------------------------------------ + +def test_clearing_the_parent_never_cycles(): + """None means "make this a root", which is always safe.""" + assert would_cycle(2, None, {1: None, 2: 1}) is False + + +def test_a_system_cannot_be_its_own_parent(): + assert would_cycle(1, 1, {1: None}) is True + + +def test_an_ordinary_reparent_is_allowed(): + """family <- app is the shape the whole model exists for; it must not trip + the guard.""" + assert would_cycle(2, 1, {1: None, 2: None}) is False + + +def test_a_direct_swap_is_refused(): + """A -> B, then B -> A. The two-system case, and the one a UI produces first + because both systems are on screen together.""" + parents = {1: None, 2: 1} + assert would_cycle(1, 2, parents) is True + + +def test_an_indirect_loop_is_refused(): + """Three deep: root <- mid <- leaf, then root's parent set to leaf. Catching + this is what makes the check a chain walk rather than a parent comparison.""" + parents = {1: None, 2: 1, 3: 2} + assert would_cycle(1, 3, parents) is True + + +def test_reparenting_onto_a_sibling_subtree_is_allowed(): + """Two branches off one root. Moving one under the other is legitimate — the + guard must refuse loops, not reorganisation.""" + parents = {1: None, 2: 1, 3: 1} + assert would_cycle(3, 2, parents) is False + + +def test_the_guard_survives_a_hierarchy_that_is_already_corrupt(): + """If a cycle somehow already exists, the guard still has to answer rather + than spin — the write path is exactly where such a hierarchy gets repaired.""" + parents = {1: 2, 2: 1, 3: None} + assert would_cycle(3, 1, parents) is False + assert would_cycle(1, 2, parents) is True + + +# --- resolution ------------------------------------------------------------- +# +# The cascade proper (milestone #254 step 2). Hierarchies are stated as literals +# because resolve_tokens is pure and duck-typed — which is the whole reason it +# lives here rather than inside the service. + +def _token(name, value_by_mode, group_name=None, purpose=None, order_index=0, + supersedes=None): + return SimpleNamespace( + name=name, value_by_mode=value_by_mode, group_name=group_name, + purpose=purpose, order_index=order_index, supersedes=supersedes or [], + ) + + +# A family (1) and an app inheriting from it (2) — the shape the model exists for. +FAMILY, APP = 1, 2 +PARENTS = {FAMILY: None, APP: FAMILY} + + +def _by_name(tokens): + return {t.name: t for t in tokens} + + +def test_a_system_with_no_tokens_of_its_own_inherits_the_whole_family_set(): + """The correct answer for an app that has not departed from the family yet — + and the state every app system starts in.""" + resolved = resolve_tokens( + APP, PARENTS, + {FAMILY: [_token("--fs-obsidian", {"base": "#14171a"})], APP: []}, + ) + assert [t.name for t in resolved] == ["--fs-obsidian"] + assert resolved[0].value_by_mode == {"base": "#14171a"} + assert resolved[0].origin_by_mode == {"base": FAMILY} + + +def test_the_deepest_system_wins_and_says_what_it_overrode(): + """Provenance is the point of the whole model: the effective set is just a + list without it, and "where does this app depart from the family?" becomes a + diff someone has to compute.""" + resolved = _by_name(resolve_tokens( + APP, PARENTS, + { + FAMILY: [_token("--fs-accent", {"base": "#6b2118"})], + APP: [_token("--fs-accent", {"base": "#5b4a8a"})], + }, + )) + accent = resolved["--fs-accent"] + assert accent.value_by_mode == {"base": "#5b4a8a"} + assert accent.origin_by_mode == {"base": APP} + # The shadowed ancestor is still there, behind the winner. + assert [c.system_id for c in accent.contributions["base"]] == [APP, FAMILY] + assert accent.contributions["base"][1].value == "#6b2118" + + +def test_overriding_one_mode_leaves_the_others_inherited(): + """LOAD-BEARING, and the argument that decided the storage shape. An accent + deepened for light backgrounds while dark is left alone must own `base` and + still inherit `dark` — which is why merging is per (name, MODE) and why the + value column is a map rather than a pair of columns.""" + resolved = _by_name(resolve_tokens( + APP, PARENTS, + { + FAMILY: [_token("--fs-accent", {"base": "#34a877", "dark": "#34a877"})], + APP: [_token("--fs-accent", {"base": "#15803d"})], + }, + )) + accent = resolved["--fs-accent"] + assert accent.value_by_mode == {"base": "#15803d", "dark": "#34a877"} + assert accent.origin_by_mode == {"base": APP, "dark": FAMILY} + + +def test_a_token_only_the_app_defines_is_not_an_override(): + """Introducing a token and overriding one are different acts, and a UI that + labelled both "overridden here" would misdescribe the first.""" + resolved = _by_name(resolve_tokens( + APP, PARENTS, + {FAMILY: [], APP: [_token("--fs-editor-caret", {"base": "#5b4a8a"})]}, + )) + caret = resolved["--fs-editor-caret"] + assert caret.origin_by_mode == {"base": APP} + assert caret.is_overridden_in(APP) is False + + +def test_is_overridden_in_is_true_only_for_the_system_that_shadowed(): + resolved = _by_name(resolve_tokens( + APP, PARENTS, + { + FAMILY: [_token("--fs-accent", {"base": "#6b2118"})], + APP: [_token("--fs-accent", {"base": "#5b4a8a"})], + }, + )) + accent = resolved["--fs-accent"] + assert accent.is_overridden_in(APP) is True + assert accent.is_overridden_in(FAMILY) is False + + +def test_three_levels_stack_nearest_first(): + """A chain deeper than family->app, to prove the walk isn't a single hop.""" + parents = {1: None, 2: 1, 3: 2} + resolved = _by_name(resolve_tokens( + 3, parents, + { + 1: [_token("--fs-bg", {"base": "a"})], + 2: [_token("--fs-bg", {"base": "b"})], + 3: [_token("--fs-bg", {"base": "c"})], + }, + )) + bg = resolved["--fs-bg"] + assert bg.value_by_mode == {"base": "c"} + assert [c.value for c in bg.contributions["base"]] == ["c", "b", "a"] + + +def test_resolving_the_family_itself_ignores_its_children(): + """Inheritance runs one way. A family system resolved on its own must not + pick up an app's overrides — otherwise every app would restyle the house.""" + resolved = _by_name(resolve_tokens( + FAMILY, PARENTS, + { + FAMILY: [_token("--fs-accent", {"base": "#6b2118"})], + APP: [_token("--fs-accent", {"base": "#5b4a8a"})], + }, + )) + assert resolved["--fs-accent"].value_by_mode == {"base": "#6b2118"} + + +def test_value_for_falls_back_to_the_base_mode(): + """A token that isn't mode-dependent carries only `base`, and asking it for + "dark" must yield that rather than nothing — the read rule the storage shape + implies.""" + resolved = _by_name(resolve_tokens( + FAMILY, PARENTS, {FAMILY: [_token("--fs-radius-md", {"base": "8px"})]}, + )) + radius = resolved["--fs-radius-md"] + assert radius.value_for("dark") == "8px" + assert radius.value_for("base") == "8px" + + +def test_value_for_prefers_an_explicit_mode_over_the_fallback(): + resolved = _by_name(resolve_tokens( + FAMILY, PARENTS, + {FAMILY: [_token("--fs-bg", {"base": "#f7f5ef", "dark": "#14171a"})]}, + )) + assert resolved["--fs-bg"].value_for("dark") == "#14171a" + + +def test_metadata_is_inherited_when_the_override_leaves_it_blank(): + """A child overriding a colour routinely says nothing about what the token is + FOR. Inheriting the family's description beats blanking it — the override was + about the value, not the meaning.""" + resolved = _by_name(resolve_tokens( + APP, PARENTS, + { + FAMILY: [_token( + "--fs-obsidian", {"base": "#14171a"}, + group_name="surface", purpose="page bg, deepest surface", + )], + APP: [_token("--fs-obsidian", {"base": "#101317"})], + }, + )) + obsidian = resolved["--fs-obsidian"] + assert obsidian.group_name == "surface" + assert obsidian.purpose == "page bg, deepest surface" + assert obsidian.value_by_mode == {"base": "#101317"} # the value still won + + +def test_an_override_that_states_metadata_wins_it_too(): + resolved = _by_name(resolve_tokens( + APP, PARENTS, + { + FAMILY: [_token("--fs-x", {"base": "a"}, purpose="family says")], + APP: [_token("--fs-x", {"base": "b"}, purpose="app says")], + }, + )) + assert resolved["--fs-x"].purpose == "app says" + + +def test_an_override_at_default_order_keeps_the_familys_position(): + """order_index 0 is the column DEFAULT, so it reads as unstated. Treating it + as "first" would let a colour-only override drag its token to the top of the + group — a visible reshuffle nobody asked for.""" + resolved = _by_name(resolve_tokens( + APP, PARENTS, + { + FAMILY: [_token("--fs-x", {"base": "a"}, order_index=7)], + APP: [_token("--fs-x", {"base": "b"})], + }, + )) + assert resolved["--fs-x"].order_index == 7 + + +def test_the_effective_set_is_ordered_by_group_then_position_with_ungrouped_last(): + resolved = resolve_tokens( + FAMILY, PARENTS, + {FAMILY: [ + _token("--fs-z", {"base": "1"}), # ungrouped + _token("--fs-b", {"base": "2"}, group_name="text", order_index=1), + _token("--fs-a", {"base": "3"}, group_name="surface", order_index=2), + _token("--fs-c", {"base": "4"}, group_name="surface", order_index=1), + ]}, + ) + assert [t.name for t in resolved] == ["--fs-c", "--fs-a", "--fs-b", "--fs-z"] + + +def test_resolution_terminates_on_a_corrupt_hierarchy(): + """Resolution inherits ancestry's visited-set. A loop reaching this function + must produce a truncated set, not a hung request — the defensive half of the + cycle guard, exercised end to end.""" + parents = {1: 2, 2: 1} + resolved = _by_name(resolve_tokens( + 1, parents, + {1: [_token("--fs-a", {"base": "one"})], 2: [_token("--fs-b", {"base": "two"})]}, + )) + assert set(resolved) == {"--fs-a", "--fs-b"} + # Each system contributes exactly once, not endlessly. + assert len(resolved["--fs-a"].contributions["base"]) == 1 + + +# --- supersedes ------------------------------------------------------------- +# +# The declaration that replaces a prohibition. A design system stores what things +# ARE, so "pure white is never text" has no row — but "write this token instead +# of #fff" does, and it is the same fact stated forwards. + +def test_supersedes_is_inherited_when_the_override_is_silent_about_it(): + """LOAD-BEARING. A child overriding a colour says nothing about which + literals it replaces, and blanking the family's declaration there would + silently disarm the check for every app that customises the token.""" + resolved = _by_name(resolve_tokens( + APP, PARENTS, + { + FAMILY: [_token("--fs-text", {"base": "#e8e4d8"}, supersedes=["#fff", "#ffffff"])], + APP: [_token("--fs-text", {"base": "#f0ece0"})], + }, + )) + text = resolved["--fs-text"] + assert text.supersedes == ("#fff", "#ffffff") + assert text.value_by_mode == {"base": "#f0ece0"} # the value still overrode + + +def test_an_override_that_states_its_own_supersedes_replaces_the_list(): + """Whole-list replacement, not a merge — an app that means "only #fff" must + be able to say so without inheriting entries it deliberately dropped.""" + resolved = _by_name(resolve_tokens( + APP, PARENTS, + { + FAMILY: [_token("--fs-text", {"base": "a"}, supersedes=["#fff", "#ffffff"])], + APP: [_token("--fs-text", {"base": "b"}, supersedes=["#fff"])], + }, + )) + assert resolved["--fs-text"].supersedes == ("#fff",) + + +def test_a_token_that_supersedes_nothing_resolves_to_an_empty_tuple(): + """Most tokens replace nothing. That has to be an empty sequence rather than + None, so no caller has to test for two kinds of nothing.""" + resolved = _by_name(resolve_tokens( + FAMILY, PARENTS, {FAMILY: [_token("--fs-radius-md", {"base": "8px"})]}, + )) + assert resolved["--fs-radius-md"].supersedes == () + + +def test_supersedes_survives_serialisation_as_a_list(): + resolved = _by_name(resolve_tokens( + FAMILY, PARENTS, + {FAMILY: [_token("--fs-text", {"base": "#e8e4d8"}, supersedes=["#fff"])]}, + )) + assert resolved["--fs-text"].to_dict()["supersedes"] == ["#fff"] + + +def test_the_superseded_literal_need_not_match_the_tokens_own_value(): + """The whole reason this is DECLARED rather than derived. `#fff` and + Parchment are different colours, so a value-matching rule could never have + connected them — which is why the prohibition looked unrepresentable until + it was turned around.""" + resolved = _by_name(resolve_tokens( + FAMILY, PARENTS, + {FAMILY: [_token("--fs-text", {"base": "#e8e4d8"}, supersedes=["#fff"])]}, + )) + text = resolved["--fs-text"] + assert text.value_by_mode["base"] not in text.supersedes + + +def test_rows_without_a_supersedes_attribute_at_all_still_resolve(): + """Duck-typed input: a caller passing rows from before the column existed + must not crash the cascade.""" + legacy = SimpleNamespace( + name="--fs-x", value_by_mode={"base": "a"}, + group_name=None, purpose=None, order_index=0, + ) + resolved = _by_name(resolve_tokens(FAMILY, PARENTS, {FAMILY: [legacy]})) + assert resolved["--fs-x"].supersedes == () + + +# --- rationale -------------------------------------------------------------- + +def test_rationale_cascades_like_purpose_and_is_a_different_question(): + """`purpose` is what the token is FOR; `rationale` is why it is this value. + Rules carry the second routinely ("Success = Moss, aligned by design") and a + token row had nowhere to put it until now.""" + resolved = _by_name(resolve_tokens( + APP, PARENTS, + { + FAMILY: [SimpleNamespace( + name="--fs-success", value_by_mode={"base": "#4a5d3f"}, + group_name="semantic", purpose="success states", + rationale="equals Moss, aligned by design", + order_index=0, supersedes=[], + )], + APP: [_token("--fs-success", {"base": "#3f5236"})], + }, + )) + token = resolved["--fs-success"] + assert token.rationale == "equals Moss, aligned by design" + assert token.purpose == "success states" + assert token.value_by_mode == {"base": "#3f5236"} # the value still overrode + + +def test_a_token_without_a_rationale_resolves_to_none(): + resolved = _by_name(resolve_tokens( + FAMILY, PARENTS, {FAMILY: [_token("--fs-x", {"base": "1px"})]}, + )) + assert resolved["--fs-x"].rationale is None diff --git a/tests/test_design_rulebook_import.py b/tests/test_design_rulebook_import.py new file mode 100644 index 0000000..9a0c942 --- /dev/null +++ b/tests/test_design_rulebook_import.py @@ -0,0 +1,161 @@ +"""Rulebook prose → checkable claims (milestone #251 step 2). + +This is the piece of the design explorer that most needed to be testable, which +is why it lives in Python at all: the frontend has no test runner, so the fiddly +extraction happens server-side and the browser only does set arithmetic over it. + +Rule text below is representative of a real design rulebook rather than copied +from this operator's — rule #115: the product must work for an install that has +none of their data, and a test that only passes against their exact wording would +be testing the instance, not the parser. +""" +from types import SimpleNamespace + +from scribe.services.design_rulebook_import import ( + expand_token_shorthand, + extract_expectations, + normalize_hex, +) + + +def _rule(rule_id, title, statement, how_to_apply=None): + return SimpleNamespace( + id=rule_id, title=title, statement=statement, how_to_apply=how_to_apply + ) + + +# --- hex normalisation ------------------------------------------------------- + +def test_normalize_hex_makes_shorthand_and_case_comparable(): + """LOAD-BEARING. The rulebook writes `#FFFFFF` and components write `#fff`. + If those don't compare equal, the single largest drift finding — 67 hardcoded + white text colours (#2275) — reads as zero findings.""" + assert normalize_hex("#fff") == normalize_hex("#FFFFFF") == "#ffffff" + assert normalize_hex("#E8E4D8") == "#e8e4d8" + assert normalize_hex("#14171a") == "#14171a" + + +def test_normalize_hex_keeps_alpha_rather_than_inventing_equality(): + """`#fff` and `#ffff` are different colours. Dropping the alpha to make them + match would manufacture agreement that isn't there.""" + assert normalize_hex("#ffff") == "#ffffffff" + assert normalize_hex("#fff") != normalize_hex("#ffff") + + +def test_normalize_hex_rejects_non_colours(): + for junk in ("", " ", "not-a-colour", "#", "#gg", "#12345"): + assert normalize_hex(junk) is None + + +# --- the slash shorthand ----------------------------------------------------- + +def test_expand_token_shorthand_handles_every_form_a_rulebook_uses(): + """One rule expands all three shapes: take everything up to and including the + LAST hyphen of the first segment as the prefix.""" + assert expand_token_shorthand("--fs-radius-sm/md/lg/xl") == [ + "--fs-radius-sm", "--fs-radius-md", "--fs-radius-lg", "--fs-radius-xl", + ] + # Prefix is just `--fs-` here, and the same rule finds it. + assert expand_token_shorthand("--fs-obsidian/iron/slate/pewter") == [ + "--fs-obsidian", "--fs-iron", "--fs-slate", "--fs-pewter", + ] + assert expand_token_shorthand("--fs-dur-fast/base/slow") == [ + "--fs-dur-fast", "--fs-dur-base", "--fs-dur-slow", + ] + + +def test_expand_token_shorthand_passes_plain_names_through(): + assert expand_token_shorthand("--fs-ease") == ["--fs-ease"] + + +# --- extraction -------------------------------------------------------------- + +def test_negation_is_scoped_to_the_sentence_not_the_rule(): + """THE trick that makes prohibition detection usable. + + A single rule routinely states what the palette REQUIRES and what it FORBIDS + in consecutive sentences. Detecting negation across the whole statement would + mark the required colours as forbidden too — inverting the finding rather + than missing it, which is worse. + """ + rule = _rule( + 52, "Text palette", + "Text tokens: Parchment #E8E4D8 (primary), Vellum #C2BFB4 (secondary), " + "Ash #9C9A92 (tertiary). Pure white #FFFFFF is NEVER used as text color.", + ) + found = extract_expectations([rule]) + required = {e.value for e in found if e.kind == "color"} + forbidden = {e.value for e in found if e.kind == "prohibited_color"} + + assert required == {"#e8e4d8", "#c2bfb4", "#9c9a92"} + assert forbidden == {"#ffffff"} + assert not (required & forbidden) + + +def test_token_names_are_extracted_and_expanded(): + rule = _rule( + 72, "CSS custom properties", + "Expose the system as custom properties on :root — surfaces " + "(--fs-obsidian/iron/slate/pewter), radius (--fs-radius-sm/md/lg/xl), " + "and motion (--fs-ease).", + ) + names = {e.value for e in extract_expectations([rule]) if e.kind == "token"} + assert "--fs-obsidian" in names and "--fs-pewter" in names + assert "--fs-radius-xl" in names + assert "--fs-ease" in names + assert len(names) == 9 + + +def test_how_to_apply_is_read_as_well_as_the_statement(): + """Rulebooks routinely put the concrete values in how_to_apply and keep the + statement declarative, so ignoring it would miss the checkable half.""" + rule = _rule( + 56, "Per-app accent", "Each app owns exactly one accent.", + how_to_apply='[data-app="scribe"] #5B4A8A, [data-app="minstrel"] #4A6B5C.', + ) + colours = {e.value for e in extract_expectations([rule]) if e.kind == "color"} + assert colours == {"#5b4a8a", "#4a6b5c"} + + +def test_claims_are_deduped_across_rules_keeping_the_first_source(): + """A colour named by several rules is one expectation, attributed to the rule + that introduced it — usually the most specific place to send a reader.""" + rules = [ + _rule(51, "Surfaces", "Obsidian #14171A is the page background."), + _rule(99, "Elsewhere", "Obsidian #14171A again, mentioned in passing."), + ] + found = [e for e in extract_expectations(rules) if e.kind == "color"] + assert len(found) == 1 + assert found[0].rule_id == 51 + + +def test_prose_with_nothing_checkable_yields_nothing(): + """Most rules are judgement, not specification. They must contribute no + findings rather than a shrug — a panel that reports unparseable rules as + problems would be unusable.""" + rule = _rule( + 68, "Voice and tone", + "Voice is understated: plain language for anything functional, flavour " + "only where the user is waiting or failing. Be brief.", + ) + assert extract_expectations([rule]) == [] + + +def test_every_expectation_carries_the_sentence_it_came_from(): + """The panel has to show its working — "the rulebook says X" is only + actionable if you can see where, and in what context. + + Asserts the context is the SENTENCE, not the whole statement: a rule that + states a requirement and a prohibition in consecutive sentences would + otherwise attribute both to the same undifferentiated blob of prose. + """ + rule = _rule(63, "Radius", "Radius: Small 4px. Pure white #FFFFFF is never used.") + found = extract_expectations([rule]) + assert len(found) == 1 + + only = found[0] + assert only.kind == "prohibited_color" + assert only.rule_id == 63 + assert only.rule_title == "Radius" + assert only.context == "Pure white #FFFFFF is never used." + assert "Radius: Small 4px" not in only.context diff --git a/tests/test_design_rulebook_propose.py b/tests/test_design_rulebook_propose.py new file mode 100644 index 0000000..4197f33 --- /dev/null +++ b/tests/test_design_rulebook_propose.py @@ -0,0 +1,145 @@ +"""Rulebook prose -> a PROPOSED design system (milestone #254 step 3). + +The extraction tested in test_design_rulebook_import.py answers "what claims does +this rulebook make". This answers a harder question — "what design system is it +describing" — which needs the two halves joined: one rule names the colours, +another names the custom properties, and neither alone is a token. + +Rule text is representative rather than copied from this operator's rulebook +(rule #115): a test that only passes against their exact wording would be +testing the instance. +""" +from types import SimpleNamespace + +from scribe.services.design_rulebook_import import propose_tokens + + +def _rule(rule_id, title, statement, how_to_apply=None): + return SimpleNamespace( + id=rule_id, title=title, statement=statement, how_to_apply=how_to_apply + ) + + +SURFACES = _rule( + 51, "Universal surfaces", + "Obsidian #14171A (page bg, deepest surface), Iron #1E2228 (cards), " + "Slate #2C313A (hovered surfaces).", +) +TEXT = _rule( + 52, "Text palette", + "Text tokens: Parchment #E8E4D8 (primary text), Vellum #C2BFB4 (secondary). " + "Pure white #FFFFFF is NEVER used as text color.", +) +PROPERTIES = _rule( + 72, "CSS custom properties", + "Expose the system as custom properties: surfaces " + "(--fs-obsidian/iron/slate), text (--fs-parchment/vellum), and radius " + "(--fs-radius-sm/md/lg).", +) + + +def _by_name(proposals): + return {p.name: p for p in proposals} + + +# --- the join --------------------------------------------------------------- + +def test_a_token_takes_its_value_from_the_colour_of_the_same_name(): + """THE mechanism. `--fs-obsidian` and "Obsidian #14171A" are declared in + different rules and neither is a token on its own. Joining them on the word + is the only reason an import produces something usable instead of a list of + empty names.""" + proposals = _by_name(propose_tokens([SURFACES, PROPERTIES])) + assert proposals["--fs-obsidian"].value_by_mode == {"base": "#14171a"} + assert proposals["--fs-iron"].value_by_mode == {"base": "#1e2228"} + + +def test_the_parenthetical_becomes_the_tokens_purpose(): + """Rulebooks say what a colour is FOR right beside its value, and that is + the field a bare hex can never carry.""" + proposals = _by_name(propose_tokens([SURFACES, PROPERTIES])) + assert proposals["--fs-obsidian"].purpose == "page bg, deepest surface" + + +def test_a_token_with_no_matching_colour_is_proposed_with_no_value(): + """HONEST OUTPUT, not a failure. The rulebook states radius steps as prose + ("Small 4px"), which nothing here parses. The name is real and the value + needs a human — proposing the name with an empty value says exactly that, + where dropping it would hide a token the rulebook asked for.""" + proposals = _by_name(propose_tokens([SURFACES, PROPERTIES])) + assert proposals["--fs-radius-sm"].value_by_mode == {} + assert "--fs-radius-lg" in proposals + + +def test_every_proposal_carries_the_rule_and_sentence_it_came_from(): + """An import is a proposal a human reviews, and a claim you cannot trace is + a claim you have to take on faith.""" + obsidian = _by_name(propose_tokens([SURFACES, PROPERTIES]))["--fs-obsidian"] + assert obsidian.source_rule_id == 51 + assert obsidian.source_rule_title == "Universal surfaces" + assert "Obsidian #14171A" in obsidian.source_context + + +# --- prohibitions become replacements --------------------------------------- + +def test_a_prohibition_becomes_supersedes_on_that_rules_primary_token(): + """The reframe, end to end. Rule 52 declares Parchment and forbids pure + white in one breath; the import turns that into "write --fs-parchment + instead of #ffffff" — the same fact, stated forwards, and actionable.""" + proposals = _by_name(propose_tokens([TEXT, PROPERTIES])) + assert proposals["--fs-parchment"].supersedes == ["#ffffff"] + + +def test_a_prohibition_attaches_to_one_token_not_every_token_of_its_rule(): + """Rule 52 declares two colours. Attaching the prohibition to both would + claim the rulebook said something it didn't — that Vellum is also the + replacement for white.""" + proposals = _by_name(propose_tokens([TEXT, PROPERTIES])) + assert proposals["--fs-vellum"].supersedes == [] + + +def test_a_forbidden_colour_never_becomes_a_token_value(): + """Sentence-scoped negation carried through to the import: #FFFFFF appears + in rule 52 as a hex, and a naive read would make it Parchment's value.""" + proposals = propose_tokens([TEXT, PROPERTIES]) + for proposal in proposals: + assert proposal.value_by_mode.get("base") != "#ffffff" + + +# --- grouping --------------------------------------------------------------- + +def test_a_family_token_is_grouped_by_its_middle_segment(): + """`--fs-radius-sm` -> "radius". Structural, so it works on a naming scheme + this code has never seen — the prefix is each install's own (rule #115).""" + proposals = _by_name(propose_tokens([SURFACES, PROPERTIES])) + assert proposals["--fs-radius-sm"].group_name == "radius" + + +def test_a_flat_token_is_left_ungrouped_rather_than_guessed_at(): + proposals = _by_name(propose_tokens([SURFACES, PROPERTIES])) + assert proposals["--fs-obsidian"].group_name is None + + +# --- shape ------------------------------------------------------------------ + +def test_each_token_name_is_proposed_exactly_once(): + """The slash shorthand expands and rules repeat colours; neither may produce + a duplicate, since two live rows with one name is the duplicate-definition + bug the unique index exists to refuse.""" + names = [p.name for p in propose_tokens([SURFACES, TEXT, PROPERTIES])] + assert len(names) == len(set(names)) + + +def test_a_rulebook_that_names_no_tokens_proposes_nothing(): + """Most rulebooks are not design rulebooks. That has to be an empty result + rather than an error — an install can point this at anything.""" + unrelated = _rule(1, "Branching", "Work happens on the dev branch.") + assert propose_tokens([unrelated]) == [] + + +def test_colours_declared_without_a_token_name_are_not_invented_into_tokens(): + """A rulebook naming a colour it never exposes as a custom property has not + asked for a token, and inventing a name for it would put a token in the + record that no rule sanctions.""" + proposals = propose_tokens([SURFACES]) + assert proposals == [] diff --git a/tests/test_design_stylesheet.py b/tests/test_design_stylesheet.py new file mode 100644 index 0000000..56e9e62 --- /dev/null +++ b/tests/test_design_stylesheet.py @@ -0,0 +1,342 @@ +"""Rendering a design system as its master CSS sheet. + +The generator is pure, so a whole sheet is one literal of tokens in and a string +out. Two things here are load-bearing rather than cosmetic: what the sheet +deliberately does NOT contain, and the fact that a value cannot escape its +declaration. +""" +from types import SimpleNamespace + +from scribe.services.design_stylesheet import ( + check_code_against_tokens, + duplicate_values, + is_valid_token_name, + render_stylesheet, + safe_comment, + safe_value, + selector_for_mode, +) + + +def _token(name, value_by_mode, group_name=None, purpose=None): + return SimpleNamespace( + name=name, value_by_mode=value_by_mode, + group_name=group_name, purpose=purpose, + ) + + +# --- safety ----------------------------------------------------------------- +# +# Design systems are shareable records. A value that can close its declaration +# can inject arbitrary CSS into the page of anyone the system was shared with, +# which makes this a real boundary rather than tidiness. + +def test_a_value_that_would_escape_its_declaration_is_refused(): + """`red; } body { display: none` is the whole attack: end the declaration, + close the block, open your own.""" + assert safe_value("red; } body { display: none") is None + assert safe_value("#fff}") is None + assert safe_value("#fff;") is None + + +def test_at_rules_and_tags_are_refused(): + assert safe_value("@import url(evil.css)") is None + assert safe_value("