# Reviewing documentation

Doc gen produces a lot of pages quickly, and the agent already runs its own
review and deterministic validation passes. Review for **both content and
format**. The agent is non-deterministic and routinely takes shortcuts with
formatting — a page can pass the automated gates and still be structurally
sloppy — and nothing except a person who knows the system can tell you that a
documented business rule is wrong.

## What the tooling checks first

Before a run completes, the tooling runs scripted checks
(`Invoke-StructuralReview.ps1`, `Invoke-ContentLint.ps1`,
`Invoke-DependencyResolutionCheck.ps1`) plus a Documentation Reviewer agent
pass. Between them they enforce roughly 65 rules covering:

- Required sections, section order, and exactly one H1 per page.
- `## Sources` being the terminal section on every page.
- Internal links and anchors resolving, including business rule ID links.
- Mermaid diagrams using standard ```` ```mermaid ```` fences — legacy Azure
  DevOps `:::` fences are a **blocker**.
- YAML frontmatter fields, and `library` holding the canonical dotted name
  (`SAIF.Framework.Core`) rather than a folder slug.
- Placeholder text: `TODO`, `TBD`, `See source`, ellipses, and dangling VB line
  continuations are flagged as `major`.

Findings are graded `blocker`, `major`, or `minor`. The orchestrator
auto-creates fix tasks and re-reviews, up to five remediation passes.

!!! warning "A clean automated pass is not a correct document"
    Treat these rules as a floor, not a guarantee. The remediation loop is
    driven by an agent, so fixes are applied inconsistently — a finding can be
    marked resolved while the underlying formatting problem is still there, or
    "fixed" by deleting content rather than correcting it. And none of the rules
    verify that the prose matches the code.

## Universal review pass

Regardless of documentation type, spot-check these on a sample of pages:

1. **Formatting actually holds up.** Read a few full pages rather than skimming.
   Watch for tables with mismatched columns, headings that skip a level, code
   fences that were never closed, Mermaid diagrams that render as raw text, and
   sections that exist but are effectively empty. Anything the agent found
   tedious to format correctly is where it cut corners.
2. **Sources are real.** Open two or three `## Sources` entries per document and
   confirm the referenced file actually contains what the page claims.
3. **Confidence markers are honest.** Inferred (`*`) and unknown (`**`)
   statements must carry the note box before `## Sources`. Anything asserted
   flatly should be verifiable in source.
4. **Business rules are rules.** `business-rules.md` should describe *business*
   behavior with BR IDs, not restate implementation detail.
5. **Dependencies match reality.** Cross-check `dependencies.md` against the
   project file. Reference Type must be `Project Reference`, `NuGet Package`, or
   `NuGet Package (same-repo)` — vague values like "Internal library" are wrong.
6. **The manifest is right.** `docgen-manifest.md` must exist at the repository
   root, and its documentation type plus classification basis must be correct.
   **This value decides where the repository lands in the portal**, so a wrong
   type is worth blocking on.
7. **No leakage of internal artifacts.** No `review-report.md` under `docs/`, no
   `packages.md` outside the repository root, and no references to dot-prefixed
   folders such as `.saifdocgen`.

## By documentation type

!!! tip "Compare against a worked example"
    [Document examples](examples/index.md) has a complete, correct example of
    every document type below, grouped the same way. Open the matching example
    beside the page you are reviewing.

=== "Framework / Component"

    These are library-centric. Documentation lives in `docs/libraries/<slug>/`.

    - **API reference completeness.** Every public type documented under
      `api/<namespace>/<Type>.md` must appear in the `api/index.md` Types table,
      and every name in that table must link to its page.
    - **Signatures are complete.** Truncated or continuation-mangled signatures
      are the most common defect in VB-heavy libraries. Scan for anything ending
      mid-expression.
    - **Architecture diagram exists and is meaningful.** `architecture.md` needs
      a `## Architecture diagram`. A single-node diagram is acceptable for a
      trivial library, but check that the position in the stack is right.
    - **Inherited behavior.** For framework-dense libraries, missing inherited
      sections are a blocker — verify base class behavior is actually described,
      not just named.
    - **`packages.md`** should exist only when the repository publishes NuGet
      packages, must sit at the repository root, and every library it lists must
      link to that library's `overview.md`.
    - **.NET (NCore) libraries.** `packages.md` reports the pack method, target
      frameworks, and bundled assemblies — there is no `.nuspec` to cite.
      `configuration.md` documents `IConfiguration` and options binding, not an
      `<appSettings>` catalog. When the library exposes `IServiceCollection` or
      middleware extensions, `service-registration.md` must exist and name the
      lifetime of every service registered.

    Examples: [overview](examples/framework-overview.md) ·
    [architecture](examples/framework-architecture.md) ·
    [API reference](examples/framework-api-reference.md) ·
    [business rules](examples/framework-business-rules.md) ·
    [operations and processes](examples/framework-operations-processes.md) ·
    [operation document](examples/framework-operation-document.md) ·
    [dependencies](examples/framework-dependencies.md) ·
    [configuration](examples/framework-configuration.md)

    .NET (NCore) library examples: [service registration](examples/ncore-service-registration.md) ·
    [configuration](examples/ncore-configuration.md) ·
    [packages](examples/ncore-packages.md)

