From a9c53d32b17c8883074e6c003b5f313f2b4022be Mon Sep 17 00:00:00 2001 From: Kieron Lanning Date: Wed, 14 Oct 2026 23:46:56 +0100 Subject: [PATCH] docs: add agent skills distribution page and canonical skills Authored skills now live in a tracked repository-root skills/ folder. The .agents/ mirror is gitignored, so it is absent from a git clone and cannot be the source the skills CLI reads. Adds purview-architecture, purview-telemetry and purview-persistence, plus the Agent-Skills-Distribution wiki page covering publication, package-to-skill discovery, version resolution and installer responsibilities. --- AGENTS.md | 10 + docs/wiki/Agent-Enablement.md | 28 +- docs/wiki/Agent-Skills-Distribution.md | 297 ++++++++++++++++++ docs/wiki/Home.md | 1 + docs/wiki/_Sidebar.md | 1 + skills/purview-architecture/SKILL.md | 102 ++++++ .../references/project-context-discovery.md | 115 +++++++ .../references/proportional-architecture.md | 92 ++++++ skills/purview-persistence/SKILL.md | 106 +++++++ .../references/guid-generation.md | 93 ++++++ .../references/postgresql.md | 98 ++++++ .../references/sqlserver.md | 101 ++++++ skills/purview-telemetry/SKILL.md | 123 ++++++++ .../references/observability-levels.md | 93 ++++++ 14 files changed, 1256 insertions(+), 4 deletions(-) create mode 100644 docs/wiki/Agent-Skills-Distribution.md create mode 100644 skills/purview-architecture/SKILL.md create mode 100644 skills/purview-architecture/references/project-context-discovery.md create mode 100644 skills/purview-architecture/references/proportional-architecture.md create mode 100644 skills/purview-persistence/SKILL.md create mode 100644 skills/purview-persistence/references/guid-generation.md create mode 100644 skills/purview-persistence/references/postgresql.md create mode 100644 skills/purview-persistence/references/sqlserver.md create mode 100644 skills/purview-telemetry/SKILL.md create mode 100644 skills/purview-telemetry/references/observability-levels.md diff --git a/AGENTS.md b/AGENTS.md index addf83f..7639dd4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -30,6 +30,7 @@ implementation whose only job is to prove the conventions compile, run, and fail | `src/tests/Application.UnitTests` | The service, against a TUnit.Mocks substitute | | `src/tests/Architecture.UnitTests` | Dependency-direction rules; a wrong `ProjectReference` fails the build | | `src/tests/SharedTestingInfra` | `TestBuilder`; referenced automatically by every test project | +| `skills/` | Authored agent skills, installable with `npx skills add purview-dev/engineering-playbook` | | `templates/` | Ready-to-copy starter files for a repository adopting the playbook | | `src/Directory.Build.props` | `NamespacePrefix`, the SDK import, and `NamespaceRemoveSuffix` | | `.config/purview-build.json` | Pipeline configuration: solution, test discovery, filter | @@ -64,6 +65,15 @@ demonstrate it. - **Fences always declare a language.** Mermaid is not enabled in `mkdocs.yml`, so raw Mermaid source goes in a ` ```text ` block. +## Skills + +Authored skills live in `skills//SKILL.md` at the repository root — not in `.agents/skills/`, which +is the gitignored mirror that packages write into. The root folder is the one the skills CLI can discover +in a git clone. A skill states when it applies in its `description`, keeps the workflow in `SKILL.md`, and +pushes detail into `references/`. See [Agent Skills Distribution](docs/wiki/Agent-Skills-Distribution.md). + +This repository ships no package, so its skills travel by the git route only. + ## Code conventions `src/` follows the same conventions every Purview repository follows. The ones that bite here specifically: diff --git a/docs/wiki/Agent-Enablement.md b/docs/wiki/Agent-Enablement.md index fe92def..a30731e 100644 --- a/docs/wiki/Agent-Enablement.md +++ b/docs/wiki/Agent-Enablement.md @@ -56,16 +56,24 @@ rule the humans have not written down, and a rule that only the wiki states is o ## The `.agents/` folder ```text +skills/ authored skills, tracked in git +└── / + ├── SKILL.md the skill itself + └── references/ optional deeper material the skill links to + .agents/ ├── .gitignore generated; lists the mirrored files ├── agents/ sub-agent definitions ├── prompts/ reusable prompts -└── skills/ +└── skills/ mirrored from packages; gitignored └── / - ├── SKILL.md the skill itself - └── references/ optional deeper material the skill links to ``` +Note the split. A skill you **author** lives at the repository root in `skills/`, where it is tracked +and where a cloning installer can find it. A skill you **receive** from a package arrives in +`.agents/skills/`, where it is gitignored. The same folder cannot do both jobs, and +[Agent Skills Distribution](Agent-Skills-Distribution.md) explains why. + A **skill** is a self-contained instruction set for one kind of task. It opens with YAML frontmatter whose `description` is the trigger — it is what tells an agent that this skill applies: @@ -132,10 +140,22 @@ appears in a status output. - **A repository with several task shapes.** Add a prompt per shape, and consider a sub-agent definition for the one that needs a different remit rather than more instructions. - **A repository that others copy from.** Ship your skills in a package so the guidance travels with the - code, in the same way the SDK does — the mirror mechanism is the pattern, not the SDK. + code, in the same way the SDK does — the mirror mechanism is the pattern, not the SDK. See + [Agent Skills Distribution](Agent-Skills-Distribution.md). + +## The gap the mirror does not close + +`.agents/` is a convention this playbook and the SDK share. It is **not** a directory any agent reads +natively — Claude Code, Codex, Copilot and the rest each look somewhere else. Mirroring a skill into +`.agents/` therefore makes it present, not active. + +Closing that gap is a separate, explicit step, delegated to a tool that tracks every agent's discovery +paths so the SDK does not have to. That is the subject of +[Agent Skills Distribution](Agent-Skills-Distribution.md). ## See also +- [Agent Skills Distribution](Agent-Skills-Distribution.md) - [Documentation Convention](Documentation-Convention.md) - [Repository Shape](Repository-Shape.md) - [build-sdk: Agent Folder](https://github.com/purview-dev/build-sdk/blob/main/docs/wiki/Agent-Folder.md) diff --git a/docs/wiki/Agent-Skills-Distribution.md b/docs/wiki/Agent-Skills-Distribution.md new file mode 100644 index 0000000..1ed6c94 --- /dev/null +++ b/docs/wiki/Agent-Skills-Distribution.md @@ -0,0 +1,297 @@ +# Agent Skills Distribution + +## The rule + +A skill is published **once**, at the repository root in `skills//SKILL.md`, and travels to its +consumers by two routes that share that single copy: + +| Route | Who it serves | Mechanism | +| -- | -- | -- | +| NuGet package → `.agents/skills/` | A .NET repository that references the package | Mirrored on build by `Purview.BuildSdk` | +| Git repository → agent directories | Any agent, any language, via `npx skills` | Installed on request by the skills CLI | + +**Installation into an agent's own directories is always explicit.** It is never a side effect of +`dotnet restore`, `dotnet build`, a design-time build in an IDE, or a test run. + +Version compatibility is resolved from metadata the package already carries — the skills it ships and +the commit it was built from — so there is no hand-maintained package-to-skill manifest. + +## Why + +A skill is guidance that goes stale against the code it describes. Three failures follow from that, +and the design answers each one. + +**Guidance that does not travel with the code.** If a library's conventions live only in its own +repository's wiki, every consumer re-derives them, and an agent in a consuming repository has no +access to them at all. Shipping the skill inside the package means a `PackageReference` is enough. + +**Guidance that does not match the version in use.** A project pinned to an older release that +receives the newest skill is told about APIs it does not have. The nuspec records the commit the +package was built from, so the skill resolved is the skill *that version* shipped. + +**An agent directory modified by a build.** A build that writes into `.claude/`, `.codex/` or a +peer's equivalent is a build with an invisible side effect: it mutates a developer's tooling, races +between parallel projects, and makes a restore non-reproducible. Resolution is cheap and automatic; +installation is a decision a person makes. + +There is a fourth, subtler reason for the root `skills/` folder. The mirrored `.agents/` folder is +gitignored in every consuming repository — that is what stops a package upgrade appearing as a local +modification. A gitignored folder is absent from a git clone, so it cannot also be the source a +cloning installer reads. The authored copy has to live somewhere tracked. + +## Skill format + +The Agent Skills convention, unchanged: + +```text +skills/ +└── purview-example/ + ├── SKILL.md required; YAML frontmatter plus the main workflow + ├── references/ deeper material the skill links to, read on demand + ├── scripts/ deterministic operations + └── assets/ templates and fixtures +``` + +`SKILL.md` opens with frontmatter whose `description` is the activation trigger: + +```markdown +--- +name: purview-persistence +description: >- + Use when a .NET project actually persists data and you are choosing a storage approach... +--- +``` + +Use progressive disclosure. The frontmatter says *when* the skill applies; `SKILL.md` carries the +workflow; `references/` holds the detail that only some tasks need. A skill that inlines everything +costs context on every activation, whether or not the detail was relevant. + +Link to the authoritative documentation rather than restating it. A skill that paraphrases a library's +own wiki is a second source of truth, and it is the one that will drift. + +## Publication and distribution + +```text +┌──────────────────────────────────────────────────────────────┐ +│ Library repository │ +│ │ +│ skills//SKILL.md ← the single authored copy │ +│ │ │ +│ ├──────────────┐ │ +│ ▼ ▼ │ +│ git (tracked) IncludePurviewSkillsInPackage │ +│ │ │ │ +└────────┼──────────────┼──────────────────────────────────────┘ + │ ▼ + │ .nupkg .agents/skills// + │ │ + generated .gitignore + │ │ + nuspec + │ ▼ + │ NuGet feed + │ │ + ▼ ▼ + skills CLI Purview.BuildSdk mirror + (clone + ref) (restore graph) +``` + +The nuspec's `` element is written by SourceLink for every package built by the SDK. It +is what lets a consumer get from a restored package back to the exact source commit. + +## Package-to-skill discovery + +No custom metadata file is introduced, because the package already carries both halves of the mapping. + +```text +project.assets.json + │ every restored package: id, version, on-disk path + ▼ +/.agents/skills/*/SKILL.md ──► which skills this package ships + │ +/*.nuspec + └─ + │ + ▼ + owner/repo# + [skill names] + │ + ├─ has GitHub URL and commit ──► PurviewSkillSource (installable) + └─ missing either ──► PurviewUnresolvableSkillSource (PRSKL0002) +``` + +A folder under `.agents/skills/` counts as a skill only when it contains a `SKILL.md`; a `references/` +or `scripts/` folder beside one is part of that skill, not another. + +## Responsibilities + +The separation matters more than any single piece of it. + +```text +┌───────────────────┬──────────────────────────────────────────────┐ +│ Skill metadata │ The package itself: .agents/skills/* and the │ +│ │ nuspec element. Nothing custom. │ +├───────────────────┼──────────────────────────────────────────────┤ +│ Skill resolver │ ResolvePurviewSkillSources. Reads the restore │ +│ │ graph, emits source + ref + skill names. │ +│ │ Decides nothing about agents or paths. │ +├───────────────────┼──────────────────────────────────────────────┤ +│ Skill installer │ The skills CLI. Owns every agent-specific │ +│ │ destination, the symlink-or-copy decision and │ +│ │ skills-lock.json. │ +├───────────────────┼──────────────────────────────────────────────┤ +│ Build SDK │ Discovers packages, exposes explicit targets, │ +│ │ invokes the installer. Never installs itself. │ +└───────────────────┴──────────────────────────────────────────────┘ +``` + +The SDK does not maintain a copy of each skill per supported agent. Agent discovery paths differ, are +numerous, and change on their own schedule; the CLI tracks them, so the SDK does not have to. + +## How it is enforced + +| Rule | Enforced by | +| -- | -- | +| A package's skills are discoverable from the restore graph | `ResolvePurviewSkillSources`, tested in `PurviewSkillResolutionTests` | +| The resolved skill matches the restored package version | The nuspec commit is the ref; tested | +| A package that cannot be pinned is reported, not guessed | `PRSKL0002` warning; tested | +| Resolution without a restore gives an actionable error | `PRSKL0001` error | +| An ordinary restore-and-build installs nothing | Test asserts no lockfile and no agent directory after a full build | +| Repeated installation is idempotent | The skills CLI skips a skill already installed from the same source and ref | +| Root `skills/` reaches the package's `.agents/skills/` | `AgentPackFolderTests`, asserted against the built `.nupkg` | +| A packaged skill does not show as a local modification | Generated per-skill `.gitignore`, asserted in the same test | +| A skill's frontmatter is valid and its name unique | Review, plus the CLI's own parse on install — **not** a build check | + +The last row is a review obligation. The build does not validate skill frontmatter, and this page does +not claim it does. + +## Version resolution + +```text + referenced package version (from project.assets.json) + │ + ▼ + nuspec present? + │ │ + yes no + │ │ + ▼ ▼ + owner/repo# PRSKL0002 warning: + pinned, reproducible not installed; the package's + │ own mirror still delivers it + ▼ + skills CLI records source + ref in skills-lock.json + │ + ▼ + npx skills experimental_install restores the same set +``` + +Pinning to a commit rather than a tag is deliberate: a tag can be moved, and a package can be built +from a commit that was never tagged — a pre-release, for instance. The commit is the only identifier +guaranteed to correspond to the restored package. + +Where exact compatibility cannot be determined, the SDK warns and installs nothing. Installing the +default branch's skill against an older package is the failure this is designed to avoid. + +## Using it + +```bash +dotnet restore # resolution reads the restore graph +dotnet msbuild -t:ListPurviewSkills # diagnostics; installs nothing +dotnet msbuild -t:InstallPurviewSkills # explicit install, via npx skills +``` + +| Property | Default | Purpose | +| -- | -- | -- | +| `PurviewSkillsSourceFolder` | `/skills` | Where authored skills live | +| `PurviewPackRepositorySkills` | `false` | Opts a project into packing them | +| `PurviewSkillsToPack` | *(empty — all)* | The subset of skills *this* project ships | +| `PurviewSkillsAgents` | `*` | Which agents to install to | +| `PurviewSkillsCommand` | *(detected)* | Overrides the CLI launcher | +| `PurviewSkillsSource` | *(empty)* | Install from an explicit git source instead of from packages | + +### Detecting the installer + +The installer is Node tooling, and a machine that can build .NET need not have it. The SDK probes +`PATH` for `skills` first (a global or local install — no download, works offline), then `npx`, +honouring `PATHEXT` on Windows where both are `.cmd` shims. `PATH` is read directly rather than by +spawning a version check: deterministic, no process, and it cannot hang on a cold `npx` download. + +When `npx` is the fallback it is pinned to a major (`skills@1`), so a future breaking release of the +CLI cannot silently change what the target does. + +Neither found is an actionable error naming the fix, not a raw `'npx' is not recognized`. And +`ListPurviewSkills` only *reports* availability — resolving and reporting needs no Node tooling, so +the diagnostic path always works. + +You can also install from a named repository, bypassing package resolution entirely: + +```bash +dotnet msbuild -t:InstallPurviewSkills -p:PurviewSkillsSource=purview-dev/engineering-playbook +``` + +That is the escape hatch for a repository that wants a Purview skill it has no `PackageReference` +for. The caller owns the ref — append `#` to pin it — and no restore is required. + +### One repository, several packages + +A repository usually produces more than one package, and a skill belongs to the one whose library it +describes — `Purview.Results.AspNetCore`'s HTTP-mapping skill has no business in `Purview.Results`. +`PurviewSkillsToPack` names that subset per project: + +```text +skills/ ← one authored copy of each +├── purview-results-core ─┐ +├── purview-results-union-errors ─┴─► Purview.Results +├── purview-results-http-mapping ───► Purview.Results.AspNetCore +├── purview-results-zodsharp-validation ───► Purview.Results.ZodSharp +└── purview-results-zodsharp-problems ───► Purview.Results.ZodSharp.AspNetCore +``` + +A skill left out of every `PurviewSkillsToPack` is still installable from the git source and ships in no +package — which is how a repository keeps skills about *working in that repository* (test runners, commit +conventions) separate from the guidance it publishes to consumers. + +A name in `PurviewSkillsToPack` with no matching folder is `PRSKL0003`, so a renamed or deleted skill +fails the pack instead of silently vanishing from the package. + +Updating and removing are the CLI's, not the SDK's: `npx skills update`, `npx skills remove`, +`npx skills list`. + +### Troubleshooting + +| Symptom | Cause | +| -- | -- | +| `PRSKL0001` | No `project.assets.json`. Run `dotnet restore`; resolution never restores for you. | +| `PRSKL0002` | The package's nuspec has no GitHub repository URL or no commit — usually SourceLink disabled at pack time. | +| `PRSKL0003` | `PurviewSkillsToPack` names a skill folder that does not exist — usually a rename. | +| `PRSKL0004` | Neither `skills` nor `npx` is on `PATH`. Install Node.js, or `npm install -g skills`. `ListPurviewSkills` still works. | +| "No skills found" from the CLI | The source repository has no root `skills/` folder. The CLI does not discover skills in nested or dotted folders, even with `--full-depth`. | +| Skills appear in `.agents/` but no agent sees them | `.agents/` is the mirror, not an agent directory. Run the install target. | +| `npx` not available | The install target needs Node (`PRSKL0004` names the fix). Resolution and `ListPurviewSkills` do not. | + +## Publishing skills for a new library + +1. Author the skill at the repository root: `skills//SKILL.md`. +2. Set `PurviewPackRepositorySkills=true` on the project that ships it, and name it in that project's + `PurviewSkillsToPack` when the repository produces more than one package. +3. Leave SourceLink enabled, so the nuspec records the repository and commit. +4. Verify with `npx skills add / -l` — if it reports no skills, the folder is in the + wrong place. +5. Verify the package with `dotnet pack` and inspect `.agents/skills/` in the `.nupkg` — that is what + existing consumers receive, and it should not change for skills you did not touch. + +## At your scale + +- **One repository, one agent.** You do not need any of this. Put `SKILL.md` files in `skills/` and + run `npx skills add .` when they change. The format is the part worth adopting first. +- **Several repositories sharing conventions.** Publish skills from the repository that owns the + convention and install them with the CLI. Still no SDK required. +- **A library with consumers.** Ship skills in the package. The guidance arrives with the dependency, + and it arrives at the version the consumer is actually on. +- **A platform with many packages and agents.** The full split above: metadata in the package, + resolution in the build, installation in a tool that owns agent paths. The thing to copy is the + separation, not the implementation. + +## See also + +- [Agent Enablement](Agent-Enablement.md) — `AGENTS.md`, `.agents/`, and what makes a good skill +- [Documentation Convention](Documentation-Convention.md) +- [skills CLI](https://github.com/vercel-labs/skills) diff --git a/docs/wiki/Home.md b/docs/wiki/Home.md index 98d9e34..65720e7 100644 --- a/docs/wiki/Home.md +++ b/docs/wiki/Home.md @@ -36,6 +36,7 @@ at all, so the two are never blurred. | [Contracts and Data Exchange](Contracts-And-Data-Exchange.md) | How value objects, schemas and results cross a boundary. | | [Testing and Test Data](Testing-And-Test-Data.md) | How tests are written, and where their data comes from. | | [Agent Enablement](Agent-Enablement.md) | How a repository is made legible to a coding agent. | +| [Agent Skills Distribution](Agent-Skills-Distribution.md) | How a skill is published once and reaches both a package's consumers and an agent's own directories. | | [Documentation Convention](Documentation-Convention.md) | How the wiki is written, structured and published. | | [Worked Examples](Worked-Examples.md) | The reference implementation in this repository, walked end to end. | | [Adopting This at Your Scale](Adopting-This-At-Your-Scale.md) | Which parts apply to a small, medium or large repository. | diff --git a/docs/wiki/_Sidebar.md b/docs/wiki/_Sidebar.md index 85cfab3..fdd182e 100644 --- a/docs/wiki/_Sidebar.md +++ b/docs/wiki/_Sidebar.md @@ -7,6 +7,7 @@ - [Contracts and Data Exchange](Contracts-And-Data-Exchange.md) - [Testing and Test Data](Testing-And-Test-Data.md) - [Agent Enablement](Agent-Enablement.md) +- [Agent Skills Distribution](Agent-Skills-Distribution.md) - [Documentation Convention](Documentation-Convention.md) - [Worked Examples](Worked-Examples.md) - [Adopting This at Your Scale](Adopting-This-At-Your-Scale.md) diff --git a/skills/purview-architecture/SKILL.md b/skills/purview-architecture/SKILL.md new file mode 100644 index 0000000..eee48ea --- /dev/null +++ b/skills/purview-architecture/SKILL.md @@ -0,0 +1,102 @@ +--- +name: purview-architecture +description: >- + Use when deciding the shape of a .NET solution that uses Purview libraries — how many projects, + whether to add a repository or CQRS layer, where validation lives — or before generating substantial + implementation code. Establishes project context first, then applies DRY, KISS and architecture + proportional to the actual requirements rather than to an enterprise reference diagram. +license: MIT +metadata: + author: Purview-Dev + version: "1.0" +--- + +# Purview architecture + +The job of this skill is to stop two failures that cost far more than they look: + +- **Architecture bought in advance.** Four projects, a repository interface and a mapping layer for an + application that has one table and one caller. +- **A recommendation made without context.** Advice about database-side GUID generation given to a + stateless CLI, or a Clean Architecture split proposed for a 300-line utility. + +So: establish context, then choose proportionally. Both steps are mandatory, and the first one is the +one that gets skipped. + +## 1. Establish context before deciding + +Before recommending a structure, adding a project, or generating substantial code, you must know four +things. **Read them out of the repository first** — asking a user a question the code already answers +is the most common way this skill is applied badly. + +| Dimension | Read it from | Ask only if | +| -- | -- | -- | +| Complexity (small / medium / large) | Number of functional areas, integrations and external consumers — not file count | The requirements are genuinely not written down anywhere | +| Project type | SDK attribute, `OutputType`, `Microsoft.NET.Sdk.Web`, an AppHost project, a `Main` | The repository is empty | +| Persistence | `PackageReference` to an EF Core provider, `DbContext`, a connection string, a `Purview.Containers` resource | Persistence is being introduced now | +| Telemetry | `Purview.Telemetry.SourceGenerator`, `ILogger` injection, `ActivitySource`, an OTLP exporter | Nothing is configured and you are about to add it | + +Identifier generation is a **conditional** dimension: resolve it only when persistence exists *and* +GUID keys are in play. See `purview-persistence`. + +Judge complexity on requirements, not size. A 2,000-line CLI with one output format is small. A +900-line service that reconciles three upstream systems and owns money movement is not. + +**Do not re-ask.** A dimension answered by repository configuration, existing source, a project file, +an earlier turn in the conversation, or a recorded architectural decision is answered. Ask concise, +targeted questions only for the unknowns that change your recommendation — and if nothing changes, +ask nothing. + +Full decision procedure: `references/project-context-discovery.md`. + +## 2. Choose proportionally + +| Scale | Shape | What you still do not add | +| -- | -- | -- | +| Small | One project, or two when a test project needs a non-trivial subject. Direct implementations. | Repository over `DbContext`, CQRS, a mapping layer, speculative interfaces | +| Medium | Explicit boundaries where complexity already hurts — usually a domain project and a host | Event sourcing, a generic infrastructure framework, a second infrastructure implementation nobody asked for | +| Large | Modularity, independent infrastructure implementations, enforceable boundaries (see the playbook's Architecture-As-Tests page) | Patterns adopted for symmetry rather than for a named problem | + +Architecture is allowed to arrive late. A boundary added when the second caller appears is cheap; a +boundary added in anticipation of a second caller that never appears is permanent. + +**Never add a project for symmetry.** "Domain, Application, Infrastructure, API" is a response to a +problem, not a starting position. If you cannot name the failure a layer prevents in this codebase, +do not add the layer. + +Full guidance and the per-pattern justification tests: `references/proportional-architecture.md`. + +## 3. DRY, applied where it pays + +Reuse the Purview libraries when they remove a real source of truth, not for coverage: + +| Concern | Library | Use it when | +| -- | -- | -- | +| Operation outcomes without exceptions for control flow | `Purview.Results` | Failures are expected and the caller must handle them | +| Primitive obsession, validated identity types | `Purview.ValueObjects` | A value has rules that would otherwise be re-checked at each boundary | +| Schema shared with a TypeScript client | `Purview.ZodSharp` | A contract is duplicated between C# and TypeScript today | +| Local dependencies for dev and integration tests | `Purview.Containers` | Tests need a real database, queue or emulator | +| Logging, tracing and metrics from one declaration | `Purview.Telemetry.SourceGenerator` | Always — see `purview-telemetry` | + +Duplication that matters: validation repeated per entry point, a domain rule restated in the UI, +configuration bound in two places, two types that both claim to be the truth. Duplication that does +not: two similar-looking methods that answer to different owners. + +## 4. Telemetry is not an advanced topic + +Recommend instrumentation from the first commit, at every project size. A small application gets +small telemetry — it does not get none. See `purview-telemetry`. + +## Checks before you answer + +- Did you read the four context dimensions rather than ask for them? +- Can you name, for each project and each abstraction you are proposing, the failure it prevents here? +- Is anything in your proposal there only because a reference architecture has it? +- Did you include telemetry? + +## See also + +- `purview-telemetry` — instrumentation at every scale +- `purview-persistence` — storage and GUID-key guidance, only when persistence is real +- [Engineering Principles](https://github.com/purview-dev/engineering-playbook/blob/main/docs/wiki/Engineering-Principles.md) +- [Adopting This At Your Scale](https://github.com/purview-dev/engineering-playbook/blob/main/docs/wiki/Adopting-This-At-Your-Scale.md) diff --git a/skills/purview-architecture/references/project-context-discovery.md b/skills/purview-architecture/references/project-context-discovery.md new file mode 100644 index 0000000..d40569a --- /dev/null +++ b/skills/purview-architecture/references/project-context-discovery.md @@ -0,0 +1,115 @@ +# Project context discovery + +The procedure for establishing enough context to make an architectural recommendation, and — just as +importantly — for knowing when to stop asking. + +## The principle + +Every question you ask a user is a question they have usually already answered in the repository. +Reading is free and silent; asking is slow and, when the answer was already in a project file, it +reads as not having looked. So the order is fixed: + +1. Read the repository. +2. Read the conversation so far. +3. Ask only about what remains *and* changes your answer. + +A dimension whose answer would not change your recommendation is not worth a question. If a project +is a stateless CLI, the database GUID strategy is not an unknown — it is irrelevant, and raising it +is noise. + +## Dimension 1 — complexity + +Assess from requirements, never from line or project count. + +| Signal | Suggests | +| -- | -- | +| One functional area, one consumer, no integrations | Small | +| A handful of features, one or two integrations, a single team | Medium | +| Several bounded areas, multiple services or teams, operational obligations (SLOs, auditability, money) | Large | + +Where to look: `README.md`, the wiki, issue or ticket references, the solution file's project list, +`PackageReference` entries naming external systems, the number of distinct hosts. + +Traps: + +- **A big repository is not a complex one.** A monorepo of ten tiny tools is ten small projects. +- **A small repository can be complex.** Few files, hard invariants. +- **Generated code inflates everything.** Discount it. + +## Dimension 2 — project type + +Read it; do not ask it. + +| Evidence | Type | +| -- | -- | +| `Microsoft.NET.Sdk.Web` | Web application or HTTP API — distinguish by whether views/pages or endpoints dominate | +| `OutputType` of `Exe` plus an argument parser | CLI | +| `IsPackable`, no entry point | Reusable library | +| `IHostedService` / `BackgroundService`, `Worker` SDK | Background worker | +| An AppHost project, `Aspire.Hosting` | Distributed application | +| WPF/WinForms/Avalonia/MAUI SDK or `UseWPF` | Desktop application | + +A reusable library carries the strongest constraint on the rest of your advice: it must not configure +global telemetry providers, must not read configuration from the ambient environment, and must not +decide a persistence technology for its consumer. + +## Dimension 3 — persistence + +| Question | Read it from | +| -- | -- | +| Is persistence required at all? | Any data-access `PackageReference`, a `DbContext`, a connection string, a repository type, an Aspire resource | +| Which technology? | The EF Core provider package (`Microsoft.EntityFrameworkCore.SqlServer`, `Npgsql.EntityFrameworkCore.PostgreSQL`), the client package, or the `Purview.Containers` resource in the AppHost | +| Is more than one implementation required? | Two providers referenced, a provider abstraction already present, or an explicitly stated requirement | +| EF Core or something else? | Presence of `Microsoft.EntityFrameworkCore` versus Dapper, a raw client, or a document store SDK | + +"No persistence" is a complete and common answer. When it is the answer, every storage-shaped +recommendation — repositories, migrations, GUID strategies, connection resilience — is out of scope. + +## Dimension 4 — telemetry + +| Question | Read it from | +| -- | -- | +| Is telemetry configured? | `ILogger` injection, `ActivitySource`/`Meter` construction, `AddOpenTelemetry`, a `ServiceDefaults` project | +| Is Purview Telemetry available? | A `Purview.Telemetry.SourceGenerator` `PackageReference`, or interfaces attributed `[Logger]`, `[ActivitySource]`, `[Meter]` | +| Which signals are relevant? | Logs always; traces once a call crosses a process or a dependency; metrics once someone would alert on a rate or a duration | +| Is there an exporter? | An OTLP endpoint in configuration, a console exporter, an Aspire dashboard, an APM package | + +Existing instrumentation is a constraint, not a starting point to replace. Preserve it, extend it, +and never register the same source twice. + +## Conditional dimension — identifier generation + +Resolve **only** when persistence exists and GUIDs are, or are about to be, primary keys. + +- Are GUIDs primary keys, or just surface identifiers? +- Is the value generated in the application or by the database? +- Which provider and version? (This changes the answer — see `purview-persistence`.) +- Does index locality matter at the expected write volume? +- Must the identifier exist before the entity is persisted (outbox, correlation, client-side graph)? + +The last question usually decides it: if the application needs the value before `SaveChanges`, the +database cannot be the generator. + +## Asking well + +When you must ask, ask few and ask concretely. Batch the unknowns into one short set, offer the +likely answers, and state what you will do if the user does not care. + +Good: + +> Two things I can't tell from the repository: is this service expected to own its own database, or +> read from the existing one? And should failures be returned to the caller or thrown? I'll assume +> its own database and `Purview.Results` for expected failures unless you say otherwise. + +Bad: + +> What is the project complexity? What is the project type? Is persistence required? Which database? +> Is telemetry configured? Do you use GUID primary keys? + +The second asks six questions, five of which the repository answers, and one of which is irrelevant. + +## Recording the answer + +When a dimension is resolved by conversation rather than by the repository, that is a decision the +next agent will not be able to read. Write it down — in `AGENTS.md`, a wiki page, or an ADR — so the +question is asked once and never again. diff --git a/skills/purview-architecture/references/proportional-architecture.md b/skills/purview-architecture/references/proportional-architecture.md new file mode 100644 index 0000000..3ed7f55 --- /dev/null +++ b/skills/purview-architecture/references/proportional-architecture.md @@ -0,0 +1,92 @@ +# Proportional architecture + +Structure is a cost paid now against a failure expected later. Proportional architecture is the +practice of only paying when the failure is named, present, and more expensive than the structure. + +## The justification test + +Before adding a project, an interface, a layer or a pattern, answer in one sentence: + +> **What failure does this prevent in *this* codebase, and has it happened or is it certain to?** + +If the sentence needs the words "in case", "later", "when we scale", or "it's good practice", the +structure is not yet justified. Write the simpler thing. The simpler thing is also the thing that is +cheapest to change when the failure finally does arrive. + +This is not an argument against structure. It is an argument against *unfalsifiable* structure. + +## By scale + +### Small — a utility, CLI, prototype, simple API, limited-scope application + +- One project. A second only when there is a test project, or when a genuinely reusable library + falls out. +- Call the database, the API or the file system directly from the code that needs it. +- Validate at the edge, once. +- Logging from the first commit; traces and metrics when there is something to correlate. + +Do not add: a repository interface over `DbContext`, a service layer that forwards, a mapping layer +between two near-identical shapes, an interface with one implementation and no test double need +(`Purview.Containers` and TUnit.Mocks remove most of that need), a `Domain`/`Application`/ +`Infrastructure` split. + +### Medium — several functional areas, some integrations, moderate domain complexity + +- Introduce a boundary where you can name what it separates. Usually that is domain rules from the + host, so a domain project and an application host. +- An abstraction over an external dependency earns its place when the dependency is genuinely + swappable, or when testing against it is otherwise painful. "Swappable in principle" is not + swappable. +- Dependency instrumentation, trace propagation across the integrations, metrics on the operations + someone would be paged about. + +Do not add: CQRS unless reads and writes have actually diverged in shape, lifetime or scaling; +event sourcing unless the history *is* the product; a second infrastructure implementation that has +no second consumer. + +### Large — complex domain, multiple services or teams, substantial operational requirements + +- Modularity along ownership lines, because the real constraint is people, not code. +- Independent infrastructure implementations where they are genuinely independent. +- Boundaries made enforceable rather than documented — a dependency-direction rule that a wrong + `ProjectReference` fails the build. See the playbook's Architecture-As-Tests page. +- End-to-end tracing, standardised resource attributes, SLIs, retention and sampling policy. + +Even here, the justification test holds per decision. "Large" licenses the *option* to add +structure, not the obligation to add all of it on day one. + +## Patterns and the conditions that earn them + +| Pattern | Earned when | Not earned by | +| -- | -- | -- | +| Repository over EF Core | You must swap the store, or the `DbContext` API genuinely leaks into domain decisions | Testability — EF Core's in-memory and container-backed testing already covers it | +| CQRS | Read and write models have diverged in shape or scaling needs | A desire for symmetry with a reference architecture | +| Event sourcing | The sequence of changes is the product, or audit is a hard requirement | "We might want history" | +| Mediator / in-process bus | Many handlers, genuinely decoupled registration | Three call sites that could be method calls | +| Mapping layer | Two shapes that must diverge and are owned separately | Two shapes that are identical today | +| Extra DI indirection | A real second implementation, or a composition root that must vary | Inversion for its own sake | +| Generic infrastructure framework | Three or more concrete uses exist now | One use and an expectation of more | + +## Evolving incrementally + +The honest sequence is: write the direct implementation; let the second caller arrive; extract the +boundary the second caller revealed. Each step is small, reversible and informed. + +The dishonest sequence is: predict the second caller; build the boundary; never get the second +caller; maintain the boundary forever. The cost is invisible because it is spread across every +future change. + +When you do extract, extract along the seam the code showed you — not the seam the diagram +predicted. + +## What this does not license + +Proportional is not an excuse for: + +- Skipping tests, or skipping telemetry. +- Leaving validation duplicated across entry points. +- Letting domain rules live in the UI or in the database. +- Two sources of truth for the same fact. + +Those are not structure bought in advance; they are defects. Simplicity means fewer moving parts, +not fewer guarantees. diff --git a/skills/purview-persistence/SKILL.md b/skills/purview-persistence/SKILL.md new file mode 100644 index 0000000..4667fa2 --- /dev/null +++ b/skills/purview-persistence/SKILL.md @@ -0,0 +1,106 @@ +--- +name: purview-persistence +description: >- + Use when a .NET project actually persists data and you are choosing a storage approach, deciding + whether to abstract EF Core, or selecting a GUID primary-key strategy. Gives provider-specific + guidance — SQL Server and PostgreSQL differ and must not be treated alike — and stays silent for + projects that do not persist anything. +license: MIT +metadata: + author: Purview-Dev + version: "1.0" +--- + +# Purview persistence + +**Activation condition:** this skill applies only when persistence is real. If the project is a +stateless CLI, a pure library, or a service that calls an API and stores nothing, stop here — none of +this is relevant, and raising a database GUID strategy with that project is noise. + +Confirm persistence from the repository before proceeding (see `purview-architecture`): a data-access +`PackageReference`, a `DbContext`, a connection string, or an explicit requirement to store state. + +## 1. Shape the data access proportionally + +| Scale | Shape | +| -- | -- | +| Small | `DbContext` used directly from the code that needs it. No repository, no unit-of-work wrapper — `DbContext` already is both. | +| Medium | A boundary when domain rules must not know about EF Core, or when a query is complex enough to deserve a name. Often that is a few focused query types, not a generic `IRepository`. | +| Large | Independent infrastructure implementations where there genuinely are several, with the dependency direction enforced by a test. | + +**Do not wrap EF Core in a repository for testability.** `Purview.Containers` gives you a real +database in tests, which is both more faithful and less code than an abstraction built to be mocked. +A repository earns its place when you must swap the store or when `DbContext` is genuinely leaking +into domain decisions — not before. + +## 2. Local dependencies + +`Purview.Containers` provides Aspire resource integrations for the dependencies you need in +development and integration tests — including `MsSql`, `PostgreSql`, `MySql`, `Redis`, `Nats`, +`RabbitMq`, `Azurite` and an Azure Key Vault emulator. Prefer a real container over an in-memory +provider: the in-memory provider does not enforce the constraints that break you in production. + +## 3. GUID keys — only if GUIDs are keys + +Resolve these before recommending anything; most are answered by the repository: + +1. Are GUIDs primary keys, or merely external identifiers over an integer key? +2. Is the value generated by the application or by the database? +3. Which provider, and which version? +4. Does index locality matter at the expected write volume? +5. Must the identifier exist before the row is persisted? + +Question 5 usually decides it. If the application needs the identifier before `SaveChanges` — for an +outbox row, a correlation value, or to build an object graph client-side — the database cannot be the +generator, and the remaining questions are about *which* client-side strategy. + +### The rule that matters most + +**SQL Server and PostgreSQL do not benefit from the same sequential GUID implementation.** Their +index structures and their GUID comparison semantics differ, and a "sequential GUID" that helps one +can be no better than random on the other. Never carry a strategy from one to the other without +re-deriving it. + +- SQL Server specifics: `references/sqlserver.md` +- PostgreSQL specifics: `references/postgresql.md` +- Choosing between them, and when a custom generator is justified: `references/guid-generation.md` + +### Prefer built-in capability + +Use the framework or database capability when it is sufficient — `Guid.CreateVersion7()` on .NET 9+, +`NEWSEQUENTIALID()`, EF Core's own value generators, a provider's native UUIDv7 function. A custom +generator is a maintenance obligation and a correctness risk; introduce one only against a measured +requirement. + +**There is no Purview GUID generation package.** Do not reference one, and do not create one without +first establishing a concrete, measured need that the built-in options cannot meet. If such a +capability is added later, this skill is where the guidance for recommending it belongs. + +## 4. Value objects as keys + +`Purview.ValueObjects` gives strongly-typed identifiers that stop an `OrderId` being passed where a +`CustomerId` belongs. They need an EF Core value converter; configure it once in the model rather +than per property. + +This is about type safety at the boundary, not about the generation strategy — the two decisions are +independent. + +## 5. Migrations and schema + +- Migrations are code: reviewed, tested against a real container, and applied by a deliberate step — + not by the application at startup in production. +- Expect the schema to be ahead of or behind the code during a deployment; design changes to be + compatible in both directions. +- Keep the connection string out of source. Keep it out of telemetry too. + +## Checks before you answer + +- Have you confirmed persistence is actually required? +- If you are recommending a GUID strategy, do you know the provider *and* its version? +- Are you proposing an abstraction you can justify by something other than testability? +- Have you preferred a built-in capability over a custom generator? + +## See also + +- `purview-architecture` — context discovery and proportional structure +- `purview-telemetry` — instrument the data access too; it is where the latency is diff --git a/skills/purview-persistence/references/guid-generation.md b/skills/purview-persistence/references/guid-generation.md new file mode 100644 index 0000000..432053a --- /dev/null +++ b/skills/purview-persistence/references/guid-generation.md @@ -0,0 +1,93 @@ +# Choosing a GUID generation strategy + +The cross-provider decision procedure. Provider detail lives in `sqlserver.md` and `postgresql.md`. + +## Step 0 — is this question in scope? + +Ask nothing and recommend nothing unless **both** are true: + +- The project persists data, and +- GUIDs are (or are about to be) primary keys. + +A stateless CLI, a pure library, a service with an integer key, or a project using GUIDs only as +external correlation identifiers does not need this decision. Raising it anyway is the failure this +page is written to prevent. + +## Step 1 — must the identifier exist before persistence? + +This question decides more than any other. + +**Yes** — the application needs the value before `SaveChanges` for an outbox row, a correlation +identifier, an object graph built client-side, or an identifier returned before commit. The database +cannot be the generator. Go to step 2 with client-side options only. + +**No** — the database may generate it, and database generation is available to you. + +## Step 2 — which provider, and which version? + +Do not skip the version. It changes the available options materially: + +| Provider | Version matters because | +| -- | -- | +| SQL Server | `NEWSEQUENTIALID()` is long-standing; the comparison semantics that make byte-ordered UUIDs useless have never changed | +| PostgreSQL | Native `uuidv7()` exists only from 18; `gen_random_uuid()` is built in from 13 | +| .NET | `Guid.CreateVersion7()` exists from .NET 9 | + +**The two databases do not share an answer.** SQL Server compares `uniqueidentifier` with the last +six bytes most significant, so a byte-ordered UUIDv7 is effectively random to its clustered index. +PostgreSQL compares `uuid` in byte order, so UUIDv7 is genuinely sequential there. A strategy that is +optimal on one is unremarkable or wrong on the other. + +## Step 3 — does locality actually matter? + +Sequential keys exist to reduce index fragmentation and page splits under write pressure. If the +table takes tens of inserts a minute, this is not your problem and a random GUID is the simpler, +more private choice. + +Reach for a sequential strategy when you can point at write volume, measured fragmentation, page +splits, or buffer-pool pressure. "It's best practice" is not a measurement. + +Note also that on SQL Server the problem is often better solved by making the clustered index +something other than the GUID key — see `sqlserver.md`. + +## Step 4 — is unpredictability required? + +Sequential and time-ordered identifiers are guessable, and UUIDv7 discloses creation time to the +millisecond. Where an identifier appears in a URL, is handed to a third party, or acts as a +capability, use a random UUID — or keep the ordered value internal and expose a random surrogate. + +Never use `NEWSEQUENTIALID()` output as a secret. + +## Decision summary + +| Provider | Needs value before insert | Locality matters | Recommendation | +| -- | -- | -- | -- | +| SQL Server | No | Yes | `NEWSEQUENTIALID()` (EF Core's default for `Guid` keys) | +| SQL Server | Yes | Yes | EF Core `SequentialGuidValueGenerator` | +| SQL Server | Either | No | `Guid.NewGuid()` | +| PostgreSQL | Yes | Yes | `Guid.CreateVersion7()` with `ValueGeneratedNever()` | +| PostgreSQL | No | Yes | `uuidv7()` on 18+, otherwise generate v7 in the application | +| PostgreSQL | Either | No | `Guid.NewGuid()` or `gen_random_uuid()` | +| Any | Either | Either, but must be unpredictable | Random v4 | + +## Prefer built-in capability + +In order of preference: + +1. A framework capability — `Guid.CreateVersion7()`, EF Core's value generators. +2. A database capability — `NEWSEQUENTIALID()`, `uuidv7()`, `gen_random_uuid()`. +3. A reviewed, tested implementation of a published standard, when neither of the above is available + on your target versions. +4. A bespoke generator — only against a measured requirement the first three cannot meet. + +A hand-written sequential GUID generator is easy to get subtly wrong — endianness, clock regression, +counter rollover within a millisecond, and thread safety are all live hazards, and the failure mode +is duplicate or badly-ordered keys found in production. + +## There is no Purview GUID package + +No Purview library provides GUID generation today. Do not reference one and do not invent one. + +If a concrete need emerges that the built-in options cannot meet — established by measurement, not by +anticipation — that is the point at which creating one is worth discussing. Until then this guidance +is the deliverable, and this page is where a recommendation for such a package would be documented. diff --git a/skills/purview-persistence/references/postgresql.md b/skills/purview-persistence/references/postgresql.md new file mode 100644 index 0000000..cd557ec --- /dev/null +++ b/skills/purview-persistence/references/postgresql.md @@ -0,0 +1,98 @@ +# PostgreSQL UUID keys + +Read this only when the provider is PostgreSQL and UUIDs are primary keys. The SQL Server answer is +different — see `sqlserver.md`. + +## Why PostgreSQL differs from SQL Server + +Two differences drive everything on this page: + +1. **PostgreSQL has no clustered index.** Tables are heaps; the primary key is a separate B-tree. A + random UUID therefore does not cause row-level page splits the way it does on a clustered SQL + Server table. The cost is in the index: random inserts touch scattered index pages, which hurts + cache locality and WAL volume, but it is a smaller and differently-shaped problem. +2. **`uuid` compares in byte order.** Unlike SQL Server, PostgreSQL orders UUIDs by their bytes. A + UUID that is sequential in byte layout *is* sequential to the index. + +The consequence: **UUIDv7 works here.** It is the natural answer on PostgreSQL and the wrong answer +on SQL Server. Never carry a sequential-GUID choice across the two. + +## The options + +### `Guid.CreateVersion7()` — .NET 9+, application-generated + +The default recommendation for new PostgreSQL schemas with UUID keys. + +```csharp +public sealed class Order +{ + public Guid Id { get; init; } = Guid.CreateVersion7(); +} +``` + +- Time-ordered in byte layout, so index locality is good. +- The value exists before persistence, so outbox rows, correlation and client-side graphs all work. +- Standard (RFC 9562), so it is portable and readable by other systems. +- Embeds a millisecond timestamp — which is also its disclosure caveat. + +Tell EF Core the application supplies it: + +```csharp +builder.Property(o => o.Id).ValueGeneratedNever(); +``` + +Without that, EF Core may treat the key as store-generated and skip your value. + +### Database-generated UUIDv7 — PostgreSQL 18+ + +PostgreSQL 18 added a native `uuidv7()` function. Earlier versions do not have it; on 13–17 you have +`gen_random_uuid()` (v4, from the built-in pgcrypto-derived function) and nothing native for v7. + +**Check the server version before recommending `uuidv7()`.** It is the single most common way this +guidance goes wrong — the function simply does not exist on the version most deployments are running. + +```sql +-- PostgreSQL 18 and later only +ALTER TABLE orders ALTER COLUMN id SET DEFAULT uuidv7(); +``` + +Prefer application-side generation anyway unless rows are inserted by paths that bypass the +application, because the value-before-persistence property is worth more than the marginal +difference. + +### `gen_random_uuid()` — database-generated v4 + +Available without an extension from PostgreSQL 13. Random, so index locality is poor, but +unpredictable and simple. + +### `Guid.NewGuid()` — application-generated v4 + +Random and unpredictable. Fine at low to moderate write volume, which covers most applications. + +## Npgsql and EF Core notes + +- `Guid` maps to `uuid` natively. Do not store UUIDs as `text` or `varchar` — it costs space, loses + index efficiency and defeats comparison. +- Npgsql's `uuid` mapping is byte-order faithful, so a v7 value sorts in the index as you expect. +- If the application generates the key, set `ValueGeneratedNever()` so EF Core does not fetch a + store-generated value that does not exist. +- A legacy database may have UUIDs in `text` columns. Migrating to `uuid` is usually worth it, but it + is a data migration, not a model change. + +## Choosing + +| Situation | Use | +| -- | -- | +| New schema, .NET 9+, UUID keys | `Guid.CreateVersion7()`, `ValueGeneratedNever()` | +| .NET 8 or earlier, high write volume | A reviewed UUIDv7 implementation, or accept v4 after measuring | +| Rows inserted outside the application, PostgreSQL 18+ | `uuidv7()` as a column default | +| Rows inserted outside the application, PostgreSQL 13–17 | `gen_random_uuid()` | +| Key must be unpredictable | `Guid.NewGuid()` or `gen_random_uuid()` — v7 leaks creation time | +| Low or moderate volume | v4 — the locality difference will not be your bottleneck | + +## The timestamp caveat + +A UUIDv7 exposes when the row was created, to the millisecond. Where identifiers are public, that can +leak signup times, order rates or activity patterns — and it makes identifiers partially guessable by +time window. Where that matters, use a random UUID for the public identifier and keep the v7 value +internal, or use a surrogate. diff --git a/skills/purview-persistence/references/sqlserver.md b/skills/purview-persistence/references/sqlserver.md new file mode 100644 index 0000000..c728295 --- /dev/null +++ b/skills/purview-persistence/references/sqlserver.md @@ -0,0 +1,101 @@ +# SQL Server GUID keys + +Read this only when the provider is SQL Server and GUIDs are primary keys. The PostgreSQL answer is +different — see `postgresql.md`. + +## Why SQL Server is a special case + +A clustered index determines physical row order. A `uniqueidentifier` primary key is clustered by +default, so random GUIDs insert into the middle of the index, causing page splits, fragmentation and +a larger buffer-pool working set. On a write-heavy table this is measurable. + +The complication is that **SQL Server does not compare `uniqueidentifier` values the way the bytes +are laid out.** Its ordering weights the last six bytes (node field) most significantly, then earlier +groups. So a GUID that is sequential in big-endian byte order — including a standard UUIDv7 — is +**not** sequential under SQL Server's comparison. This is precisely why the SQL Server and PostgreSQL +answers diverge. + +## The options + +### `NEWSEQUENTIALID()` — database-generated + +A column default that produces values ascending in SQL Server's own ordering. Best index locality +available. + +```sql +ALTER TABLE [Orders] ADD CONSTRAINT [DF_Orders_Id] + DEFAULT NEWSEQUENTIALID() FOR [Id]; +``` + +EF Core, for a `Guid` key on SQL Server, configures this by default — `ValueGeneratedOnAdd` maps to +`NEWSEQUENTIALID()` through the default value-generation strategy. + +Trade-offs: + +- The value does not exist until the row is inserted. EF Core retrieves it, but you cannot build an + object graph around the key beforehand. +- Values are **not** unpredictable: the sequence restarts on service restart but is otherwise + guessable. Never use one as a security token or capability URL. +- It reveals rough insert ordering, which can be an information-disclosure concern on a public + surface. + +### EF Core's `SequentialGuidValueGenerator` — client-generated + +EF Core ships a value generator that produces GUIDs ordered for SQL Server's comparison semantics, +generated in the application. + +```csharp +builder.Property(o => o.Id) + .HasValueGenerator() + .ValueGeneratedOnAdd(); +``` + +Use this when the identifier must exist before `SaveChanges`. It keeps most of the index locality +while letting the application own the value. + +### `Guid.NewGuid()` — random + +Correct, portable and unpredictable; poor index locality. + +Appropriate when write volume is low (the fragmentation never matters), when the key is +non-clustered, or when unpredictability is a requirement. Do not dismiss it by default — for most +applications it is fine, and "most applications" includes more than people expect. + +### `Guid.CreateVersion7()` — .NET 9+ + +Ordered in byte layout, but **not** ordered under SQL Server's comparison. On SQL Server it behaves +roughly like a random GUID for clustered-index purposes. It is the right answer on PostgreSQL and the +wrong answer here, which is the trap this page exists to prevent. + +Use it on SQL Server only when you want the embedded timestamp for application reasons and accept +random-like insert locality. + +## Decoupling the key from the index + +Often the best answer is not a different GUID but a different index. Keep the GUID as the primary key +and make the clustered index something naturally ascending: + +```csharp +builder.HasKey(o => o.Id).IsClustered(false); +builder.HasIndex(o => o.CreatedAt).IsClustered(); +``` + +This removes the page-split problem entirely and leaves the key unpredictable. Consider it before +reaching for a sequential generator. + +## Choosing + +| Situation | Use | +| -- | -- | +| High insert volume, key may be database-generated | `NEWSEQUENTIALID()` | +| High insert volume, identifier needed before persisting | EF Core `SequentialGuidValueGenerator` | +| Key must be unpredictable (public surface, security boundary) | `Guid.NewGuid()`, with a non-clustered PK | +| Low or moderate volume | `Guid.NewGuid()` — do not optimise what does not hurt | +| You want a sortable timestamp in the key and accept the locality cost | `Guid.CreateVersion7()` | + +## Measure before optimising + +Index fragmentation from random GUIDs is real but frequently irrelevant. Before changing a key +strategy, confirm it matters: check page splits, fragmentation and buffer-pool pressure on the actual +table at the actual volume. Changing a primary key strategy on a live table is expensive; do it +against evidence. diff --git a/skills/purview-telemetry/SKILL.md b/skills/purview-telemetry/SKILL.md new file mode 100644 index 0000000..6d83173 --- /dev/null +++ b/skills/purview-telemetry/SKILL.md @@ -0,0 +1,123 @@ +--- +name: purview-telemetry +description: >- + Use when adding, reviewing or scaling observability in a .NET project — logging, tracing, metrics, + OpenTelemetry wiring — or when deciding whether a project is "too small for telemetry". Recommends + Purview.Telemetry.SourceGenerator at every project size, keeps libraries free of global provider + configuration, and scales the export infrastructure rather than the instrumentation. +license: MIT +metadata: + author: Purview-Dev + version: "1.0" +--- + +# Purview telemetry + +**Telemetry is a baseline, not an advanced topic.** Instrument from the first commit at every project +size. What changes with scale is the *export and operations* story — collectors, dashboards, sampling +— not whether the code emits anything. + +A project that defers telemetry until it becomes distributed acquires its observability at exactly +the moment it can least afford the refactor. + +## Instrumentation versus export + +Keep these separate; conflating them is what produces "we're too small for telemetry". + +| | What it is | Who owns it | +| -- | -- | -- | +| **Instrumentation** | The log, span and measurement the code emits | Every project, every size, from day one | +| **Export** | Where signals go: console, OTLP, a backend, a dashboard | The application host, sized to the operation | + +A small CLI is fully instrumented and exports to the console. That is not a reduced form of +telemetry; it is the correctly sized export for a CLI. + +## Use the generator + +`Purview.Telemetry.SourceGenerator` turns an attributed interface into logging, tracing and metrics +implementations plus a DI registration extension. One declaration, three signals, no hand-written +`ILogger` message templates or `ActivitySource` plumbing. + +Do **not** reinvent its API here. The three shipped skills are authoritative and travel with the +package: + +- `telemetry-sourcegenerator-quickstart` — adding the package, the first interface, DI registration, + and publishing meter/activity-source names to the host via `TelemetryNames` +- `telemetry-sourcegenerator-design` — designing telemetry interfaces well +- `telemetry-sourcegenerator-migration` — moving existing `ILogger`/`ActivitySource`/metrics code onto + the generator + +Recommend the generator whenever the project targets a compatible TFM and the team is willing to take +an analyzer-only package reference. Where it is not compatible, fall back to the standard .NET +abstractions below — never to a bespoke logging wrapper. + +## Libraries versus hosts + +This is the rule most often broken, and it is not negotiable. + +**A reusable library:** + +- Emits through `ILogger`, `ActivitySource` and `Meter` — or through generated telemetry + interfaces, which produce exactly those. +- Publishes its activity-source and meter names so a host can subscribe. The generator does this for + you through assembly metadata. +- **Never** calls `AddOpenTelemetry`, configures an exporter, sets a sampler, or requires a specific + backend. +- Never reads an OTLP endpoint from configuration itself. + +**The application host:** + +- Owns provider configuration, exporters, sampling and resource attributes. +- Subscribes to the sources its libraries publish — with the generator, by opting into the + `TelemetryNames` class and passing its arrays to the OpenTelemetry builder. + +A library that configures a global provider breaks every consumer that already configured one, and +silently wins or loses depending on initialisation order. + +## Sizing the export + +| Scale | Export and operations | +| -- | -- | +| **Small** | Console or a single OTLP endpoint. Structured logs, exceptions recorded on the span. No collector, no dashboards, no custom exporter, no distributed tracing infrastructure. | +| **Medium** | OTLP export, dependency instrumentation (HTTP, EF Core), trace propagation across the integrations, application metrics, health checks, a sampling decision made deliberately. | +| **Large** | End-to-end distributed tracing, standardised resource attributes, operational metrics, dashboards, alerting, SLIs, documented retention and sampling policy. | + +Full detail per level: `references/observability-levels.md`. + +Instrumentation is identical across all three rows. Only the right-hand column grows. + +## Baseline for any application + +- Structured logging — parameters as fields, never interpolated into the message. +- Errors and exceptions recorded as telemetry, with the exception attached to the span. +- Operation correlation, so one unit of work is one trace. +- OpenTelemetry-compatible instrumentation, so the backend stays a choice. +- Log level configurable at runtime. +- Export behaviour configurable per environment. +- Sensitive values — credentials, tokens, connection strings, personal data — never logged or placed + on span attributes or baggage. + +## Before you change anything + +1. **Inspect what exists.** Find current `ILogger` use, `ActivitySource`/`Meter` instances, any + `AddOpenTelemetry` call, a `ServiceDefaults` project. +2. **Preserve it.** Extend existing instrumentation; do not replace working telemetry to adopt a + library. +3. **Do not duplicate registrations.** One registration per source; adding a second is a silent + double-count on every metric. +4. **Size the configuration to the project**, using the table above. +5. **Check for leaking secrets** in anything you add. +6. **Keep instrumentation and export distinct** in what you recommend. + +## Do not + +- Omit telemetry because the application is small. +- Invent Purview telemetry APIs — read the three shipped skills instead. +- Configure a global provider from a library. +- Recommend a collector, dashboard stack or custom exporter for a small project. +- Put a user identifier, token or connection string into a log field, span attribute or baggage item. + +## See also + +- `purview-architecture` — establishing project context and scale before deciding +- [Purview.Telemetry.SourceGenerator](https://github.com/purview-dev/purview-telemetry-sourcegenerator) diff --git a/skills/purview-telemetry/references/observability-levels.md b/skills/purview-telemetry/references/observability-levels.md new file mode 100644 index 0000000..df97d78 --- /dev/null +++ b/skills/purview-telemetry/references/observability-levels.md @@ -0,0 +1,93 @@ +# Observability levels + +Instrumentation is constant across these levels. Export, infrastructure and operational practice are +what scale. Read this after establishing project complexity — see `purview-architecture`. + +## Small + +*A utility, CLI, prototype, simple API or limited-scope application.* + +**Do:** + +- Structured logging with parameters as fields. +- Exceptions recorded with enough context to reproduce. +- An `ActivitySource` around the unit of work, so a failure has a shape and a duration. +- Console export, or a single OTLP endpoint when one already exists. +- Log level from configuration, so a user can raise it without a rebuild. + +**Do not:** + +- Stand up a collector. +- Adopt a centralised monitoring platform. +- Build distributed tracing infrastructure for a single process. +- Create dashboards nobody reads. +- Write a custom exporter. + +The test: a user reports a failure and you can ask for one log output that tells you what happened. + +## Medium + +*Several functional areas, integrations, moderate domain complexity.* + +Everything above, plus: + +- **Dependency instrumentation** — HTTP clients, EF Core, messaging. These are where latency and + failure actually live, and the instrumentation is a package reference, not code. +- **Trace propagation** across every integration boundary, so a trace survives the process hop. W3C + `traceparent` by default; do not invent a correlation header. +- **Application metrics** for the operations someone would notice: request rate, error rate, duration, + queue depth. Prefer histograms over averages. +- **OTLP export** to a real backend. +- **Health checks**, distinguishing liveness from readiness. +- **A deliberate sampling decision.** Head sampling at a fixed ratio is fine; the point is that it was + chosen rather than defaulted into. + +Common mistake at this level: adding metrics for everything measurable instead of the few that drive +a decision. A metric nobody alerts or decides on is cost without signal. + +## Large + +*Complex domain, multiple services or teams, substantial operational requirements.* + +Everything above, plus: + +- **End-to-end distributed tracing**, including asynchronous hops — message producers and consumers + linked, not orphaned. +- **Standardised resource attributes** — `service.name`, `service.version`, `deployment.environment` + — applied uniformly so cross-service queries work at all. Follow OpenTelemetry semantic conventions + rather than inventing names. +- **Operational metrics** beyond application ones: saturation, dependency health, cost drivers. +- **Dashboards** owned by the team that gets paged. +- **Alerting** on symptoms users feel, not on causes. Alert on error rate and latency; investigate + with CPU and queue depth. +- **Service-level indicators** derived from the telemetry, with objectives agreed with whoever cares. +- **Retention and sampling policy**, documented and costed. Tail sampling if error traces must always + survive. + +## Signal selection + +| Signal | Add when | Warning | +| -- | -- | -- | +| Logs | Always | Unstructured logs are not telemetry; they are strings | +| Traces | Work crosses a process, a dependency or an async boundary | A span per method produces noise, not insight | +| Metrics | Someone would alert on, or decide from, a rate or duration | High-cardinality attributes (user id, request id) are a cost incident | + +Put high-cardinality context on spans and logs, never on metric dimensions. + +## Sensitive information + +At every level, and without exception: + +- No credentials, tokens, keys or connection strings in messages, attributes or baggage. +- No personal data beyond what the system is entitled to process — and where an identifier is + necessary, prefer a pseudonymous one. +- Remember that **baggage propagates across service boundaries**; treat it as the most public place + in your telemetry. +- Redact at the point of emission. A redaction step in the pipeline is a backstop, not a control. + +## Export configuration + +The host owns this. Keep the endpoint, protocol, sampling ratio and resource attributes in +configuration, not in code, so the same build runs in every environment. + +Libraries contribute names and signals; they never contribute configuration.