=== "Front End"

    Web front ends add `docs/application/` and `docs/screens/` on top of the
    library documentation. **Check which variant you are reviewing first** — BOF
    WebForms, .NET Framework 4 MVC / Razor, or ASP.NET Core Razor Pages — and
    review against that variant's expectations. The bullets below are for BOF
    WebForms; the Razor Pages checks follow them.

    - **Screen coverage.** Every non-utility screen needs a
      `docs/screens/<slug>.md`, and `docs/screens/index.md` must list all of
      them with Name, Purpose, Entry Path, and Infragistics columns.
    - **Screen sources.** Each screen document must cite the real `.aspx` and
      its code-behind (`.aspx.vb` / `.aspx.cs`). Sources pointing at analysis
      files are wrong.
    - **Navigation is accurate.** Navigation targets must resolve to sibling
      screen documents. This is the section most worth manually walking against
      the running application.
    - **Screen flow diagrams.** Main screens (3+ outbound navigations) need a
      `## Screen flow diagram`; secondary screens should not have one.
    - **Infragistics section.** Present only when the screen actually uses
      Infragistics controls.
    - **Application-level pages.** Check `docs/application/business-rules.md`
      anchors resolve from screen documents, and that `tech-debt.md` reflects
      genuine debt — for .NET Framework web apps an NTLM authentication finding
      is expected.
    - **API and cross-app dependencies.** If screens call web services or
      navigate to other applications, `dependencies.md` must carry
      `## API dependencies` and `## External web applications`.

    Examples: [screen](examples/frontend-screen.md) ·
    [application overview](examples/frontend-app-overview.md) ·
    [application business rules](examples/frontend-app-business-rules.md) ·
    [application dependencies](examples/frontend-app-dependencies.md) ·
    [tech debt](examples/frontend-app-tech-debt.md)

    **ASP.NET Core Razor Pages** applications are reviewed differently:

    - **Screens are `@page` files.** The screen index must reconcile against the
      `@page` files, with scaffold pages excluded. A Razor Pages screen has no
      Infragistics section, and WebForms vocabulary — postback, `ViewState`,
      `.aspx`, Infragistics — anywhere in the set is a defect.
    - **Routes and operations are real.** The Route table must match the file
      path, `@page` template, and any `AddPageRoute` convention, and operations
      must be written `VERB /<Page>?handler=<Name>`.
    - **Package behavior is marked.** Access, layout, and start-up behavior that
      comes from a package base class (`BolPolicyModel`, `BolRazorLayoutStartup`)
      must carry `†` or `**`, not be presented as application code.
    - **Anti-forgery is stated** for the POST handlers, and the architecture
      page covers the middleware order and the Microsoft DI composition root.
    - **Hosted endpoints stay in one place.** JSON, partial, and file handlers
      are catalogued on `docs/application/hosted-endpoints.md` and linked from
      screens; a `docs/apis/` folder in a front-end repository is a defect.

    Razor Pages examples: [screen](examples/ncore-screen.md) ·
    [application architecture](examples/ncore-app-architecture.md) ·
    [hosted endpoints](examples/hosted-endpoints.md)

    **.NET Framework 4 MVC / Razor** applications use the same ideas with MVC
    vocabulary. Screens are controller actions that return full views and are
    reconciled against what the project compiles, not what is on disk; the
    composition root is Autofac in a `SAIF.DependencyMaps.Web.*` companion
    project; and WebForms vocabulary (`postback`, `ViewState`, `.aspx`, an
    Infragistics section) is a blocker.

    .NET Framework 4 MVC example: [screen](examples/net4-screen.md)

=== "Console App"

    Console and batch applications add `docs/applications/<app-slug>/`.

    - **Check which console family you are reviewing first.** A BOF batch
      application has `batch-lifecycle.md` and numeric return codes; a .NET
      Framework console application has `run-lifecycle.md`, `arguments.md`, and
      exit codes inherited from `SaifConsoleException`. Reviewing one against
      the other's expectations produces false findings.
    - **Lifecycle accuracy.** The lifecycle document should match how the job is
      actually scheduled and invoked. Verify against the scheduler
      configuration, not just the code.
    - **Arguments are named, not positional** for .NET Framework console apps.
      A table with a Position column is a defect there, and `arguments.md` must
      record what happens when resolution fails.
    - **Operational runbook is actionable.** Failure modes, restart behavior,
      and manual intervention steps must be specific enough for an on-call
      engineer who has never seen the job.
    - **Configuration and inputs.** Confirm connection strings, file paths, and
      arguments listed are the ones actually used per environment.
    - **Data model / SQL.** Every `sql/SQL-*.md` must use the template's exact
      Classification field names, real parameter names, and real SQL. Generic
      placeholders are a defect.

    BOF examples: [overview](examples/console-overview.md) ·
    [batch lifecycle](examples/console-batch-lifecycle.md) ·
    [business rules](examples/console-business-rules.md) ·
    [dependencies](examples/console-dependencies.md) ·
    [operational runbook](examples/console-operational-runbook.md) ·
    [data model](examples/console-data-model.md)

    .NET Framework examples: [overview](examples/net4-console-overview.md) ·
    [run lifecycle](examples/net4-console-run-lifecycle.md) ·
    [arguments](examples/net4-console-arguments.md) ·
    [configuration](examples/net4-console-configuration.md) ·
    [dependencies](examples/net4-console-dependencies.md) ·
    [operational runbook](examples/net4-console-operational-runbook.md)

=== "Rest API"

    REST services document their endpoints under `docs/apis/<api-slug>/`, and
    any package that wraps one under `docs/clients/<client-slug>/`.

    - **No screens, no application folder.** A REST API has no user interface.
      A `docs/screens/` folder or an `application/` folder in an API repository
      means the repository was classified as a front end by mistake — stop and
      report it rather than reviewing the contents.
    - **Endpoint coverage.** Every controller in the source needs a page under
      `endpoints/`, every page must appear in the `index.md` Controllers table,
      and every route in `endpoints.md` must resolve to one of those pages.
    - **Routes are real.** Attribute routes must match the `[Route]` and
      `[RoutePrefix]` values in source, including the version segment. Invented
      or tidied-up routes are the failure mode to watch for.
    - **Request and response shapes are concrete.** Each endpoint needs its
      actual DTO types and status codes, not a generic success/failure table.
    - **Authorization is sourced.** `authentication-authorization.md` must name
      the real filters, roles, and AD resources, each with an enforcement point
      backed by a repository-relative path.
    - **MVC scaffolding is ignored, not documented.** Stock `HomeController` or
      help-page scaffolding should be noted once in the overview and otherwise
      left alone.
    - **Client packages stay at `docs/clients/<slug>/`,** never nested under the
      API, and `endpoint-map.md` must only link endpoint pages the same run
      actually produced.
    - **ASP.NET Core Web API.** There is also a `hosting-pipeline.md` listing
      the middleware in registration order. Routes must have `[controller]`,
      `[action]`, and `{version:apiVersion}` substituted — a literal token in a
      route is a defect — and minimal-API endpoints must be catalogued. Package
      behavior (for example from `CoreAPIStartup`) is marked `†`. Configuration
      is `appsettings*.json`, options, and environment variables rather than
      `Web.config`.
    - **ASP.NET Core clients** are only documented when packaged. When the API
      is in the same run, `endpoint-map.md` needs a `## Route drift` section
      comparing client routes with the API's actual routes.

    The API and client examples below show the .NET Framework 4 shape.

    API examples: [index](examples/api-index.md) ·
    [overview](examples/api-overview.md) ·
    [architecture](examples/api-architecture.md) ·
    [endpoints](examples/api-endpoints.md) ·
    [endpoint document](examples/api-endpoint-document.md) ·
    [authentication and authorization](examples/api-authentication-authorization.md) ·
    [business rules](examples/api-business-rules.md) ·
    [operational runbook](examples/api-operational-runbook.md)

    Client package examples: [index](examples/client-index.md) ·
    [package](examples/client-package.md) ·
    [client reference](examples/client-reference.md) ·
    [endpoint map](examples/client-endpoint-map.md) ·
    [consumption](examples/client-consumption.md)

    ASP.NET Core Web API examples: [hosting pipeline](examples/ncore-api-hosting-pipeline.md) ·
    [endpoints](examples/ncore-api-endpoints.md) ·
    [configuration](examples/ncore-api-configuration.md)

    ASP.NET Core client examples: [package](examples/ncore-client-package.md) ·
    [consumption](examples/ncore-client-consumption.md) ·
    [endpoint map with route drift](examples/ncore-client-endpoint-map.md)

=== "SOAP Service"

    SOAP services document each hosted service under
    `docs/services/<service-slug>/`, cataloged by `docs/services/index.md`, and
    any proxy package that wraps one under `docs/clients/<client-slug>/`. WCF
    and ASMX produce the identical file set.

    - **One section per host.** Every deployable SOAP host has its own
      `docs/services/<slug>/` section and a working link from
      `docs/services/index.md`. A missing host is a blocker; so is documenting
      a host that has no deployment evidence.
    - **The set is complete.** `index.md`, `overview.md`, `architecture.md`,
      `activation-hosting.md`, `operations.md`, `faults.md`,
      `authentication-authorization.md`, `dependencies.md`, `configuration.md`,
      `operations-processes.md`, `business-rules.md`,
      `operational-runbook.md`, and `tech-debt.md` all exist, and every service
      contract has a page under `contracts/`.
    - **No REST or front-end vocabulary.** An HTTP verb, route template, status
      code, `postback`, or `.aspx` anywhere under `docs/services/` is a
      blocker. An ASMX project looks like a WebForms project at the file level,
      so do not expect screens.
    - **The stack is evidenced.** The activation mechanism (`.svc`, route-table
      `ServiceRoute`, `.asmx` marker) must be the one analysis found, not one
      assumed because the stack is WCF. The pipeline `AppType` never decides.
    - **Detection results, not defaults.** "None found in code" is a valid
      finding for authorization, fault handling, and duplication across hosts;
      flagging it as a gap is a review error. A credential literal anywhere in
      the documentation is a blocker. An ASMX `WebService` namespace still at
      `http://tempuri.org/` is tech debt, not the service's identity.
    - **Proxies.** `consumption.md` separates the operations the proxy exposes
      from the ones the repository actually calls, with call sites. Generated
      `Reference.cs` code is evidence, not subject matter; the generation
      mechanism (svcutil Service Reference or wsdl.exe Web Reference) must be
      the one found; timeout and retry behavior is stated, including "none
      configured". Proxy documentation never nests under `docs/services/`.

    WCF examples: [index](examples/soap-service-index.md) ·
    [contract](examples/soap-contract-document.md) ·
    [faults](examples/soap-faults.md)

    ASMX example: [activation and hosting](examples/soap-activation-hosting.md)

    Proxy examples: [operation map](examples/soap-client-operation-map.md) ·
    [consumption](examples/soap-client-consumption.md)

=== "Security / SQL pages"

    These appear across types whenever the analysis detects them.

    - `security-authorization.md` must name real AD resources, each with a
      non-empty Purpose and Enforcement point backed by a repository-relative
      source path.
    - Application-level security documents must consolidate every AD resource
      that appears in any library's security document.
    - Cross-cutting rules use `SEC-APP-NNN` IDs.
    - SQL Classification tables must include a `Schema` row, and schema names
      must be plausible for the target database — a hallucinated schema is the
      failure mode to watch for.

    Examples: [security and authorization](examples/shared-security-authorization.md) ·
    [SQL catalog](examples/shared-sql-catalog.md) ·
    [SQL document](examples/shared-sql-document.md) ·
    [procedure catalog](examples/shared-procedures-index.md) ·
    [stored procedure](examples/shared-procedure-document.md) ·
    [docgen manifest](examples/shared-docgen-manifest.md)

## Recording the outcome

- **Blockers** — send back for a fix before merging. Report them with
  `/docgen-bugfix`; see [Bugfix / quick fix](bugfix-quick-fix.md).
- **Majors** — fix in the same pull request unless there is a reason to defer.
- **Minors** — acceptable to merge and clean up in the next run. A purely
  cosmetic one is a good fit for `/docgen-quickfix`.

Review reports produced by the agent live in
`.saifdocgen/state/runs/<run-id>/`. They are process artifacts and must never
be committed under `docs/`.