diff --git a/.github/custom-layer-autoclose.md b/.github/custom-layer-autoclose.md index f0b059e..800d5fd 100644 --- a/.github/custom-layer-autoclose.md +++ b/.github/custom-layer-autoclose.md @@ -1,23 +1,19 @@ -Hey @{{AUTHOR}} ๐Ÿ‘‹ +Thank you for contributing, @{{AUTHOR}}. -First off โ€” thank you for jumping in and experimenting! It's awesome to see people pushing on the framework. ๐ŸŽ‰ +This PR was closed automatically because it changes custom-layer content. +`custom/` is reserved for organization-specific knowledge and skills in +**your own fork**, not the shared upstream repository. -That said, let me gently redirect you, because I think there's a small but important misunderstanding about how the `custom` layer is meant to work: +Keep company-only rules in your fork and point your host at that copy. The +[customization guide](https://github.com/microsoft/BCQuality/blob/main/docs/customizing-bcquality.md) +shows the complete flow. -The `custom` layer in *this* repo isn't a destination for PRs โ€” it's the designated sandbox inside **your own fork**. Think of it as the "your timeline" branch of the multiverse ๐ŸŒŒ: this repo is canon, your fork is where you get to remix the lore without needing anyone's approval. That's the whole point of the layer existing โ€” so you *don't* have to upstream your team-specific or experimental work. - -The intended workflow is: - -1. ๐Ÿด **Fork** BCQuality to your own GitHub account -2. Clone *your fork* locally -3. Drop your custom agents and knowledge into the `custom` layer **there** -4. Commit and push to your fork โ€” no PR back to upstream needed for custom stuff - -That way you get full control, your changes survive upstream updates cleanly, and you can pull in new core releases from this repo whenever you want. โœจ - -**Now โ€” here's the fun part:** if while building out your fork you discover knowledge, patterns, or agents that you think would genuinely benefit *everyone* using BCQuality (not just your team), that's exactly what the `/community` layer is for! ๐ŸŒŸ PRs to `/community` here in the upstream repo are absolutely welcome and encouraged โ€” it's how the collective hive mind ๐Ÿง  levels up. So please: tinker in your fork, and when you strike gold that's worth sharing, send it our way via `/community`. - -Going to close this PR for now (since it's targeting `custom` rather than `/community`), but please don't read it as a "no" โ€” it's a "yes, but let's route it correctly." ๐Ÿ™ Happy to help if you hit any snags spinning up your fork, and genuinely looking forward to seeing what you contribute to `/community` down the line. +If the guidance is useful to everyone, submit it to the layer that owns the +domain: Microsoft-owned domains belong under `microsoft/knowledge/`, even +when contributed by a partner; Community-owned domains belong under +`community/knowledge/`. See +[Contributing](https://github.com/microsoft/BCQuality/blob/main/docs/contributing.md). +BCQuality contains knowledge and skills, not agents.
Files in this PR that triggered the auto-close @@ -25,7 +21,5 @@ Going to close this PR for now (since it's targeting `custom` rather than `/comm {{FILES}}
-May your merges be conflict-free. ๐Ÿš€ - ---- -๐Ÿค– This PR was closed automatically by the `Guard custom layer` workflow because it adds or changes content under `/custom/`. If you were only updating the template (`custom/README.md` or a `.gitkeep`), a maintainer can re-open it. If you think this was closed in error, just comment here. +Template changes to `custom/README.md` and `.gitkeep` files are allowed. If +you believe this closure was a mistake, comment here for maintainer review. diff --git a/.github/new-top-level-flag.md b/.github/new-top-level-flag.md index 3d297ea..9656922 100644 --- a/.github/new-top-level-flag.md +++ b/.github/new-top-level-flag.md @@ -3,7 +3,7 @@ {{ENTRIES}} -This isn't a block โ€” just a flag. ๐Ÿšฉ New top-level folders and files are *usually* unintended (a stray export, a tool's scratch dir, or content that meant to land inside an existing layer like `/community/knowledge/`). BCQuality keeps a deliberately small root: `.github/`, `community/`, `custom/`, `microsoft/`, `skills/`, and `tools/`, plus a handful of root docs. +This isn't a block โ€” just a flag. ๐Ÿšฉ New top-level folders and files are *usually* unintended (a stray export, a tool's scratch dir, or content that meant to land inside an existing layer). BCQuality keeps a deliberately small root: plugin metadata, `community/`, `custom/`, `microsoft/`, `skills/`, `tools/`, `docs/`, `evaluation/`, `.github/`, and a handful of root docs. Partner guides belong under `docs/`; shared knowledge belongs beside the skill that owns its domain. **If this was intentional** and the new entry genuinely belongs at the repo root, a maintainer can review and merge as normal โ€” no action needed beyond a quick sanity check. **If it wasn't**, please move the content into the right existing layer (or drop it) and push an update. ๐Ÿ™ diff --git a/.github/scripts/validate_frontmatter.py b/.github/scripts/validate_frontmatter.py index b0076cc..f422d53 100644 --- a/.github/scripts/validate_frontmatter.py +++ b/.github/scripts/validate_frontmatter.py @@ -45,7 +45,7 @@ ENTRY_SKILL_REQUIRED_KEYS = {"kind", "id", "version", "title"} HOST_SKILL_REQUIRED_KEYS = {"name", "description"} STANDARD_INPUTS = { - "pr-diff", "object-list", "file-path", "repository", "telemetry-query", + "pr-diff", "object-list", "file-path", "folder-path", "repository", "telemetry-query", } ALLOWED_OUTPUTS = {"findings-report"} VALID_SAMPLE_KINDS = {"good", "bad"} diff --git a/.github/workflows/flag-new-top-level.yml b/.github/workflows/flag-new-top-level.yml index 31ab105..865a120 100644 --- a/.github/workflows/flag-new-top-level.yml +++ b/.github/workflows/flag-new-top-level.yml @@ -40,11 +40,12 @@ jobs: // Known, intended repository root. Anything else added at the root // is flagged for a human to eyeball. const ALLOWED_DIRS = new Set([ - '.claude-plugin', '.github', 'community', 'custom', 'microsoft', 'skills', 'tools', + '.claude-plugin', '.github', 'community', 'custom', 'docs', 'evaluation', + 'microsoft', 'skills', 'tools', ]); const ALLOWED_FILES = new Set([ '.gitignore', 'CODEOWNERS', 'LICENSE', 'README.md', - 'SECURITY.md', 'agent-consumption.md', + 'SECURITY.md', 'plugin.json', ]); const MARKER = ''; diff --git a/README.md b/README.md index 823db24..9eeee91 100644 --- a/README.md +++ b/README.md @@ -1,236 +1,125 @@ # BCQuality -Quality skills and knowledge for Business Central development. +Quality skills and knowledge that help AI tools make better Business Central +development decisions: catch BC-specific defects, avoid misleading advice, +and explain findings with references you can read. -BCQuality is a curated knowledge base and skills library for Business Central. It provides structured, machine-readable guidance that development agents and tools can consume โ€” establishing a consistent quality bar across tooling and teams. +BCQuality contains **knowledge and reusable skills**, not agents or a Business +Central extension. Your host supplies the agent. You can install the content +as a plugin, use it from another integration, or browse the knowledge directly. -## What belongs here +## Quick start -BCQuality is a remedial knowledge base. A file exists because a capable LLM **would get something wrong, or miss something, without it** โ€” not because the topic is important. The admission test for a knowledge file is one question: - -> If this file did not exist, would a modern LLM reviewing or generating BC code make a mistake this file would have prevented? - -If the answer is no โ€” the advice is generic software-engineering guidance, or the LLM already knows the BC mechanic in question โ€” the file does not belong here, regardless of how sound the content is. A file earns its place by encoding something BC-specific that LLMs demonstrably get wrong: a CodeCop rule number, a platform API whose semantics the training data gets backwards, a non-obvious ordering rule, a BC property whose default is a footgun. - -Good fit: "`SetLoadFields` must be called before filters, not after" (non-obvious ordering rule). "`FindSet(true)` takes a LockTable and the two-parameter signature is obsolete" (subtle platform behaviour + outdated training data). "CodeCop AA0233 flags `FindFirst โ€ฆ Next` loops" (rule-specific). - -Poor fit: "Use HTTPS instead of HTTP." "Don't hardcode secrets." "Keep transactions short." These are true but any capable LLM already applies them without prompting. - -The practical consequence: when a code-review agent flags something it shouldn't have, or misses something it should have caught, the remedy is a new knowledge file. When it already behaves correctly on a topic, no file is needed. - -A file that *prevents* a false positive โ€” documenting why a pattern is legitimate so the agent stops flagging it โ€” is as valid as one that catches a defect: negative clarifications are first-class knowledge files. What never belongs is a BC fact hard-coded into a skill. Skills are finders and appliers; knowledge files are what the agent knows. See [`skills/do.md`](skills/do.md) and [`skills/write.md`](skills/write.md). - -## What's in this repo - -BCQuality contains **knowledge** and **skills**. It does not contain agents. -Agents that consume BCQuality are supplied by the host or orchestrator. - -### Knowledge files - -Atomic markdown files with YAML frontmatter. Each file covers one concern โ€” one thing an agent would cite when reviewing or generating code. Knowledge files live in three layers: - -- **`/microsoft/`** โ€” Microsoft-endorsed layer. - - `/microsoft/knowledge/` โ€” Platform guardrails, official guidance. - - `/microsoft/skills/` โ€” Microsoft-endorsed action skills. -- **`/community/`** โ€” BC community layer. - - `/community/knowledge/` โ€” Community patterns and shared guidance. - - `/community/skills/` โ€” Community-contributed action skills. - -- **`/custom/`** โ€” Partner- and customer-specific overrides. Empty by default; populated in forks. - - `/custom/knowledge/` โ€” Organization-specific knowledge files. - - `/custom/skills/` โ€” Organization-specific action skills. - -All three layers are enabled by default when an agent consumes BCQuality. In the shared upstream layers, an action skill and the canonical knowledge it owns should live together: knowledge used by a Microsoft-endorsed skill belongs in `/microsoft/`, while `/community/` holds community-owned skills and their related knowledge. A split is acceptable briefly while a skill or corpus is being promoted, but it should not be the steady state. The `/custom/` layer remains the intentional exception because it overrides shared content in consumer forks. - -Layer authority follows review and ownership, not the contributor's affiliation. Community contributions to a Microsoft-owned knowledge domain can therefore be accepted directly into `/microsoft/`; content can also be promoted from Community to Microsoft-endorsed once its owning skill is promoted. - -### Skills - -Skills define how agents consume knowledge. They come in three flavors: - -- **The entry-point skill** ([`skills/entry.md`](skills/entry.md)) โ€” the first skill an agent invokes at runtime. Given a task context (goal, available inputs, technologies, BC version, etc.), it returns a **dispatch record** naming the action skill or skills to invoke next. Routing logic lives here, not in the orchestrator. - -- **Meta-skill contracts** (`/skills/`) โ€” three stable references that define the rest of the repo: - 1. **Schema + Use** (READ, [`skills/read.md`](skills/read.md)) โ€” how to read a knowledge file: interpret frontmatter, parse sections, understand layer precedence. Any agent or skill that reads knowledge files depends on it. - 2. **Action Skill** (DO, [`skills/do.md`](skills/do.md)) โ€” the template every action skill follows. Defines the four-step pattern (Source โ†’ Relevance โ†’ Worklist โ†’ Action) and the structured output format that orchestrators expect. - 3. **New Knowledge** (WRITE, [`skills/write.md`](skills/write.md)) โ€” how to author a valid knowledge file. References Schema + Use for the format specification and adds authoring rules (atomicity, section guidance). - - READ and DO are read on demand โ€” typically when the first dispatched action skill runs. They are not prerequisites for invoking Entry. WRITE is only used when scaffolding new content. - -- **Action skills** โ€” concrete skills that follow the Action Skill template to do real work (review code, audit telemetry, etc.). Action skills live inside the layers that own them (`/microsoft/skills/`, `/community/skills/`, `/custom/skills/`). An action skill is either a **leaf** that evaluates knowledge files directly, or a **super-skill** that composes other action skills (declared via `sub-skills` in frontmatter). The canonical reference is [`microsoft/skills/review/al-code-review.md`](microsoft/skills/review/al-code-review.md) (super-skill), which composes the AL review leaf skills under [`microsoft/skills/review/`](microsoft/skills/review/) โ€” one per knowledge domain. - -### Agent bootstrapping - -A host or orchestrator points the agent at BCQuality and provides a task -context. The agent's first call is `/skills/entry.md`, which returns a dispatch -record naming the action skill(s) to invoke. The agent then invokes the -dispatched skills, reading READ and DO on demand. No prior knowledge of -BCQuality's structure is required beyond the convention *"invoke -`/skills/entry.md` first."* +The walkthrough below uses **GitHub Copilot CLI in a terminal**, not the +Copilot Chat panel in VS Code. First +[install Copilot CLI and sign in](https://docs.github.com/en/copilot/get-started/cli-quickstart). +Your account and organization policy must allow its use. You do not need to +clone BCQuality, build a runner, or deploy an app to Business Central for this +source-review example. ### Standalone plugin installation -BCQuality can also be installed directly as a plugin so supported hosts can -discover and invoke its host-native skills. The plugin currently registers -[`al-code-review`](skills/al-code-review/SKILL.md), which adapts the caller's -request to the same Entry protocol used by orchestrators. +Run these commands in your terminal: -For GitHub Copilot CLI: - -```shell +```powershell copilot plugin install microsoft/BCQuality +copilot plugin list ``` -#### Example: Review a complete app folder +The list should include `bcquality`. The plugin currently exposes the +[`al-code-review`](skills/al-code-review/SKILL.md) skill. Installation and skill +discovery are the general pattern; reviewing an app is one example of using it. -This example demonstrates the walk-up pattern with the currently exposed -review skill. Future host-native skills follow the same discovery and -invocation pattern; they do not each require a dedicated README walkthrough. +### Example: Review a complete app folder -1. Open the Business Central app folder in GitHub Copilot and start a fresh - session after installing the plugin. -2. Ask: +Start a **new** CLI session in your own app folder, replacing the example path: - > Use the installed `al-code-review` skill to review the complete Business - > Central app in this folder. Execute every dispatched review domain and - > return the complete BCQuality findings report. - -That is the complete walk-up flow. The folder does not need to be a Git -repository; BCQuality reviews `app.json` and the AL source below it. To pick up -a newer BCQuality release later, run: - -```shell -copilot plugin update bcquality +```powershell +cd "C:\Repos\MyBusinessCentralApp" +copilot ``` -The adapter is intentionally not a second review implementation: +Approve access only to a project you trust, then ask: -```text -standalone host skill: skills/al-code-review/SKILL.md - -> routing contract: skills/entry.md - -> review coordinator: microsoft/skills/review/al-code-review.md - -> domain review leaves -``` +> Use the installed al-code-review skill to review the complete Business Central +> app in this folder without changing my source files. Return the complete +> BCQuality findings report. -Only the first file follows the host's `SKILL.md` packaging format. The -remaining files are BCQuality's internal protocol and layered action skills. -Entry remains the single owner of routing and index preparation; -`al-code-review.md` remains the single owner of broad-review composition. This -separation keeps standalone installation available without duplicating those -policies in the plugin adapter. +The folder should contain `app.json` and your AL source; it does **not** need +to be a Git repository. On macOS or Linux, use your app's local path instead. -Note that a plugin install ships the entire tree, so `BCQUALITY_ENABLED_LAYERS` -narrows discovery without removing any files. Layer selection is a filter here, -not a deny mechanism โ€” see [the adapter](skills/al-code-review/SKILL.md) for the -difference from the pruned-clone model. +Expect a report for each selected review, with findings, source locations, +severity, confidence, and references to the relevant guidance. Some hosts show +the structured JSON directly. `completed` with no findings means nothing was +flagged in that review's scope; `partial` or `failed` is **not** a clean result. +See [reading your results](docs/using-bcquality.md#reading-your-results). -The host adapter and internal action skill intentionally share the -`al-code-review` name: they expose the same operation in two different skill -formats. Their paths make the boundary explicit. The adapter lives under -`skills/al-code-review/SKILL.md`; the internal Microsoft-layer coordinator -lives at `microsoft/skills/review/al-code-review.md`. +[PowerShell 7](https://learn.microsoft.com/en-us/powershell/scripting/install/installing-powershell) +(`pwsh`) is recommended for fast knowledge discovery. If it is unavailable, +the review can still discover knowledge by reading the folders. -Partners that want model selection, parallel leaf execution, retries, or usage -telemetry can add a thin runner outside BCQuality. See -[Build a lightweight standalone review runner](docs/standalone-runner.md) for the -integration contract and a minimal implementation checklist. Architecture and -partner guides are collected in the [documentation index](docs/README.md). +## Documentation -## Knowledge file format +| I want to... | Start here | +| --- | --- | +| Review a file, changes, a branch, or a particular concern | [Using BCQuality](docs/using-bcquality.md) | +| Resolve setup problems, incomplete reviews, or incorrect findings | [Troubleshooting and support](docs/troubleshooting.md) | +| Browse the available guidance | [Knowledge by domain](docs/using-bcquality.md#knowledge-by-domain) | +| Configure the plugin or use my organization's rules | [Customizing BCQuality](docs/customizing-bcquality.md) | +| Contribute knowledge or improve a rule | [Contributing](docs/contributing.md) | +| Connect a host, agent, or CI integration | [How agents consume BCQuality](docs/agent-consumption.md) | -Every knowledge file is a markdown file with mandatory YAML frontmatter. Files target under 100 lines (ideal under 50). If two ideas would share a file, split them. - -### Frontmatter schema (v1) - -```yaml ---- -bc-version: [all] # or [26..28], or [26..] for "26 and later" -domain: performance # security | performance | ux | telemetry | ... -keywords: [query, filtering, partial] # free-text tags for retrieval -technologies: [al] # al | javascript | powershell | ... -countries: [w1] # ISO codes, or [w1] -application-area: [all] # finance | manufacturing | jobs | [all] ---- -``` - -All six fields are required. The schema is locked โ€” changes require a PR approved by both maintainers. - -### Sections - -Every knowledge file must contain a `## Description` section. The following sections are optional but recommended: - -- **`## Best Practice`** โ€” the recommended approach -- **`## Anti Pattern`** โ€” what to avoid and why - -Code examples belong in separate files, not in the knowledge file itself. Knowledge files must not contain fenced code blocks. +[All documentation and technical references](docs/README.md). ## Scope -The current curated corpus is focused on **technical AL code review**: Agents, AppSource and compatibility, data modeling, error handling, events, interfaces, performance, privacy, Query objects, security, style, telemetry, testing, UI, upgrade, and web services. These are the domains backed by knowledge files and registered review leaves today. +Today's curated content focuses on **technical AL code review**. It augments +the agent's judgment; it is not an exhaustive BC manual or a substitute for +compilation, analyzers, tests, or human review. See +[coverage and limits](docs/using-bcquality.md#coverage-and-limits) for the +available domains and the difference between a folder review and a comparison. -Business Central functional domains (Finance, Supply Chain Management, Manufacturing, Jobs, Warehousing, Service), PowerShell, pipelines, and Power Platform remain valid future repository scope, but they are **not current coverage claims** until corresponding knowledge and action skills exist. Consumers should derive supported review scope from the live knowledge index and dispatched skills, not from roadmap breadth. +Functional areas such as Finance, Supply Chain Management, Manufacturing, Jobs, +Warehousing, and Service, and technologies such as PowerShell, pipelines, and +Power Platform, remain valid future scope, **not current coverage claims**. -## How agents consume BCQuality +## What's in this repo -Action skills follow a four-step pattern: +Knowledge articles cover one concern each. Skills tell an agent how to find +and apply the relevant knowledge. Both live in three layers: -1. **Source** โ€” which knowledge folders and tags to search -2. **Relevance** โ€” filter by frontmatter (version, technology, country, area) -3. **Worklist** โ€” narrow from N candidates to the M that apply to the current task -4. **Action** โ€” apply the relevant knowledge and produce structured output +| Layer | Purpose | +| --- | --- | +| [Microsoft](microsoft/) | Microsoft-endorsed skills and their knowledge. | +| [Community](community/) | Community-owned skills and their knowledge. | +| [Custom](custom/) | Organization-specific additions and overrides in your own fork. | -Every action skill produces output in a common format that orchestrators can consume without skill-specific parsing. The format is JSON and includes an `outcome` (so a clean run, a not-applicable skill, and a partial failure are all distinguishable), `findings` (what the skill observed), structured `references` back to the knowledge files that informed each finding, per-finding `confidence`, and a `suppressed` list recording any knowledge files overridden by layer precedence. This contract is defined in the Action Skill meta-skill so that orchestrators and action skills remain independently evolvable. - -BCQuality is an **additive** knowledge layer: it augments the agent's review judgement, it does not replace it. Super-skills (such as `al-code-review`) run a self-review pass alongside their sub-skills and surface concerns the agent identified on its own, marked with `from-sub-skill: "agent"` and an empty `references: []` so consumers can render them distinctly from knowledge-backed findings. See [How agents consume BCQuality](docs/agent-consumption.md) and [`skills/do.md`](skills/do.md) for the full contract. - -The meta-skills in `/skills/` define this pattern. Every concrete action skill follows it. - -For the end-to-end flow โ€” from orchestrator trigger through to how output reaches developers โ€” see [How agents consume BCQuality](docs/agent-consumption.md). - -## Repository structure - -``` -โ”œโ”€โ”€ /skills/ # Global: entry-point skill + meta-skill contracts (READ, DO, WRITE) -โ”œโ”€โ”€ /docs/ # Architecture and partner integration guides -โ”œโ”€โ”€ /evaluation/ # Neutral good/bad review fixtures and scoring contract -โ”œโ”€โ”€ /.github/ # Actions and workflows -โ”œโ”€โ”€ /microsoft/ # Microsoft-endorsed layer -โ”‚ โ”œโ”€โ”€ /knowledge/ # Knowledge files by domain -โ”‚ โ”‚ โ””โ”€โ”€ // # Each article: .md + optional .good.al / .bad.al -โ”‚ โ””โ”€โ”€ /skills/ # Microsoft-endorsed action skills -โ”œโ”€โ”€ /community/ # BC community layer -โ”‚ โ”œโ”€โ”€ /knowledge/ # Knowledge files by domain -โ”‚ โ”‚ โ””โ”€โ”€ // # Article + sibling samples, same convention -โ”‚ โ””โ”€โ”€ /skills/ # Community action skills -โ”œโ”€โ”€ /custom/ # Partner/customer-specific overrides (empty; populated in forks) -โ”‚ โ”œโ”€โ”€ /knowledge/ -โ”‚ โ””โ”€โ”€ /skills/ -``` +All three are enabled by default; Custom is empty upstream. You do not need +to configure layers to get started. ## Versioning -BCQuality content is released on demand โ€” roughly monthly, not on every commit. A -release is a `major.minor` value derived from git tags, cut manually via the -`Release version` workflow: pick whether to bump the minor or the major, and it -computes the next version and tags the current `main` as `v{major}.{minor}`. +Update the installed plugin from your terminal, then start a new session: -- Bump the **minor** for the usual periodic content update; bump the **major** - only for a breaking change. -- The minor is a **monotonic counter** โ€” it only ever increments and never - resets, even across a major bump โ€” so it uniquely identifies a release. +```powershell +copilot plugin update bcquality +``` + +Plugin versions and content-release tags are different. For reproducible runs +and organization forks, see [updates and versions](docs/customizing-bcquality.md#updates-and-versions). + +## What belongs here + +Knowledge belongs here when it prevents a BC-specific mistake an otherwise +capable agent would make, including false-positive findings. BC facts belong +in knowledge articles, not skill instructions. See the +[admission test and examples](docs/contributing.md#what-belongs-here). ## Contributing -Contributions are welcome. Before submitting a PR: - -1. Read the knowledge file format above โ€” frontmatter and sections are validated by CI. -2. Keep files atomic: one concern per file, under 100 lines. -3. Target your contribution to the layer that owns the action skill: use `/microsoft/knowledge/` for Microsoft-owned domains and `/community/knowledge/` for knowledge that accompanies a community-owned skill. -4. Adding a BC fact โ€” or stopping the agent from flagging a false positive โ€” is a knowledge file, not a skill edit. If a PR changes *what* a review skill flags, the change almost certainly belongs in a knowledge file. See [`skills/write.md`](skills/write.md). - -CI runs validation on every PR. If your knowledge file has schema violations, missing sections, code blocks, or exceeds 100 lines, the check will fail with a clear error message. - -Companion samples must be referenced by filename from their article, and every referenced sample must exist. The review evaluation corpus under [`evaluation/`](evaluation/) adds one positive and one clean control for every registered AL review leaf; see [`evaluation/README.md`](evaluation/README.md) for credential-free validation and optional fast-model scoring. +Partners are welcome to contribute to the layer that owns the domain, +regardless of affiliation. Start with the [contribution guide](docs/contributing.md). +To report a problem without authoring a rule, see [support](docs/troubleshooting.md#reporting-a-problem). ## License diff --git a/community/knowledge/agents/agent-permissions-intersect-with-assigner.md b/community/knowledge/agents/agent-permissions-intersect-with-assigner.md index 2259f66..7c85a5b 100644 --- a/community/knowledge/agents/agent-permissions-intersect-with-assigner.md +++ b/community/knowledge/agents/agent-permissions-intersect-with-assigner.md @@ -17,13 +17,13 @@ An agent is a user, but it cannot configure users or other agents, and it cannot Document that intersection. Give the agent only the table and page rights its tasks need. Do not add user-setup or permission-assignment pages to the agent profile or permission sets; those operations will fail by design. -See sample: `agent-permissions-intersect-with-assigner.good.al`. +See sample: [`agent-permissions-intersect-with-assigner.good.al`](agent-permissions-intersect-with-assigner.good.al). ## Anti Pattern Permission sets or profiles that include User card, Permission Set Assignment, or agent-admin pages, or comments that the agent runs as SUPER regardless of who assigned it. Detection signal: default access controls or profile including user-administration objects. -See sample: `agent-permissions-intersect-with-assigner.bad.al`. +See sample: [`agent-permissions-intersect-with-assigner.bad.al`](agent-permissions-intersect-with-assigner.bad.al). ## See also diff --git a/community/knowledge/agents/agent-profile-narrows-visible-ui.md b/community/knowledge/agents/agent-profile-narrows-visible-ui.md index 30d286a..914a777 100644 --- a/community/knowledge/agents/agent-profile-narrows-visible-ui.md +++ b/community/knowledge/agents/agent-profile-narrows-visible-ui.md @@ -17,13 +17,13 @@ The agent only sees what its profile shows. Extra actions, views, and Role Cente Ship an agent-specific profile and page customizations: hide unrelated actions, keep descriptive tooltips, add Role Center links to the few pages the agent should open. Prefer fewer navigation hops. -See sample: `agent-profile-narrows-visible-ui.good.al`. +See sample: [`agent-profile-narrows-visible-ui.good.al`](agent-profile-narrows-visible-ui.good.al). ## Anti Pattern Assigning `BUSINESS MANAGER` or `ORDER PROCESSOR` as `GetDefaultProfile` so the agent can do anything. Detection signal: default profile equal to a full-user role with no agent page customizations. -See sample: `agent-profile-narrows-visible-ui.bad.al`. +See sample: [`agent-profile-narrows-visible-ui.bad.al`](agent-profile-narrows-visible-ui.bad.al). ## See also diff --git a/community/knowledge/agents/agent-setup-page-is-configuration-dialog.md b/community/knowledge/agents/agent-setup-page-is-configuration-dialog.md index d844d33..083a83b 100644 --- a/community/knowledge/agents/agent-setup-page-is-configuration-dialog.md +++ b/community/knowledge/agents/agent-setup-page-is-configuration-dialog.md @@ -17,10 +17,10 @@ Instance setup is not a Card or StandardDialog. The toolkit expects `PageType = Declare `PageType = ConfigurationDialog`, host `part(...; "Agent Setup Part")`, and put agent-specific fields in another group. Keep system OK/Cancel. Use a temporary source record and defer persistence until Update, as described in `agent-setup-source-table-is-temporary.md`. Following Microsoft's agent setup samples, set `Extensible = false`. -See sample: `agent-setup-page-is-configuration-dialog.good.al`. +See sample: [`agent-setup-page-is-configuration-dialog.good.al`](agent-setup-page-is-configuration-dialog.good.al). ## Anti Pattern A Card or StandardDialog setup page with no `Agent Setup Part`. Detection signal: setup page ID from `IAgentFactory` / `IAgentMetadata` whose page is not `ConfigurationDialog` or has no `Agent Setup Part`. -See sample: `agent-setup-page-is-configuration-dialog.bad.al`. +See sample: [`agent-setup-page-is-configuration-dialog.bad.al`](agent-setup-page-is-configuration-dialog.bad.al). diff --git a/community/knowledge/agents/agent-setup-source-table-is-temporary.md b/community/knowledge/agents/agent-setup-source-table-is-temporary.md index 8f31aa0..539bc0f 100644 --- a/community/knowledge/agents/agent-setup-source-table-is-temporary.md +++ b/community/knowledge/agents/agent-setup-source-table-is-temporary.md @@ -17,13 +17,13 @@ ConfigurationDialog setup is a draft: the user can Cancel without writing. That Mark the page `SourceTableTemporary = true`. Copy into the temp record on open. Persist the Agent Setup buffer and custom fields only from the close path when the action is not Cancel, using `Agent Setup.GetChangesMade` / `SaveChanges`. -See sample: `agent-setup-source-table-is-temporary.good.al`. +See sample: [`agent-setup-source-table-is-temporary.good.al`](agent-setup-source-table-is-temporary.good.al). ## Anti Pattern A non-temporary source table, or `Insert`/`Modify` on the persisted setup row from field OnValidate. Detection signal: agent `ConfigurationDialog` without `SourceTableTemporary = true`, or database writes before Update. -See sample: `agent-setup-source-table-is-temporary.bad.al`. +See sample: [`agent-setup-source-table-is-temporary.bad.al`](agent-setup-source-table-is-temporary.bad.al). ## See also diff --git a/community/knowledge/agents/agent-setup-table-keyed-by-user-security-id.md b/community/knowledge/agents/agent-setup-table-keyed-by-user-security-id.md index c59a9f1..c8a8671 100644 --- a/community/knowledge/agents/agent-setup-table-keyed-by-user-security-id.md +++ b/community/knowledge/agents/agent-setup-table-keyed-by-user-security-id.md @@ -17,10 +17,10 @@ Each agent instance is a user. Instance-specific setup is keyed by that user's ` Give the setup table a Guid field `User Security ID` as the clustered primary key. Other settings are attributes of that key. When the page opens, `Get` or insert by the Guid the Agent Setup part already holds. -See sample: `agent-setup-table-keyed-by-user-security-id.good.al`. +See sample: [`agent-setup-table-keyed-by-user-security-id.good.al`](agent-setup-table-keyed-by-user-security-id.good.al). ## Anti Pattern A setup table keyed by Code, Integer, or with no Guid user key, then mapping one row to every instance. Detection signal: source table of the agent setup page whose primary key is not `User Security ID`. -See sample: `agent-setup-table-keyed-by-user-security-id.bad.al`. +See sample: [`agent-setup-table-keyed-by-user-security-id.bad.al`](agent-setup-table-keyed-by-user-security-id.bad.al). diff --git a/community/knowledge/agents/analyze-message-error-stops-warning-forces-review.md b/community/knowledge/agents/analyze-message-error-stops-warning-forces-review.md index f8c6eb5..9c465f4 100644 --- a/community/knowledge/agents/analyze-message-error-stops-warning-forces-review.md +++ b/community/knowledge/agents/analyze-message-error-stops-warning-forces-review.md @@ -17,10 +17,10 @@ application-area: [all] Validate inbound payloads in analysis: Error when the task must not run; Warning when a human must confirm. For outbound messages, adjust text in this method rather than in a later subscriber. Do not rely on skip-review to bypass warnings. -See sample: `analyze-message-error-stops-warning-forces-review.good.al`. +See sample: [`analyze-message-error-stops-warning-forces-review.good.al`](analyze-message-error-stops-warning-forces-review.good.al). ## Anti Pattern Ignoring analysis entirely, or emitting Warning while documenting that `SetRequiresReview(false)` means unattended run. Detection signal: empty `AnalyzeAgentTaskMessage` plus skip-review on external input. -See sample: `analyze-message-error-stops-warning-forces-review.bad.al`. +See sample: [`analyze-message-error-stops-warning-forces-review.bad.al`](analyze-message-error-stops-warning-forces-review.bad.al). diff --git a/community/knowledge/agents/bind-agent-subscribers-only-in-agent-session.md b/community/knowledge/agents/bind-agent-subscribers-only-in-agent-session.md index 3d18818..be227f4 100644 --- a/community/knowledge/agents/bind-agent-subscribers-only-in-agent-session.md +++ b/community/knowledge/agents/bind-agent-subscribers-only-in-agent-session.md @@ -17,10 +17,10 @@ Page-filter tweaks, extra validation, and prompt dialogs for the agent should no On `OnAfterInitialization`, exit unless `Agent Session.IsAgentSession`. Then `BindSubscription` a single-instance codeunit that holds the current task id. Keep those subscribers internal. -See sample: `bind-agent-subscribers-only-in-agent-session.good.al`. +See sample: [`bind-agent-subscribers-only-in-agent-session.good.al`](bind-agent-subscribers-only-in-agent-session.good.al). ## Anti Pattern Event subscribers on `Sales Header` OnAfterInsert that always `Message` the agent, with no `IsAgentSession` guard. Detection signal: agent-only behaviour in a static subscriber that is not bind-gated. -See sample: `bind-agent-subscribers-only-in-agent-session.bad.al`. +See sample: [`bind-agent-subscribers-only-in-agent-session.bad.al`](bind-agent-subscribers-only-in-agent-session.bad.al). diff --git a/community/knowledge/agents/cross-app-agent-calls-need-your-public-api.md b/community/knowledge/agents/cross-app-agent-calls-need-your-public-api.md index 41c1c2f..7d44699 100644 --- a/community/knowledge/agents/cross-app-agent-calls-need-your-public-api.md +++ b/community/knowledge/agents/cross-app-agent-calls-need-your-public-api.md @@ -17,10 +17,10 @@ For isolation, `Agent`, `Agent Task Builder`, and related toolkit codeunits erro Expose a public codeunit in the agent app (`Access = Public`) whose procedures take `User Security ID` and forward to `Agent` / `Agent Task Builder`. Document that surface as the integration contract. Keep toolkit calls inside that app. -See sample: `cross-app-agent-calls-need-your-public-api.good.al`. +See sample: [`cross-app-agent-calls-need-your-public-api.good.al`](cross-app-agent-calls-need-your-public-api.good.al). ## Anti Pattern From app B, calling `Agent.SetDisplayName` or `Agent.Create` with app A's metadata provider. Detection signal: toolkit agent APIs used with an `Agent Metadata Provider` value not declared in the same app. -See sample: `cross-app-agent-calls-need-your-public-api.bad.al`. +See sample: [`cross-app-agent-calls-need-your-public-api.bad.al`](cross-app-agent-calls-need-your-public-api.bad.al). diff --git a/community/knowledge/agents/do-not-create-agents-in-install-upgrade-or-background.md b/community/knowledge/agents/do-not-create-agents-in-install-upgrade-or-background.md index 41d0573..2018de4 100644 --- a/community/knowledge/agents/do-not-create-agents-in-install-upgrade-or-background.md +++ b/community/knowledge/agents/do-not-create-agents-in-install-upgrade-or-background.md @@ -17,10 +17,10 @@ application-area: [all] Create instances from a setup page, a wizard, or another UI-driven path after the user is in a client session. Apply instructions and `Activate` there. For existing companies after an upgrade, document that an admin must open setup; do not create from the upgrade codeunit. -See sample: `do-not-create-agents-in-install-upgrade-or-background.good.al`. +See sample: [`do-not-create-agents-in-install-upgrade-or-background.good.al`](do-not-create-agents-in-install-upgrade-or-background.good.al). ## Anti Pattern `Agent.Create` inside `OnInstallAppPerCompany`, `OnUpgradePerCompany`, or a job-queue codeunit. The call fails at runtime even if it compiles. Detection signal: `Agent.Create` in `Subtype = Install`, `Subtype = Upgrade`, or a non-UI session. -See sample: `do-not-create-agents-in-install-upgrade-or-background.bad.al`. +See sample: [`do-not-create-agents-in-install-upgrade-or-background.bad.al`](do-not-create-agents-in-install-upgrade-or-background.bad.al). diff --git a/community/knowledge/agents/get-default-access-controls-least-privilege.md b/community/knowledge/agents/get-default-access-controls-least-privilege.md index 87349af..c82f71a 100644 --- a/community/knowledge/agents/get-default-access-controls-least-privilege.md +++ b/community/knowledge/agents/get-default-access-controls-least-privilege.md @@ -17,13 +17,13 @@ application-area: [all] Insert only the permission sets the agent needs. For an AL `permissionset` object, use `Scope::System` and the ID of the app that defines it. Recreate permission sets that exist only as user-defined configuration in Business Central as AL objects first. Prefer a dedicated permission set over a full-user role. -See sample: `get-default-access-controls-least-privilege.good.al`. +See sample: [`get-default-access-controls-least-privilege.good.al`](get-default-access-controls-least-privilege.good.al). ## Anti Pattern Empty `GetDefaultAccessControls`, or inserting `SUPER` / `D365 BUS FULL ACCESS` because it made the demo work. Detection signal: Role ID on the default buffer that is a full-user role, or a set that is not in the app. -See sample: `get-default-access-controls-least-privilege.bad.al`. +See sample: [`get-default-access-controls-least-privilege.bad.al`](get-default-access-controls-least-privilege.bad.al). ## See also diff --git a/community/knowledge/agents/get-default-profile-lives-in-the-app.md b/community/knowledge/agents/get-default-profile-lives-in-the-app.md index 25103b5..89c19fb 100644 --- a/community/knowledge/agents/get-default-profile-lives-in-the-app.md +++ b/community/knowledge/agents/get-default-profile-lives-in-the-app.md @@ -17,13 +17,13 @@ application-area: [all] Ship a `profile` object (and page customizations) in the app. In `GetDefaultProfile`, call `Agent.PopulateDefaultProfile` with that profile ID and `NavApp.GetCurrentModuleInfo`. Include UI-exported customizations as AL. -See sample: `get-default-profile-lives-in-the-app.good.al`. +See sample: [`get-default-profile-lives-in-the-app.good.al`](get-default-profile-lives-in-the-app.good.al). ## Anti Pattern Setting `TempAllProfile."Profile ID"` to a client-only profile, or skipping `GetDefaultProfile`. Detection signal: factory default profile ID with no matching `profile` object in the app. -See sample: `get-default-profile-lives-in-the-app.bad.al`. +See sample: [`get-default-profile-lives-in-the-app.bad.al`](get-default-profile-lives-in-the-app.bad.al). ## See also diff --git a/community/knowledge/agents/instruction-structure-is-role-rules-steps.md b/community/knowledge/agents/instruction-structure-is-role-rules-steps.md index 1b56625..4a928fd 100644 --- a/community/knowledge/agents/instruction-structure-is-role-rules-steps.md +++ b/community/knowledge/agents/instruction-structure-is-role-rules-steps.md @@ -17,13 +17,13 @@ The runtime treats instructions as the agent's standing prompt. A one-line goal Store a document that states responsibilities, then non-negotiable guidelines (when to request a review, when not to post), then numbered steps for each task. Keep that text in the resource you pass to `SetInstructions`. -See sample: `instruction-structure-is-role-rules-steps.good.al`. +See sample: [`instruction-structure-is-role-rules-steps.good.al`](instruction-structure-is-role-rules-steps.good.al). ## Anti Pattern A single sentence such as Check customer credit for the sales order. Detection signal: instruction resource or `SetInstructions` payload with no responsibilities / guidelines / steps sections. -See sample: `instruction-structure-is-role-rules-steps.bad.al`. +See sample: [`instruction-structure-is-role-rules-steps.bad.al`](instruction-structure-is-role-rules-steps.bad.al). ## See also diff --git a/community/knowledge/agents/instructions-describe-work-not-tool-ids.md b/community/knowledge/agents/instructions-describe-work-not-tool-ids.md index a1a536a..1487364 100644 --- a/community/knowledge/agents/instructions-describe-work-not-tool-ids.md +++ b/community/knowledge/agents/instructions-describe-work-not-tool-ids.md @@ -17,13 +17,13 @@ Agent tools are the UI the profile exposes. Action names and tool ids change acr Write steps as business outcomes (release the order, set the hold reason). Tell the agent to memorize identifiers it must reuse. Do not hard-code action captions or tool ids. -See sample: `instructions-describe-work-not-tool-ids.good.al`. +See sample: [`instructions-describe-work-not-tool-ids.good.al`](instructions-describe-work-not-tool-ids.good.al). ## Anti Pattern Instructions that say invoke SalesOrder.Post_Promoted or use tool page-42-action-3. Detection signal: instruction text containing Promoted action names or tool identifiers. -See sample: `instructions-describe-work-not-tool-ids.bad.al`. +See sample: [`instructions-describe-work-not-tool-ids.bad.al`](instructions-describe-work-not-tool-ids.bad.al). ## See also diff --git a/community/knowledge/agents/reapply-resource-instructions-on-upgrade.md b/community/knowledge/agents/reapply-resource-instructions-on-upgrade.md index 5345d25..db95258 100644 --- a/community/knowledge/agents/reapply-resource-instructions-on-upgrade.md +++ b/community/knowledge/agents/reapply-resource-instructions-on-upgrade.md @@ -17,10 +17,10 @@ Static instructions stored as an app resource are copied onto an instance only w In the upgrade codeunit, find existing instances of your metadata provider and call `SetInstructions` again with `NavApp.GetResourceAsText`. Guard with an upgrade tag so the rewrite runs once per version that changes the file. -See sample: `reapply-resource-instructions-on-upgrade.good.al`. +See sample: [`reapply-resource-instructions-on-upgrade.good.al`](reapply-resource-instructions-on-upgrade.good.al). ## Anti Pattern Editing only the resource file, or calling `SetInstructions` solely from the first-time setup path. Detection signal: instruction resource in `resourceFolders` with no upgrade procedure that re-applies it. -See sample: `reapply-resource-instructions-on-upgrade.bad.al`. +See sample: [`reapply-resource-instructions-on-upgrade.bad.al`](reapply-resource-instructions-on-upgrade.bad.al). diff --git a/community/knowledge/agents/register-copilot-capability-for-the-agent.md b/community/knowledge/agents/register-copilot-capability-for-the-agent.md index 79dfe44..59171cc 100644 --- a/community/knowledge/agents/register-copilot-capability-for-the-agent.md +++ b/community/knowledge/agents/register-copilot-capability-for-the-agent.md @@ -17,13 +17,13 @@ Each agent type needs a `Copilot Capability` enum value that the factory links a Extend `Copilot Capability` with a unique value. In `OnInstallAppPerDatabase`, call `Copilot Capability.IsCapabilityRegistered` and, if false, `RegisterCapability` with availability, billing type, and a learn-more URL. Point `IAgentFactory` at that capability. -See sample: `register-copilot-capability-for-the-agent.good.al`. +See sample: [`register-copilot-capability-for-the-agent.good.al`](register-copilot-capability-for-the-agent.good.al). ## Anti Pattern Shipping the agent enum without a `Copilot Capability` value, or adding the enum but never calling `RegisterCapability`. Duplicate ordinals across extensions also collide. Detection signal: agent metadata provider with no matching capability registration in an install codeunit. -See sample: `register-copilot-capability-for-the-agent.bad.al`. +See sample: [`register-copilot-capability-for-the-agent.bad.al`](register-copilot-capability-for-the-agent.bad.al). ## See also diff --git a/community/knowledge/agents/set-instructions-as-secrettext.md b/community/knowledge/agents/set-instructions-as-secrettext.md index 0f246f6..4c52863 100644 --- a/community/knowledge/agents/set-instructions-as-secrettext.md +++ b/community/knowledge/agents/set-instructions-as-secrettext.md @@ -17,10 +17,10 @@ Instructions are instance data, not an enum caption. `Agent.SetInstructions` tak Load instruction text from a resource or builder into a `SecretText` variable and call `Agent.SetInstructions(AgentUserSecurityId, Instructions)` after `Create`. Keep one instruction document per instance. -See sample: `set-instructions-as-secrettext.good.al`. +See sample: [`set-instructions-as-secrettext.good.al`](set-instructions-as-secrettext.good.al). ## Anti Pattern Passing a `Label` or `Text` to `SetInstructions`, storing instructions in a setup Text field without wrapping as `SecretText`, or putting the prompt only in a code comment. Detection signal: `SetInstructions` with a non-`SecretText` argument, or no `SetInstructions` after `Create`. -See sample: `set-instructions-as-secrettext.bad.al`. +See sample: [`set-instructions-as-secrettext.bad.al`](set-instructions-as-secrettext.bad.al). diff --git a/community/knowledge/agents/show-can-create-agent-does-not-block-code-create.md b/community/knowledge/agents/show-can-create-agent-does-not-block-code-create.md index 1eb151d..7a935e1 100644 --- a/community/knowledge/agents/show-can-create-agent-does-not-block-code-create.md +++ b/community/knowledge/agents/show-can-create-agent-does-not-block-code-create.md @@ -17,10 +17,10 @@ application-area: [all] Use `ShowCanCreateAgent` to decide discovery. If only agent administrators should see the type, return `Agent System Permissions.CurrentUserHasCanManageAllAgentsPermission`. Enforce extra policy inside your own create API. Never assume UI hiding blocks code. -See sample: `show-can-create-agent-does-not-block-code-create.good.al`. +See sample: [`show-can-create-agent-does-not-block-code-create.good.al`](show-can-create-agent-does-not-block-code-create.good.al). ## Anti Pattern Returning `exit(false)` from `ShowCanCreateAgent` and then documenting that instances cannot be created, while page actions or other apps still call `Agent.Create`. Detection signal: `ShowCanCreateAgent` always false with no matching guard on programmatic create. -See sample: `show-can-create-agent-does-not-block-code-create.bad.al`. +See sample: [`show-can-create-agent-does-not-block-code-create.bad.al`](show-can-create-agent-does-not-block-code-create.bad.al). diff --git a/community/knowledge/agents/skip-incoming-review-only-for-trusted-input.md b/community/knowledge/agents/skip-incoming-review-only-for-trusted-input.md index 449038f..ad91408 100644 --- a/community/knowledge/agents/skip-incoming-review-only-for-trusted-input.md +++ b/community/knowledge/agents/skip-incoming-review-only-for-trusted-input.md @@ -17,10 +17,10 @@ Incoming task messages default to requiring user approval before the agent runs. Leave the default review-on for anything that originated outside your extension. Call `SetRequiresReview(false)` only on messages you constructed from already-authorized BC data. -See sample: `skip-incoming-review-only-for-trusted-input.good.al`. +See sample: [`skip-incoming-review-only-for-trusted-input.good.al`](skip-incoming-review-only-for-trusted-input.good.al). ## Anti Pattern `SetRequiresReview(false)` on simulated email, incoming webhooks, or user-free text. Detection signal: `SetRequiresReview(false)` next to external content with no prior validation. -See sample: `skip-incoming-review-only-for-trusted-input.bad.al`. +See sample: [`skip-incoming-review-only-for-trusted-input.bad.al`](skip-incoming-review-only-for-trusted-input.bad.al). diff --git a/community/knowledge/agents/use-documented-instruction-keywords.md b/community/knowledge/agents/use-documented-instruction-keywords.md index 2150a5d..a65f240 100644 --- a/community/knowledge/agents/use-documented-instruction-keywords.md +++ b/community/knowledge/agents/use-documented-instruction-keywords.md @@ -17,13 +17,13 @@ The agent runtime looks for specific phrases: ask for assistance, request a revi In the instruction resource, use those keywords at the decision points: request a review before posting; write an email only after stating that outbound mail is reviewed; memorize values the later steps need. Pair `Reply` / `Write an email` with an explicit review sentence. -See sample: `use-documented-instruction-keywords.good.al`. +See sample: [`use-documented-instruction-keywords.good.al`](use-documented-instruction-keywords.good.al). ## Anti Pattern Inventing tool-like verbs (call Copilot, click Post_Promoted) or omitting request a review before posting. Detection signal: instruction text that says email the customer with no review keyword. -See sample: `use-documented-instruction-keywords.bad.al`. +See sample: [`use-documented-instruction-keywords.bad.al`](use-documented-instruction-keywords.bad.al). ## See also diff --git a/community/knowledge/agents/wire-all-three-agent-interfaces.md b/community/knowledge/agents/wire-all-three-agent-interfaces.md index d3ce4b2..87e859f 100644 --- a/community/knowledge/agents/wire-all-three-agent-interfaces.md +++ b/community/knowledge/agents/wire-all-three-agent-interfaces.md @@ -17,13 +17,13 @@ An AL agent type is registered by extending `Agent Metadata Provider`. The platf On the enum value, set `Implementation` for all three interfaces, each pointing at a dedicated codeunit. Keep factory (create, defaults, first-time setup), metadata (setup page, summary, annotations), and task execution (message analysis, intervention suggestions) in separate objects. -See sample: `wire-all-three-agent-interfaces.good.al`. +See sample: [`wire-all-three-agent-interfaces.good.al`](wire-all-three-agent-interfaces.good.al). ## Anti Pattern An `Agent Metadata Provider` value with no `Implementation`, only one interface mapped, or all three interfaces pointing at one catch-all codeunit that cannot satisfy the contracts. Detection signal: enumextension of `Agent Metadata Provider` whose value does not list `IAgentFactory`, `IAgentMetadata`, and `IAgentTaskExecution`. -See sample: `wire-all-three-agent-interfaces.bad.al`. +See sample: [`wire-all-three-agent-interfaces.bad.al`](wire-all-three-agent-interfaces.bad.al). ## See also diff --git a/community/skills/review/al-agents-review.md b/community/skills/review/al-agents-review.md index 4bc2a6b..80dadb0 100644 --- a/community/skills/review/al-agents-review.md +++ b/community/skills/review/al-agents-review.md @@ -4,7 +4,7 @@ id: al-agents-review version: 1 title: AL agents review description: Reviews AL source changes against agent guidance from BCQuality. -inputs: [pr-diff, file-path] +inputs: [pr-diff, file-path, folder-path] outputs: [findings-report] bc-version: [all] technologies: [al] @@ -18,7 +18,10 @@ Reviews AL source changes against the `agents` knowledge domain in BCQuality and Agent findings apply to AL files that implement or invoke Agent SDK surfaces, including agent interfaces, setup, creation, task execution, capability registration, profiles, access controls, instructions, and session-bound subscribers. Return `not-applicable` when the diff contains no AL changes or no Agent SDK implementation or usage. -An orchestrator invokes this skill with either a `pr-diff` or a `file-path`. The skill produces one JSON document conforming to the DO output contract. +An orchestrator invokes this skill with a `pr-diff`, `file-path`, or +`folder-path`. For a folder, review all relevant source below it under DO's +current-state input semantics. The skill produces one JSON document +conforming to the DO output contract. ## Source diff --git a/custom/README.md b/custom/README.md index 28d7aa9..2719df8 100644 --- a/custom/README.md +++ b/custom/README.md @@ -12,6 +12,17 @@ custom/ ## How to use -Fork or clone BCQuality into your own repository and add your content here. Knowledge files in `/custom/knowledge/` follow the same frontmatter schema and section requirements as every other layer. Action skills in `/custom/skills/` follow the Action Skill template defined in `/skills/`. +Use a fork or organization-controlled copy of BCQuality, not the upstream +repository or your AL app's source folder. Confirm `git remote get-url origin` +points at your repository before adding custom content. Upstream does not +accept custom rules. -When agents consume BCQuality, the custom layer is loaded alongside Microsoft and Community โ€” your overrides apply automatically. +Follow [Customizing BCQuality](../docs/customizing-bcquality.md) for a worked +rule, plugin configuration, installing your fork, and keeping it up to date. +Adding a rule here does not update an existing upstream plugin installation; +your host must consume your copy. + +Knowledge files follow [READ](../skills/read.md) and action skills follow +[DO](../skills/do.md). With the Custom layer enabled, applicable custom +knowledge overrides contradictory Community or Microsoft guidance. The report +records the displaced article; non-conflicting guidance remains additive. diff --git a/docs/README.md b/docs/README.md index 466b98e..1d7248b 100644 --- a/docs/README.md +++ b/docs/README.md @@ -1,8 +1,32 @@ -# Documentation +# BCQuality documentation -- [How agents consume BCQuality](agent-consumption.md) explains the operational - flow from Entry dispatch through structured findings and integration. -- [Build a lightweight standalone review runner](standalone-runner.md) explains - one concrete walk-up skill flow and how an external runner can add model - selection, concurrency, retries, and telemetry without moving orchestration - into BCQuality. +**New to BCQuality? Start with the [quick start](../README.md#quick-start).** +Install the plugin, discover its skills, and try an app review. No knowledge +of BCQuality's internal protocol is needed. + +## Partner guides + +| Goal | Guide | +| --- | --- | +| Review an app, file, changes, or branch | [Using BCQuality](using-bcquality.md) | +| Understand a report and its limitations | [Reading your results](using-bcquality.md#reading-your-results) | +| Find a particular rule or example | [Knowledge by domain](using-bcquality.md#knowledge-by-domain) | +| Fix setup problems or report an incorrect finding | [Troubleshooting and support](troubleshooting.md) | +| Select layers, add company rules, or maintain a fork | [Customizing BCQuality](customizing-bcquality.md) | +| Add or improve shared knowledge | [Contributing](contributing.md) | + +## Integration and technical reference + +These pages are for people building integrations or maintaining skills, not +prerequisites for using the plugin. + +| Reference | Purpose | +| --- | --- | +| [How agents consume BCQuality](agent-consumption.md) | Architecture, repository structure, routing, and delivery of findings. | +| [Standalone runner](standalone-runner.md) | Optional model selection, scheduling, retries, and telemetry. | +| [Global skills](../skills/README.md) | Host adapters versus internal protocol files. | +| [Entry](../skills/entry.md) | Task context and skill dispatch. | +| [READ](../skills/read.md) | Knowledge schema, applicability, and precedence. | +| [DO](../skills/do.md) | Action-skill format and structured output contract. | +| [WRITE](../skills/write.md) | Knowledge-authoring rules. | +| [Review evaluation](../evaluation/README.md) | Sample conventions, fixture preparation, and scoring. | diff --git a/docs/agent-consumption.md b/docs/agent-consumption.md index 2cb3761..1bf4284 100644 --- a/docs/agent-consumption.md +++ b/docs/agent-consumption.md @@ -5,13 +5,15 @@ supplied by a host or orchestrator. This document explains the end-to-end flow so that skill authors, orchestrator maintainers, and contributors share one mental model. -For the high-level framing and repo structure, start with the -[README](../README.md). This document is the operational view. +[Documentation](README.md) | [Partner quick start](../README.md#quick-start) | [Runner contract](standalone-runner.md) + +This is the operational reference for integration authors. Partners using +the installed plugin do not need to implement this flow themselves. ## The actors - **Orchestrator** โ€” the tool that triggers work. Lives *outside* BCQuality. Knows *when* to run something, not *what* to run. -- **Agent** โ€” an LLM-driven process spawned by the orchestrator. The agent has no built-in knowledge of BC or of BCQuality's conventions. It knows how to read instructions and call tools. +- **Agent** โ€” an LLM-driven process supplied by the host. It brings its own coding knowledge and tools; BCQuality adds curated guidance and execution contracts. - **BCQuality repo** โ€” two kinds of content: - **Global skills** in `/skills/` โ€” the `entry.md` entry-point skill plus the READ ยท DO ยท WRITE contracts that govern the rest of the repo. - **Layer content** in `/microsoft/`, `/community/`, and `/custom/` โ€” knowledge files and action skills grouped by authority. @@ -20,6 +22,25 @@ When BCQuality is installed as a standalone plugin, it additionally exposes `skills/al-code-review/SKILL.md`. This is a host-format adapter, not another action skill: it creates the task context and enters the same flow at Entry. +## Repository structure + +| Path | Purpose | +| --- | --- | +| `skills/entry.md` | Routes a task to action skills. | +| `skills/read.md`, `skills/do.md`, `skills/write.md` | Stable knowledge, action-skill, and authoring contracts. | +| `skills/al-code-review/SKILL.md` | Host-format plugin adapter. | +| `/knowledge//` | Atomic articles and optional sibling samples. | +| `/skills/` | Layer-owned action skills. | +| `docs/` | Partner guides and integration references. | +| `evaluation/` | Neutral review fixtures and scoring contract. | +| `tools/` | Knowledge-index and evaluation tooling. | +| `.github/` | Validation and repository workflows. | + +Layers are `microsoft`, `community`, and `custom`; Custom is a template for +consumer forks. An action skill either evaluates knowledge directly (a leaf) +or composes declared leaves (a super-skill). See [global skills](../skills/README.md) +for the distinction between host-native packaging and these internal formats. + ## The flow ```mermaid @@ -70,7 +91,17 @@ At this point the agent reads READ and DO on demand โ€” it needs READ to interpr Discovering candidates at the Source step naively means opening every file under a domain folder just to read its frontmatter `keywords` โ€” on a large corpus that is hundreds of file reads per review. To avoid this, BCQuality maintains a **knowledge index**: a single artifact (`knowledge-index.json`) that lists every article surviving the consumer's layer/allow-deny filtering and carries, per article, the exact inputs the Source/Worklist steps consume โ€” `path`, `layer`, `domain`, frontmatter dimensions, `keywords`, `title`, and a one-line `description` hint. -The index is **owned and produced by BCQuality**, not by each consumer: its generator (`tools/Build-KnowledgeIndex.ps1`) ships here, next to the skills and knowledge it derives from, so the index schema stays in lockstep with the Source contract and every consumer gets the same faithful index for free instead of re-implementing the parser. The consuming orchestrator does **not** build or invoke the index โ€” it only prunes its clone to policy as it already does. The index is then (re)generated by BCQuality itself: **Entry's preparation step runs `Build-KnowledgeIndex.ps1` over the live, already-pruned clone** at the start of every run (see `skills/entry.md`), and BCQuality CI (`.github/workflows/knowledge-index.yml`) validates that the generator is healthy and deterministic. Building over the *pruned* clone โ€” rather than shipping a committed full-corpus index that consumers trust โ€” keeps the index exact for any consumer policy: it can never list an article the consumer denied, so policy-excluded rules cannot leak into discovery. +The index is **owned and produced by BCQuality**, not reimplemented by each +consumer. Its generator, `tools/Build-KnowledgeIndex.ps1`, ships here alongside +the content. Entry ensures the index reflects the live tree before routing +and regenerates it when absent or not known to be current. BCQuality CI +validates that the generator is healthy and deterministic. + +Consumers with allow/deny policy must prune their content copy **before** +Entry runs. Building over that pruned tree prevents removed articles from +entering discovery. A standalone plugin normally ships the whole tree: +`enabled-layers` filters discovery but does not remove files or enforce a +security boundary. See [layer selection](customizing-bcquality.md#select-layers-or-disable-a-review). The index changes only *how candidates are discovered*, never *which are selected*. The Worklist predicate is unchanged โ€” `keywords` still drive selection โ€” and the agent still opens each worklisted article **in full** to read its `## Best Practice` / `## Anti Pattern` rule bodies; the index is discovery metadata only and never substitutes for the article body. When no index is present, skills fall back to path-based discovery (collect by domain folder), so review still works. diff --git a/docs/contributing.md b/docs/contributing.md new file mode 100644 index 0000000..b4cbb0b --- /dev/null +++ b/docs/contributing.md @@ -0,0 +1,137 @@ +# Contributing to BCQuality + +[Documentation](README.md) | [Knowledge by domain](using-bcquality.md#knowledge-by-domain) | [Authoring reference](../skills/write.md) + +Partners are welcome to contribute shared knowledge, examples, skills, and +documentation. To report an incorrect finding without preparing a change, +use the [support guide](troubleshooting.md#reporting-a-problem). + +## What belongs here + +BCQuality is a remedial knowledge base. A knowledge file exists because a +capable LLM **would get something wrong, or miss something, without it**, not +simply because the topic is important. Apply this admission test: + +> If this file did not exist, would a modern LLM reviewing or generating BC +> code make a mistake this file would have prevented? + +Good candidates encode a BC-specific mechanic that models get wrong, a +version-dependent behavior, or a misleading interpretation of an analyzer +rule. For example: + +- [SetLoadFields and filters can be called in either order](../microsoft/knowledge/performance/use-setloadfields-for-partial-records.md): their relative order does not change the projection. This prevents an incorrect performance finding. +- [Boolean page record triggers default to true](../microsoft/knowledge/error-handling/page-boolean-triggers-default-to-true.md): omitting an explicit `exit(true)` is not itself a defect. +- [Page fields can inherit captions](../microsoft/knowledge/style/caption-required-on-page-fields.md): an omitted page-level property is not sufficient evidence that a caption is missing. + +Generic advice such as "use HTTPS," "do not hardcode secrets," or "keep +transactions short" does not earn a separate knowledge file merely by being +sound advice. Negative clarifications that prevent false positives are as +valuable as rules that catch defects. + +**Skills hold discovery and execution mechanics; knowledge files hold BC +facts.** Correct or extend a knowledge article when a BC fact is missing or +wrong. Do not hide that fact in a skill's instructions. A genuine routing, +input, or output-contract problem belongs in the skill instead. + +## Choose the right destination + +| Change | Destination | +| --- | --- | +| Knowledge in a Microsoft-owned review domain | `microsoft/knowledge//` | +| Knowledge accompanying a Community-owned skill | `community/knowledge//` | +| Company-specific policy or an override | `custom/` in your own fork; never an upstream contribution | +| Partner instructions or how-to guidance | `docs/`, linked from the documentation index | + +Layer ownership follows the skill and domain, **not your employer**. For +example, a partner's performance clarification belongs beside the Microsoft +performance skill's corpus. Do not use Community as a staging area for an +already Microsoft-owned domain. A split may exist briefly during promotion, +but the skill and its canonical corpus should move together. + +Upstream automatically closes PRs adding custom content. Follow +[Customizing BCQuality](customizing-bcquality.md) for organization-only rules. +Do not introduce a new shared domain without the action skill that consumes +it and the matching evaluation samples. + +## Author a knowledge article + +Read [READ](../skills/read.md) for the schema and +[WRITE](../skills/write.md) for the authoring rules. Use an existing article +in the same domain as a starting point, then remove unrelated guidance. + +Every article has six required frontmatter fields: `bc-version`, `domain`, +`keywords`, `technologies`, `countries`, and `application-area`. +`domain` must match its containing directory. Keep one concern per file, +ideally under 50 lines and no more than 100. + +`Description` is required. Put recommendations in `Best Practice` and mistakes +to catch in `Anti Pattern`; those are the normative sections. Explain +legitimate exceptions so a reviewer does not turn a useful rule into a false +positive. Code fences are not allowed in knowledge articles. + +### Sources and examples + +When adding or changing a platform claim, link the authoritative source that +supports it, preferably the specific Microsoft Learn API/property page or a +public source definition. State version constraints when they matter. Avoid +"upstream guidance says" without a link. If the source is unavailable or the +guidance is organization policy or empirical observation, say so explicitly +rather than presenting it as an official platform guarantee. + +Place source links in a short `References` section or beside the relevant +claim. References do not replace the rule: keep all load-bearing guidance in +the normative sections. This adds traceability without adding frontmatter +fields or changing the schema. + +Put demonstration code in sibling files: + +```text +.md +.good.al +.bad.al +``` + +Reference each sample with a clickable link whose label retains the filename, +for example `` [`.good.al`](.good.al) `` with your actual slug. +One or both samples are optional for an individual article; every review +domain must have at least one complete good/bad pair for evaluation. Samples +are self-contained demonstrations, not copied Base Application source and +not a deployable or compiled application. + +## Before opening a PR + +From your BCQuality checkout, use the existing validators. The Python +validator needs Python and PyYAML; the fixture harness needs PowerShell 7. +If PyYAML is not installed in your development environment, install it with +`python -m pip install pyyaml`. + +```powershell +python .github\scripts\validate_frontmatter.py --root . +pwsh .\tools\Test-ReviewFixtures.ps1 -Root . +``` + +The first command checks schema, sections, naming, sample references, and +skill registration. The second checks that every review leaf has a valid +positive/clean sample pair. Neither proves a model will find every defect. +See [evaluation](../evaluation/README.md) for optional model-based scoring. + +In the PR description, explain the mistake being prevented, supporting +evidence, applicable BC versions, and why the chosen domain owns it. For a +false positive, include the valid pattern and the incorrect finding being +prevented. Check that links and samples open from the rendered article. + +Schema and stable protocol changes require approval from both maintainers. +Avoid repeating schema or contract definitions in new guides: link the +canonical READ, DO, WRITE, or Entry section instead. + +## Content releases + +Maintainers cut content releases on demand, roughly monthly, using the +`Release version` workflow on `main`. It tags the selected commit as +`v{major}.{minor}`; it does not update the plugin manifest. + +Use a minor bump for normal content updates and a major bump for breaking +changes. The minor is a monotonic counter: it increments across releases and +does **not** reset on a major bump. See +[updates and versions](customizing-bcquality.md#updates-and-versions) for the +separate plugin, content, and skill version identifiers. diff --git a/docs/customizing-bcquality.md b/docs/customizing-bcquality.md new file mode 100644 index 0000000..d3d9148 --- /dev/null +++ b/docs/customizing-bcquality.md @@ -0,0 +1,173 @@ +# Customizing BCQuality + +[Documentation](README.md) | [Using BCQuality](using-bcquality.md) | [Contributing](contributing.md) + +**No customization is required to get started.** Use the upstream plugin +unless you need a different review selection or organization-specific rules. +Model choice, concurrency, retries, and billing belong to your host, not +BCQuality. A [standalone runner](standalone-runner.md) is an advanced option. + +## Select layers or disable a review + +The standalone adapter reads these environment variables from the process +that starts your host: + +| Variable | Default | Meaning | +| --- | --- | --- | +| `BCQUALITY_ENABLED_LAYERS` | `microsoft,community,custom` | Comma-separated layer names to discover. | +| `BCQUALITY_DISABLED_SKILLS` | None | Comma-separated **BCQuality repo-relative skill paths** to exclude, not display names or knowledge-article paths. | + +For example, in PowerShell, enable only Microsoft knowledge and omit the +dedicated style review: + +```powershell +$env:BCQUALITY_ENABLED_LAYERS = "microsoft" +$env:BCQUALITY_DISABLED_SKILLS = "microsoft/skills/review/al-style-review.md" +copilot +``` + +Set the variables **before** starting a new session. They apply to that +terminal and its child processes; use your host's environment configuration +if it starts elsewhere. Review selection is not a guarantee that another +domain or the agent will never mention a related concern. + +To return to defaults, remove those variables from the environment before +starting the host again (or use a fresh terminal if you only set them there). +Do not use an empty comma-separated value as a substitute for the default. + +All layers are enabled by default. Where relevant articles have overlapping +applicability and **contradictory guidance**, precedence is: + +**Custom > Community > Microsoft.** + +Otherwise the layers are additive. A matching filename alone does not suppress +an article; the [READ contract](../skills/read.md#layer-precedence) governs +knowledge conflicts. Review reports record displaced knowledge in `suppressed`. + +Layer selection is **not an access-control boundary**. A plugin installation +still contains excluded layers on disk. An integration requiring genuine +exclusion must remove denied files from its own content copy before the agent +reads it; the [adapter](../skills/al-code-review/SKILL.md#layer-selection-is-not-a-deny-mechanism) +explains this distinction. + +## Add an organization-specific rule + +Keep custom content in a fork or organization-controlled copy of BCQuality, +not in your AL app's `custom` folder and not in the installed plugin cache. +Editing the cache is not durable across updates. + +1. Fork BCQuality into a repository your organization controls, or create an + organization-controlled copy if a public fork is unsuitable for your policy. +2. Clone that repository and run `git remote get-url origin`. Confirm it is + your repository, **not** `microsoft/BCQuality`. +3. Add the article under `custom/knowledge//`, using the + [knowledge format](../skills/read.md). Keep your company's content out of + upstream pull requests. + +For example, suppose your company deliberately names one page "ACME Inventory +Workbench" while showing stockkeeping units, and already makes the row type +clear in its UI. You want a narrow exception to the shared page-naming rule. +Create `custom/knowledge/style/page-name-must-match-source-table.md` in your +copy with this content: + +```markdown +--- +bc-version: [all] +domain: style +keywords: [page-name, source-table, inventory, workbench] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Allow the ACME Inventory Workbench task name + +## Description + +Our approved page "ACME Inventory Workbench" shows stockkeeping units. Its UI +identifies the row type explicitly; its task-oriented name is company policy. + +## Best Practice + +Do not report that page solely because its name differs from its source-table +entity. Keep the shared naming guidance for other pages. + +## Anti Pattern + +Renaming the approved page solely to repeat the source-table entity, or +applying this exception to an unrelated page. +``` + +This is an **illustrative company policy**, not a new Microsoft recommendation. +Choose your actual domain, applicability, and policy; do not broaden an +exception merely to silence a valid defect. The shared rule is +[page-name-must-match-source-table.md](../microsoft/knowledge/style/page-name-must-match-source-table.md). +Guidance in `Best Practice` and `Anti Pattern` drives conflict resolution, so +do not put the exception only in a non-normative notes section. + +Follow the [contribution checks](contributing.md#before-opening-a-pr) locally, +then commit your change in your repository. A new knowledge domain also needs +an action skill that discovers it; adding an arbitrary folder does not create +a review. + +## Use your fork + +Adding custom content does not change the upstream plugin you already +installed. Point the host at your copy. + +For a pushed fork, replace `YOUR-ORG` with its owner. These commands replace +the upstream installation, since both manifests use the name `bcquality`: + +```powershell +copilot plugin uninstall bcquality +copilot plugin install YOUR-ORG/BCQuality +copilot plugin list +``` + +For local development, install your copy's absolute path instead: + +```powershell +copilot plugin install "C:\Repos\CompanyBCQuality" +``` + +Direct local installs are cached by the CLI; reinstall that path after edits, +then start a new session. See the host's +[local-plugin instructions](https://docs.github.com/en/copilot/how-tos/copilot-cli/customize-copilot/plugins-creating). +Do not assume the currently running session has reloaded the content. + +Confirm the plugin list points at the intended source and that the Custom +layer is enabled. Review a small example relevant to your rule. Ask the host +which custom article it read and inspect `suppressed` for an actual conflict. +The negative-rule example should produce no naming finding for the approved +page; do not add an information-only finding just to prove the article was +loaded. The absence of a finding alone does not prove your fork was used. + +An external runner should likewise read from your fork or local copy rather +than the upstream URL. It must still start at [Entry](../skills/entry.md). + +## Updates and versions + +| Identifier | What it identifies | +| --- | --- | +| Plugin `version` in `plugin.json` | The host-facing package version. It is separate from content-release tags. `copilot plugin list` shows the installed plugin; inspect the resolved source when reproducing a run. | +| Content tag such as `v1.6` | A release of the repository's knowledge and skills. Available tags are listed on [GitHub](https://github.com/microsoft/BCQuality/tags). | +| Skill `version` in frontmatter | That skill's contract version, carried in reports. It does not identify the complete knowledge snapshot. | +| Git commit SHA | The exact repository snapshot. Record this for reproducibility when using a checkout. | + +For the upstream plugin, run `copilot plugin update bcquality`, then start a +new session. The unpinned installation command does not promise a particular +content-release tag. For a fork, updating the plugin reads your fork; it does +not merge upstream changes into it. + +To maintain a fork, commit your custom work first, add an `upstream` remote +pointing to `https://github.com/microsoft/BCQuality.git` once, fetch upstream, +and merge the desired upstream branch or content tag. Resolve conflicts and +review the resulting policy before publishing or reinstalling your fork. +Do not overwrite the fork wholesale with an upstream download. + +For repeatable CI or runner use, select a tag or commit in a dedicated clean +checkout and record `git rev-parse HEAD`. Upgrade deliberately, compare the +old and new content, and rerun representative reviews. To roll back, select +the previously recorded snapshot in that checkout and reinstall it if your +host caches local plugins. Retain organization-specific rules in the chosen +snapshot rather than reverting to an upstream-only tag. diff --git a/docs/standalone-runner.md b/docs/standalone-runner.md index 8bb1e67..31ab1dd 100644 --- a/docs/standalone-runner.md +++ b/docs/standalone-runner.md @@ -1,5 +1,7 @@ # Build a lightweight standalone review runner +[Documentation](README.md) | [Architecture](agent-consumption.md) + BCQuality provides review knowledge, routing, execution instructions, and structured output contracts. It intentionally does not choose models, schedule agents, retry failures, or collect usage telemetry. A standalone runner can add @@ -12,12 +14,9 @@ concurrency, or integration with another review surface. ## Keep BCQuality current -Install or update the plugin with GitHub Copilot CLI: - -```shell -copilot plugin install microsoft/BCQuality -copilot plugin update bcquality -``` +For plugin installation, use the [quick start](../README.md#quick-start). +For version identifiers, forks, and reproducible snapshots, see +[updates and versions](customizing-bcquality.md#updates-and-versions). A runner that reads BCQuality from a checkout should pin a commit or release and upgrade it deliberately. Do not copy knowledge files or action-skill prose @@ -30,16 +29,13 @@ diff, supply the app's root directory as `folder-path`. The review scope is every relevant file below that directory, including `app.json` and AL source. The folder does not need to be a Git repository. -With the standalone plugin installed, start a fresh Copilot session in the app -folder and ask: - -> Use the installed `al-code-review` skill to review the complete Business -> Central app in this folder. Execute every dispatched review domain and return -> the complete BCQuality findings report. - -The adapter maps this request to `folder-path`; Entry routes it to the broad -review super-skill. Because a folder is a current-state snapshot, the review -must not invent a previous app version when evaluating comparison-only rules. +The [app-review example](../README.md#example-review-a-complete-app-folder) +uses this input through the standalone adapter. Because a folder is a +current-state snapshot, the review must not invent a previous app version +when evaluating comparison-only rules. Entry can return more than one +top-level skill; preserve all reports, including separately dispatched +Community reviews, rather than assuming the Microsoft coordinator is the +only result. ## Minimal runner flow diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md new file mode 100644 index 0000000..bdfcd72 --- /dev/null +++ b/docs/troubleshooting.md @@ -0,0 +1,59 @@ +# Troubleshooting and support + +[Documentation](README.md) | [Quick start](../README.md#quick-start) | [Using BCQuality](using-bcquality.md) + +## Setup and skill discovery + +Run terminal commands outside the interactive Copilot prompt unless they +start with `/`. + +| Symptom | What to do | +| --- | --- | +| `copilot` is not recognized | [Install Copilot CLI](https://docs.github.com/en/copilot/get-started/cli-quickstart), then open a new terminal. Installing Copilot Chat in an editor is not the same step. | +| The CLI has no `plugin` command | Update Copilot CLI using its installation method. Confirm `copilot plugin --help` works. | +| Sign-in, entitlement, or organization-policy error | Start `copilot`, use `/login`, and confirm your account is allowed to use Copilot CLI. Ask your administrator about organization restrictions; BCQuality cannot override them. | +| Plugin installation cannot reach the repository | Confirm access to `https://github.com/microsoft/BCQuality` and follow your organization's proxy/network guidance. Do not disable certificate checks. | +| The plugin installed, but the skill is missing | Run `copilot plugin list` in the terminal. Enable it with `copilot plugin enable bcquality` if disabled, then start a new session. In the session, use `/skills list` and look for `al-code-review`. | +| The agent performs a generic review | Name the **installed `al-code-review` skill** explicitly, as in the quick start. Ask which skill and BCQuality source it used. Another plugin or host may expose a similarly named operation. | +| Installation works in the terminal, but not in the editor | Plugin discovery is host-specific. Follow the editor's installation instructions; a CLI installation is not proof that another host loaded the plugin. | +| `pwsh` is missing, or index generation fails | The index is an accelerator, not required knowledge. The review can fall back to discovery from folders. For faster discovery, install [PowerShell 7](https://learn.microsoft.com/en-us/powershell/scripting/install/installing-powershell), or resolve the reported filesystem error. | +| An update or local edit is not visible | Run `copilot plugin update bcquality` for a repository-installed plugin, then start a fresh session. For a directly installed local folder, reinstall that folder to refresh the cached copy; see [customizing](customizing-bcquality.md#use-your-fork). | + +## Review results + +| Symptom | What to do | +| --- | --- | +| `partial`, a timeout, or an unfinished review | Read `outcome-reason` and domain reports. Retry the incomplete scope in a fresh session, use smaller app folders or a focused review, or select a host/model with sufficient capacity. Keep the limited scope visible; do not relabel it a complete app review. | +| `failed` | Resolve the stated problem, such as inaccessible input, a failed invocation, or an unverifiable reference, before using that report. A failed domain's findings are not reliable. | +| `no-match` or `not-applicable` | Confirm you supplied AL source, the intended folder/file/diff, and an appropriate goal. Check [disabled skills and layers](customizing-bcquality.md#select-layers-or-disable-a-review). | +| `no-knowledge` | Check the target BC version, selected domain, enabled layers, and whether the relevant knowledge files are present. No applicable rules is different from no defects. | +| `completed` with no findings | This can be a valid clean result for the selected scope. Confirm the intended files and domain reports are included. If you have a concrete missed defect, report it with a minimal example. | +| JSON rather than a readable summary | JSON is the shared output format. Ask the host to summarize the existing reports, preserving outcomes, locations, severity, confidence, and references. | +| A surprising finding | Open its guidance and samples, inspect surrounding code, and confirm version/localization assumptions. Ask the agent to explain the evidence; do not apply a suggestion solely because it has high confidence. | +| The Agents domain is absent | Agents is a separate Community review, not a child of the Microsoft broad review. Explicitly request an Agent SDK review and confirm the Community layer is enabled. | +| A missing base branch or unavailable source definition | Supply the real baseline or dependency definition. Without it, do not accept claims that rely on invented history or assumed dependency behavior. | +| Slow or expensive review | A broad review makes separate passes over multiple domains. Verify index generation succeeded, use a focused task when appropriate, and inspect usage in your host. BCQuality does not choose models, promise runtimes, or meter charges. | + +## Reporting a problem + +For incorrect BC guidance, missed findings, documentation gaps, or skill +behavior, [search existing issues](https://github.com/microsoft/BCQuality/issues) +and [open a BCQuality issue](https://github.com/microsoft/BCQuality/issues/new/choose) +if needed. You do not have to author a knowledge file before asking for help. +Host installation, authentication, billing, or policy problems belong with the +host's support channel or your organization administrator. + +Include: + +- The host and version, selected model if known, and BCQuality source/version + or commit. See [version identifiers](customizing-bcquality.md#updates-and-versions). +- The prompt, scope (folder/file/diff and comparison base), target BC version, + and relevant layer/skill settings. +- Expected versus actual behavior, the outcome/reason, and the exact rule + reference for a disputed finding. +- A **minimal, sanitized** AL example or report excerpt that reproduces the + problem. Remove secrets, customer data, and proprietary content you cannot share. + +For security vulnerabilities, follow [SECURITY.md](../SECURITY.md) instead of +opening a public issue. To contribute a correction yourself, follow the +[contribution guide](contributing.md). diff --git a/docs/using-bcquality.md b/docs/using-bcquality.md new file mode 100644 index 0000000..bb1d955 --- /dev/null +++ b/docs/using-bcquality.md @@ -0,0 +1,190 @@ +# Using BCQuality + +[Documentation](README.md) | [Quick start](../README.md#quick-start) | [Troubleshooting](troubleshooting.md) + +BCQuality supplies knowledge and reusable skills to your AI host. The plugin +currently exposes `al-code-review`; the examples below use that skill. The +host supplies authentication, model access, tools, permissions, and rendering. +Installing BCQuality does not install a Business Central extension or an agent. + +## Hosts and prerequisites + +The [quick start](../README.md#quick-start) documents GitHub Copilot CLI. Use a +current CLI release with plugin support and sign in to an account allowed to +use it. In an interactive CLI session, `/skills list` should include +`al-code-review`; in the terminal, `copilot plugin list` should include +`bcquality`. + +Do not assume a CLI installation also installs the plugin into VS Code, +another editor, or another agent host. Follow that host's plugin instructions +and confirm it discovers `skills/al-code-review/SKILL.md`. Hosts without +compatible plugin discovery need an [integration](agent-consumption.md). + +Source review needs access to your files, not a running BC environment. +Include `app.json` and any relevant surrounding source. Dependency symbols or +a historical baseline may be needed to substantiate particular findings; a +review must not invent missing definitions or an earlier version of your app. +PowerShell 7 (`pwsh`) accelerates discovery by generating the knowledge index. +Without it, folder-based discovery is available and may take longer. + +## Common review requests + +Start a new host session after installing or updating the plugin. Use the +skill name explicitly and say what is in scope. Replace example paths and +branch names with ones in your project. + +| Task | Example prompt | +| --- | --- | +| Complete app | Use the installed al-code-review skill to review the complete Business Central app in this folder without changing my source files. Return the complete BCQuality findings report. | +| One file | Use the installed al-code-review skill to review `src\CustomerMgt.Codeunit.al` without changing it. Return the complete BCQuality findings report. | +| Uncommitted changes | Use the installed al-code-review skill to review my staged and unstaged tracked changes against HEAD, without changing files. Identify any untracked AL files not included in that diff. | +| Branch changes | Use the installed al-code-review skill to review changes on this branch since its merge base with `origin/main`. Exclude uncommitted changes and do not edit files. | +| Focused review | Use the installed al-code-review skill to review performance in the app in this folder, without changing files. Return the complete performance findings report. | +| Agent SDK code | Use the installed al-code-review skill to review Agent SDK implementation and usage in this app folder, without changing files. Return the complete Agents findings report. | + +For Git comparisons, the named base ref must exist locally. If it is missing, +fetch the intended branch first. A PR review also requires the host to have +the PR's changes and repository access; installing the plugin does not +automatically connect it to your PR workflow. + +A complete-folder review considers relevant files recursively, not just +modified files. Start in a single app's root for the clearest scope. For a +repository containing several apps, name each app folder and review them +separately when their target versions or dependencies differ. + +If known, add the target BC major version and localization to the request. +Do not use your extension's own `version` as the BC version. Missing +applicability context can reduce a finding's confidence or leave a rule out. + +## Reading your results + +The skill returns structured reports. A host may render them as text, a table, +or annotations, or show the JSON directly. You can ask the host to explain the +returned report without rerunning the review or changing files. + +For example, a performance report could contain this finding: + +| Field | Illustrative value | +| --- | --- | +| Outcome | `completed` | +| Location | `src\CustomerExport.Codeunit.al`, line 42 | +| Severity / confidence | `major` / `high` | +| Finding | A country filter is evaluated inside the customer loop, so rows that will be discarded are still read. Apply the filter before iterating. | +| Guidance | [Apply filters before iterating](../microsoft/knowledge/performance/apply-filters-before-iterating.md), with linked good/bad samples. | + +This illustrates a report, not a guaranteed finding or host screen. Read the +referenced article and the surrounding source before accepting a fix. + +### Outcomes + +| Outcome | Meaning and action | +| --- | --- | +| `completed` | The selected review finished. An empty `findings` list means it found nothing to flag in that scope, not that the app is certified defect-free. | +| `not-applicable` | The review did not apply to the supplied input. It is not a clean-review result. | +| `no-knowledge` | No applicable knowledge was available. Check scope, target context, and enabled layers. | +| `partial` | Some work did not finish. Read `outcome-reason` and the individual reports; do not treat the result as a full pass. | +| `failed` | No reliable result from that review. Resolve the reported error before relying on it. | +| `no-match` | Routing found no suitable skill. Check the request, input type, and disabled skills. | + +A broad review includes individual domain reports in `sub-results`. Separately +dispatched skills return separate reports, so do not mistake the first report +for the whole run. Coverage counts describe selected knowledge items evaluated, +not a percentage of all possible defects or every rule in the repository. + +For example, this completed **domain** report evaluated one selected knowledge +item and found nothing to flag: + +```json +{ + "skill": { "id": "al-performance-review", "version": 1 }, + "outcome": "completed", + "summary": { + "counts": { "blocker": 0, "major": 0, "minor": 0, "info": 0 }, + "coverage": { "worklist-size": 1, "items-evaluated": 1 } + }, + "findings": [], + "suppressed": [] +} +``` + +This is not evidence that other domains ran; their reports must also be present +when requested. + +### Severity, confidence, and references + +| Severity | Meaning | +| --- | --- | +| `blocker` | A platform-level guarantee is violated; the work cannot proceed as-is. | +| `major` | A significant defect that should be addressed before merge. | +| `minor` | A quality concern; advisory rather than a gate. | +| `info` | Concrete context or an observation, not an instruction to change code. | + +Confidence (`high`, `medium`, or `low`) describes the strength of the evidence, +not the impact. Missing version or localization context must be disclosed in +the finding when conditionally applicable knowledge is used. + +Knowledge-backed findings link to the articles that informed them. Findings +from the agent's own reasoning have no knowledge reference (`references: []`); +these are advisory, with severity capped at `minor` and confidence at `medium`. +The display domain `Agents` means Agent SDK guidance; it is different from +`Agent`, the label for the broad coordinator's own cross-cutting observations. +Any `suppressed` entries explain knowledge overridden by configuration or +layer precedence. + +A report can include a code suggestion. **A suggestion is not an applied +change.** Review the explanation first, then request any edits explicitly, +for example: "Apply only the filter fix at line 42 from this report." Continue +using your normal compilation, analyzer, test, and human-review workflow. + +## Coverage and limits + +The Microsoft broad review composes the 16 Microsoft domains listed below. +The Community Agents review is a separate skill selected by the request, not +a nested part of that coordinator. All current review leaves accept app +folders, files, and diffs; request an Agent SDK review explicitly when that +coverage matters and look for its separate report. + +Available knowledge is **not** a promise that every rule will run. Selection +depends on the task, target context, enabled layers, and source evidence. +A whole-folder review is a current-state snapshot: detecting a published API +removal or another comparison-only regression requires an actual baseline. +The corpus is technical AL guidance, not exhaustive functional validation or +AppSource certification. + +### Knowledge by domain + +Each article describes one concern. Where samples exist, use its linked +`.good.al` and `.bad.al` files. Samples are demonstrations, not a deployable app. + +| Domain | Browse knowledge | +| --- | --- | +| Agent SDK | [Agents (Community)](../community/knowledge/agents/) | +| AppSource | [AppSource](../microsoft/knowledge/appsource/) | +| Compatibility | [Breaking changes](../microsoft/knowledge/breaking-changes/) | +| Data modeling | [Data modeling](../microsoft/knowledge/data-modeling/) | +| Error handling | [Error handling](../microsoft/knowledge/error-handling/) | +| Events | [Events](../microsoft/knowledge/events/) | +| Interfaces | [Interfaces](../microsoft/knowledge/interfaces/) | +| Performance | [Performance](../microsoft/knowledge/performance/) | +| Privacy | [Privacy](../microsoft/knowledge/privacy/) | +| Query objects | [Query](../microsoft/knowledge/query/) | +| Security | [Security](../microsoft/knowledge/security/) | +| Style | [Style](../microsoft/knowledge/style/) | +| Telemetry | [Telemetry](../microsoft/knowledge/telemetry/) | +| Testing | [Testing](../microsoft/knowledge/testing/) | +| User interface | [UI](../microsoft/knowledge/ui/) | +| Upgrades | [Upgrade](../microsoft/knowledge/upgrade/) | +| APIs and web services | [Web services](../microsoft/knowledge/web-services/) | + +## Permissions and data + +The review instructions produce findings, not source edits or deployment. +The host still controls tool permissions: keep approval prompts enabled and +do not grant blanket write or deployment access just to run a review. +BCQuality may write its generated `knowledge-index.json` into its own installed +directory; that is separate from your app's source. + +BCQuality is content, not an AI service. Your chosen host and model determine +where source code is processed, what usage is billed, and which data policies +apply. Review those policies before supplying proprietary or customer code. +Installing the plugin does not make an online host run locally or offline. diff --git a/microsoft/knowledge/appsource/object-affixes-prevent-collisions.md b/microsoft/knowledge/appsource/object-affixes-prevent-collisions.md index a3542a1..a53e584 100644 --- a/microsoft/knowledge/appsource/object-affixes-prevent-collisions.md +++ b/microsoft/knowledge/appsource/object-affixes-prevent-collisions.md @@ -21,10 +21,10 @@ This rule scopes to Marketplace ISV extensions, which is what AppSourceCop valid Own objects use the registered affix (for example `ABC Loyalty Tier`) or, when targeting BC23 or later, a qualifying namespace. Every field or action added to a standard object remains individually affixed (for example `Loyalty Points ABC` on a `Customer` tableextension). -See sample: `object-affixes-prevent-collisions.good.al`. +See sample: [`object-affixes-prevent-collisions.good.al`](object-affixes-prevent-collisions.good.al). ## Anti Pattern An owned object with neither a qualifying namespace nor an affix, an unaffixed extension member, or the common half-measure where the extension object carries the affix but a field it adds to a standard table does not. AS0011 flags the missing collision protection and the field can still collide with another app. -See sample: `object-affixes-prevent-collisions.bad.al`. +See sample: [`object-affixes-prevent-collisions.bad.al`](object-affixes-prevent-collisions.bad.al). diff --git a/microsoft/knowledge/appsource/permission-sets-cover-setup-and-usage-without-super.md b/microsoft/knowledge/appsource/permission-sets-cover-setup-and-usage-without-super.md index ca89003..1b81fb4 100644 --- a/microsoft/knowledge/appsource/permission-sets-cover-setup-and-usage-without-super.md +++ b/microsoft/knowledge/appsource/permission-sets-cover-setup-and-usage-without-super.md @@ -17,10 +17,10 @@ An AppSource app must provide permission sets that let assigned users complete t Trace every setup page, normal page, report, codeunit, and tabledata operation exposed by the app and cover it through assignable role permission sets composed from focused non-assignable sets. Validate setup and representative workflows as a user assigned only those app roles. Grant the minimum required operations; completeness is not a reason to use wildcards. -See sample: `permission-sets-cover-setup-and-usage-without-super.good.al`. +See sample: [`permission-sets-cover-setup-and-usage-without-super.good.al`](permission-sets-cover-setup-and-usage-without-super.good.al). ## Anti Pattern Shipping no permission set, omitting a tabledata or execute grant used by the app's own UI, or instructing users and validators to assign `SUPER` when setup fails. Do not flag a permission-set name that differs from the app name; no such naming requirement exists. -See sample: `permission-sets-cover-setup-and-usage-without-super.bad.al`. +See sample: [`permission-sets-cover-setup-and-usage-without-super.bad.al`](permission-sets-cover-setup-and-usage-without-super.bad.al). diff --git a/microsoft/knowledge/appsource/two-level-namespace-replaces-object-affix-not-extension-member-affix.md b/microsoft/knowledge/appsource/two-level-namespace-replaces-object-affix-not-extension-member-affix.md index 8e46049..fef6d6c 100644 --- a/microsoft/knowledge/appsource/two-level-namespace-replaces-object-affix-not-extension-member-affix.md +++ b/microsoft/knowledge/appsource/two-level-namespace-replaces-object-affix-not-extension-member-affix.md @@ -19,10 +19,10 @@ The requirement comes from AppSourceCop rule AS0011, which only runs when the ap Choose one collision strategy for owned objects: a registered affix or a globally meaningful namespace with at least two levels. Regardless of that choice, apply the registered affix to every member added to a base or third-party object. Keep the affix configured for AppSourceCop so member validation remains deterministic. Do not raise a missing member affix against an app that does not enable AppSourceCop with a mandatory affix; there AS0011 never fires, and the app's namespace is not the reason โ€” the absent configuration is. -See sample: `two-level-namespace-replaces-object-affix-not-extension-member-affix.good.al`. +See sample: [`two-level-namespace-replaces-object-affix-not-extension-member-affix.good.al`](two-level-namespace-replaces-object-affix-not-extension-member-affix.good.al). ## Anti Pattern Using `namespace Contoso;` as though one level satisfied the AppSource alternative, or declaring `namespace Contoso.Rentals;` and then adding an unaffixed `Loyalty Points` field to `Customer` in an app that does configure a mandatory affix. The namespace distinguishes the extension's own objects; it cannot disambiguate members on Customer. The mirror-image mistake is reporting an unaffixed extension member in an app that enables no mandatory affix at all โ€” AS0011 does not apply there, and the finding is a false positive. -See sample: `two-level-namespace-replaces-object-affix-not-extension-member-affix.bad.al`. +See sample: [`two-level-namespace-replaces-object-affix-not-extension-member-affix.bad.al`](two-level-namespace-replaces-object-affix-not-extension-member-affix.bad.al). diff --git a/microsoft/knowledge/breaking-changes/choose-access-modifiers-deliberately.md b/microsoft/knowledge/breaking-changes/choose-access-modifiers-deliberately.md index 835312d..6786631 100644 --- a/microsoft/knowledge/breaking-changes/choose-access-modifiers-deliberately.md +++ b/microsoft/knowledge/breaking-changes/choose-access-modifiers-deliberately.md @@ -17,10 +17,10 @@ Access is a decision about what you are willing to support forever. The moment a Start everything `local` or `internal` and promote a member to `public` only when you have decided to support it as a stable contract. Expose a small, intentional surface โ€” the supported entry point โ€” and keep validation, posting, and helper routines `internal` for in-app reuse or `local` when single-object. Do not drop `[Scope('OnPrem')]` without intent, since that too widens the contract. Every public member is a maintenance commitment; spend them deliberately. -See sample: `choose-access-modifiers-deliberately.good.al`. +See sample: [`choose-access-modifiers-deliberately.good.al`](choose-access-modifiers-deliberately.good.al). ## Anti Pattern Declaring every procedure `public` by default, so internal helpers like `ValidateOrder` and `PostOrder` become a de-facto API that consumers bind to and that can no longer be changed freely. Detection: an object where implementation-detail procedures carry no access modifier or are `public` without a reason to support them externally. Default them to `internal`/`local` and make only the intended entry point public. -See sample: `choose-access-modifiers-deliberately.bad.al`. +See sample: [`choose-access-modifiers-deliberately.bad.al`](choose-access-modifiers-deliberately.bad.al). diff --git a/microsoft/knowledge/breaking-changes/deprecate-public-members-with-the-obsolete-lifecycle.md b/microsoft/knowledge/breaking-changes/deprecate-public-members-with-the-obsolete-lifecycle.md index 35a342e..f7d05d5 100644 --- a/microsoft/knowledge/breaking-changes/deprecate-public-members-with-the-obsolete-lifecycle.md +++ b/microsoft/knowledge/breaking-changes/deprecate-public-members-with-the-obsolete-lifecycle.md @@ -17,10 +17,10 @@ Deleting or renaming a published procedure (or object) in a single release is a When a published procedure is superseded, keep it in place and mark it `[Obsolete('Use CalculateNetAmount instead.', '25.0')]`, where the message names the replacement and the tag records when the method became obsolete. Have the obsolete member forward to the new one so behavior is preserved during the window. Only after the deprecation window has elapsed should a later release delete the method. For an object or field, use `Pending` during the warning window and `Removed` afterward. -See sample: `deprecate-public-members-with-the-obsolete-lifecycle.good.al`. +See sample: [`deprecate-public-members-with-the-obsolete-lifecycle.good.al`](deprecate-public-members-with-the-obsolete-lifecycle.good.al). ## Anti Pattern Renaming or deleting the published `CalcNet` procedure in place โ€” replacing it with `CalculateNetAmount` and nothing else โ€” so consumers calling `CalcNet` break immediately with no deprecation notice. Detection: a previously shipped non-`local` procedure that vanished or was renamed between versions with no `[Obsolete]` marker left behind during a prior warning window. Do not suggest `ObsoleteState = Removed` for a method; that property belongs to supported object and element types. -See sample: `deprecate-public-members-with-the-obsolete-lifecycle.bad.al`. +See sample: [`deprecate-public-members-with-the-obsolete-lifecycle.bad.al`](deprecate-public-members-with-the-obsolete-lifecycle.bad.al). diff --git a/microsoft/knowledge/breaking-changes/do-not-change-published-procedure-signatures.md b/microsoft/knowledge/breaking-changes/do-not-change-published-procedure-signatures.md index 75fa6c1..5fa0ae5 100644 --- a/microsoft/knowledge/breaking-changes/do-not-change-published-procedure-signatures.md +++ b/microsoft/knowledge/breaking-changes/do-not-change-published-procedure-signatures.md @@ -19,10 +19,10 @@ This rule governs procedures that dependents *call*. An event publisher โ€” a pr Treat a published signature as frozen. When new behavior needs more inputs, add a new procedure or overload alongside the original โ€” for example a `CalculateDiscountWithRate(Amount; Rate)` next to the unchanged `CalculateDiscount(Amount)` โ€” and let the old one delegate to the new one. Existing callers keep compiling; new callers opt into the richer entry point. Naming an unnamed return value is the one in-place change that is always safe. -See sample: `do-not-change-published-procedure-signatures.good.al`. +See sample: [`do-not-change-published-procedure-signatures.good.al`](do-not-change-published-procedure-signatures.good.al). ## Anti Pattern Editing the existing public procedure's parameter list โ€” here, adding a `Rate` parameter to `CalculateDiscount` โ€” so every dependent extension that called the old form fails to compile. Detection: a parameter added, removed, reordered, retyped, or flipped to/from `var`, or a changed return type, on any non-`local` procedure that already shipped. Add a new overload instead. Exclude event publishers whose only change is an added parameter: subscribers bind by parameter name, not position, so that edit is additive and reporting it here is a false positive. -See sample: `do-not-change-published-procedure-signatures.bad.al`. +See sample: [`do-not-change-published-procedure-signatures.bad.al`](do-not-change-published-procedure-signatures.bad.al). diff --git a/microsoft/knowledge/breaking-changes/do-not-expose-sensitive-data-through-public-api.md b/microsoft/knowledge/breaking-changes/do-not-expose-sensitive-data-through-public-api.md index bd32d2d..cb3f772 100644 --- a/microsoft/knowledge/breaking-changes/do-not-expose-sensitive-data-through-public-api.md +++ b/microsoft/knowledge/breaking-changes/do-not-expose-sensitive-data-through-public-api.md @@ -17,10 +17,10 @@ Every member you make publicly reachable becomes a contract you must keep โ€” an Keep secrets in `internal` or `local` members, and prefer the `SecretText` type so the value cannot be read back or logged. Where callers genuinely need a credential, pass it inward (a setter) rather than handing it outward (a getter). Public API should return only non-sensitive data โ€” a masked reference, a status, a business identifier โ€” never the raw secret. Treat each public member as a lasting commitment and keep the security-sensitive surface as small as possible. -See sample: `do-not-expose-sensitive-data-through-public-api.good.al`. +See sample: [`do-not-expose-sensitive-data-through-public-api.good.al`](do-not-expose-sensitive-data-through-public-api.good.al). ## Anti Pattern A public `GetAccessToken()` that returns the raw token (or an event parameter carrying a credential to all subscribers), turning a secret into a de-facto public API any dependent can consume. Detection: a non-`local` procedure, event parameter, or global variable that surfaces a token, password, key, or other credential. Keep the secret internal and expose only non-sensitive data. -See sample: `do-not-expose-sensitive-data-through-public-api.bad.al`. +See sample: [`do-not-expose-sensitive-data-through-public-api.bad.al`](do-not-expose-sensitive-data-through-public-api.bad.al). diff --git a/microsoft/knowledge/breaking-changes/do-not-modify-code-already-marked-obsolete.md b/microsoft/knowledge/breaking-changes/do-not-modify-code-already-marked-obsolete.md index 781810b..4609ae3 100644 --- a/microsoft/knowledge/breaking-changes/do-not-modify-code-already-marked-obsolete.md +++ b/microsoft/knowledge/breaking-changes/do-not-modify-code-already-marked-obsolete.md @@ -17,10 +17,10 @@ A member carrying `[Obsolete]`, or wrapped in a `#if not CLEANxx` conditional-co Leave obsolete members exactly as they are and implement against the current, supported replacement. New logic โ€” a surcharge calculation, an event publisher, a hook โ€” belongs on the live API (`GetUnitPrice`), never inside the deprecated `GetPrice` or behind a `#if not CLEAN25` guard. If the replacement does not yet exist, create it as a first-class member and build there. The obsolete code should only shrink over time, not accrete new behavior. -See sample: `do-not-modify-code-already-marked-obsolete.good.al`. +See sample: [`do-not-modify-code-already-marked-obsolete.good.al`](do-not-modify-code-already-marked-obsolete.good.al). ## Anti Pattern Adding a surcharge calculation inside the `[Obsolete]` `GetPrice` procedure, or behind a `#if not CLEAN25` block, so the new behavior is wired to code that will be removed when `CLEAN25` is enabled. Detection: new statements, event declarations, or dependencies introduced inside an `[Obsolete]`-marked member or a `#if not CLEANxx` region. Move the logic onto the supported replacement instead. -See sample: `do-not-modify-code-already-marked-obsolete.bad.al`. +See sample: [`do-not-modify-code-already-marked-obsolete.bad.al`](do-not-modify-code-already-marked-obsolete.bad.al). diff --git a/microsoft/knowledge/breaking-changes/namespace-is-part-of-published-object-identity.md b/microsoft/knowledge/breaking-changes/namespace-is-part-of-published-object-identity.md index fde0f54..6c345ee 100644 --- a/microsoft/knowledge/breaking-changes/namespace-is-part-of-published-object-identity.md +++ b/microsoft/knowledge/breaking-changes/namespace-is-part-of-published-object-identity.md @@ -17,10 +17,10 @@ AL resolves an object by namespace and name. Once an app ships and dependent ext Choose a globally meaningful namespace before first publication and keep it stable. Add new functional areas beneath that structure without moving existing published objects. If an identity must move, use the platform's supported move/obsoletion lifecycle rather than a source-only namespace rename. -See sample: `namespace-is-part-of-published-object-identity.good.al`. +See sample: [`namespace-is-part-of-published-object-identity.good.al`](namespace-is-part-of-published-object-identity.good.al). ## Anti Pattern Changing `namespace Contoso.Rentals;` to `namespace Contoso.RentalManagement;` as a cleanup while leaving the object name and ID untouched. Every dependent `using` directive and qualified reference targets the old identity and stops compiling. -See sample: `namespace-is-part-of-published-object-identity.bad.al`. +See sample: [`namespace-is-part-of-published-object-identity.bad.al`](namespace-is-part-of-published-object-identity.bad.al). diff --git a/microsoft/knowledge/breaking-changes/obsolete-table-fields-instead-of-deleting-them.md b/microsoft/knowledge/breaking-changes/obsolete-table-fields-instead-of-deleting-them.md index 3ff6474..bf62fac 100644 --- a/microsoft/knowledge/breaking-changes/obsolete-table-fields-instead-of-deleting-them.md +++ b/microsoft/knowledge/breaking-changes/obsolete-table-fields-instead-of-deleting-them.md @@ -17,10 +17,10 @@ A shipped table field carries both a source-level contract and persisted data. R Keep the old field's ID, name, and type unchanged. Add the replacement as a separate field under an unused ID, then mark the old field `ObsoleteState = Pending` with an `ObsoleteReason` that names the replacement and an `ObsoleteTag` recording the obsoletion version. Keep the old field readable so an upgrade codeunit can copy its data during the deprecation window. Move it to `ObsoleteState = Removed` only in a later release, after the window has passed and data has migrated. -See sample: `obsolete-table-fields-instead-of-deleting-them.good.al`. +See sample: [`obsolete-table-fields-instead-of-deleting-them.good.al`](obsolete-table-fields-instead-of-deleting-them.good.al). ## Anti Pattern Renaming published `Email` to `Contact Email` with the same ID violates the compatibility contract and AS0005, even though the retained ID does not itself imply a fresh empty column. Deleting `Email` or changing its ID additionally risks losing its stored values. Detection: any previously shipped field whose name changes at the same ID, or whose original ID disappears without the unchanged field being retained as `Pending` and its data migrated to a separate replacement field. -See sample: `obsolete-table-fields-instead-of-deleting-them.bad.al`. +See sample: [`obsolete-table-fields-instead-of-deleting-them.bad.al`](obsolete-table-fields-instead-of-deleting-them.bad.al). diff --git a/microsoft/knowledge/data-modeling/check-blocked-in-referencing-code-not-in-master.md b/microsoft/knowledge/data-modeling/check-blocked-in-referencing-code-not-in-master.md index e324d45..698980f 100644 --- a/microsoft/knowledge/data-modeling/check-blocked-in-referencing-code-not-in-master.md +++ b/microsoft/knowledge/data-modeling/check-blocked-in-referencing-code-not-in-master.md @@ -19,10 +19,10 @@ Putting the block check inside the master's own `OnInsert`/`OnModify` does nothi The referencing line validates `Master.TestField(Blocked, false)` in `OnValidate` of the reference field and re-checks before posting. The master table stays logic-free on `Blocked`. -See sample: `check-blocked-in-referencing-code-not-in-master.good.al`. +See sample: [`check-blocked-in-referencing-code-not-in-master.good.al`](check-blocked-in-referencing-code-not-in-master.good.al). ## Anti Pattern The block check sits in the master's own `OnModify`/`OnInsert` (so referencing and posting proceed unchecked), or there is no check at all on the referencing side. -See sample: `check-blocked-in-referencing-code-not-in-master.bad.al`. +See sample: [`check-blocked-in-referencing-code-not-in-master.bad.al`](check-blocked-in-referencing-code-not-in-master.bad.al). diff --git a/microsoft/knowledge/data-modeling/master-table-no-from-number-series-in-oninsert.md b/microsoft/knowledge/data-modeling/master-table-no-from-number-series-in-oninsert.md index f4c6a15..69476ff 100644 --- a/microsoft/knowledge/data-modeling/master-table-no-from-number-series-in-oninsert.md +++ b/microsoft/knowledge/data-modeling/master-table-no-from-number-series-in-oninsert.md @@ -19,10 +19,10 @@ This is not an `Integer` `AutoIncrement` key, a GUID, or the `SystemId`. Those a `No.` `Code[20]` is the sole primary key; a non-editable `No. Series` `Code[20]` field records the source series. `OnInsert` checks `if "No." = ''`, reads the setup table, `TestField`s the configured series, stores it in `No. Series`, and assigns `No.` from the series. -See sample: `master-table-no-from-number-series-in-oninsert.good.al`. +See sample: [`master-table-no-from-number-series-in-oninsert.good.al`](master-table-no-from-number-series-in-oninsert.good.al). ## Anti Pattern An `Integer` `AutoIncrement` (or GUID / `SystemId`) primary key used as the business key, with no `OnInsert` number assignment. Records get an opaque identifier no user can reference, and the master no longer participates in the standard numbering and manual-entry behavior every other BC master follows. -See sample: `master-table-no-from-number-series-in-oninsert.bad.al`. +See sample: [`master-table-no-from-number-series-in-oninsert.bad.al`](master-table-no-from-number-series-in-oninsert.bad.al). diff --git a/microsoft/knowledge/data-modeling/owning-table-must-delete-dependents-in-ondelete.md b/microsoft/knowledge/data-modeling/owning-table-must-delete-dependents-in-ondelete.md index 82bf81d..5ded169 100644 --- a/microsoft/knowledge/data-modeling/owning-table-must-delete-dependents-in-ondelete.md +++ b/microsoft/knowledge/data-modeling/owning-table-must-delete-dependents-in-ondelete.md @@ -25,7 +25,7 @@ See also `validate-table-relation-false-suppresses-rename-propagation.md` for th The owning table implements `OnDelete` and deletes its dependents there, filtered on the foreign key. Declare `Permissions = tabledata = rd` on the owning table โ€” granting delete rights only on the parent is a common miss that makes the trigger fail for a non-`SUPER` user. This mirrors the base application, where every header table deletes its own lines. -See sample: `owning-table-must-delete-dependents-in-ondelete.good.al`. +See sample: [`owning-table-must-delete-dependents-in-ondelete.good.al`](owning-table-must-delete-dependents-in-ondelete.good.al). ## Anti Pattern @@ -33,4 +33,4 @@ A parent table with dependent rows and no `OnDelete` trigger, where the dependen Detection signal: a table declares `TableRelation` to table X, and table X has no `OnDelete` trigger. Whether a delete path currently exists in the UI is irrelevant to the finding. -See sample: `owning-table-must-delete-dependents-in-ondelete.bad.al`. +See sample: [`owning-table-must-delete-dependents-in-ondelete.bad.al`](owning-table-must-delete-dependents-in-ondelete.bad.al). diff --git a/microsoft/knowledge/data-modeling/set-last-date-modified-in-onmodify-and-onrename.md b/microsoft/knowledge/data-modeling/set-last-date-modified-in-onmodify-and-onrename.md index dbc0a64..f8b2a0d 100644 --- a/microsoft/knowledge/data-modeling/set-last-date-modified-in-onmodify-and-onrename.md +++ b/microsoft/knowledge/data-modeling/set-last-date-modified-in-onmodify-and-onrename.md @@ -19,10 +19,10 @@ The reason is a BC-specific trap: renaming a record changes its primary key and Both `OnModify` and `OnRename` set `"Last Date Modified" := Today();`, and the field is declared `Editable = false` so only the triggers maintain it. -See sample: `set-last-date-modified-in-onmodify-and-onrename.good.al`. +See sample: [`set-last-date-modified-in-onmodify-and-onrename.good.al`](set-last-date-modified-in-onmodify-and-onrename.good.al). ## Anti Pattern Only `OnModify` assigns `Last Date Modified`. After a rename the value is stale, and any process that trusts it to detect changes misses the record. -See sample: `set-last-date-modified-in-onmodify-and-onrename.bad.al`. +See sample: [`set-last-date-modified-in-onmodify-and-onrename.bad.al`](set-last-date-modified-in-onmodify-and-onrename.bad.al). diff --git a/microsoft/knowledge/data-modeling/setup-table-is-a-singleton.md b/microsoft/knowledge/data-modeling/setup-table-is-a-singleton.md index 774963f..0a7ac8b 100644 --- a/microsoft/knowledge/data-modeling/setup-table-is-a-singleton.md +++ b/microsoft/knowledge/data-modeling/setup-table-is-a-singleton.md @@ -19,10 +19,10 @@ The setup **card** page enforces the singleton: `InsertAllowed = false` and `Del `Primary Key` `Code[10]` is the sole key; the setup is surfaced through a Card page with `InsertAllowed = false`, `DeleteAllowed = false`, and an open-time guard that inserts the blank row if it is missing. -See sample: `setup-table-is-a-singleton.good.al`. +See sample: [`setup-table-is-a-singleton.good.al`](setup-table-is-a-singleton.good.al). ## Anti Pattern An `Integer` / `AutoIncrement` key, a page that allows insert or delete, or a List page over the setup table. Any of these lets the table hold zero or many rows, so "the setup" becomes ambiguous and `Get()` may fail or read the wrong record. -See sample: `setup-table-is-a-singleton.bad.al`. +See sample: [`setup-table-is-a-singleton.bad.al`](setup-table-is-a-singleton.bad.al). diff --git a/microsoft/knowledge/data-modeling/share-mediaset-items-with-insert-not-field-assignment.md b/microsoft/knowledge/data-modeling/share-mediaset-items-with-insert-not-field-assignment.md index 10946a5..0921227 100644 --- a/microsoft/knowledge/data-modeling/share-mediaset-items-with-insert-not-field-assignment.md +++ b/microsoft/knowledge/data-modeling/share-mediaset-items-with-insert-not-field-assignment.md @@ -17,10 +17,10 @@ application-area: [all] When sharing media between different tables, iterate the source `MediaSet` and call `Target.MediaSetField.Insert(Source.MediaSetField.Item(Index))`, then modify the target record. Direct field assignment is safe only when source and target are the same record subtype and use the same field ID. This concern is about reference/delete integrity, not the separate performance cost of `ModifyAll` on tables with media fields. -See sample: `share-mediaset-items-with-insert-not-field-assignment.good.al`. +See sample: [`share-mediaset-items-with-insert-not-field-assignment.good.al`](share-mediaset-items-with-insert-not-field-assignment.good.al). ## Anti Pattern `Target.Picture := Source.Picture;` where the two variables refer to different table types or different media-field IDs. The code copies an opaque ID, but the platform does not know that two independent fields now share the media object. -See sample: `share-mediaset-items-with-insert-not-field-assignment.bad.al`. +See sample: [`share-mediaset-items-with-insert-not-field-assignment.bad.al`](share-mediaset-items-with-insert-not-field-assignment.bad.al). diff --git a/microsoft/knowledge/data-modeling/table-relation-extensions-are-additive-and-top-down.md b/microsoft/knowledge/data-modeling/table-relation-extensions-are-additive-and-top-down.md index 1908f82..84bdc0b 100644 --- a/microsoft/knowledge/data-modeling/table-relation-extensions-are-additive-and-top-down.md +++ b/microsoft/knowledge/data-modeling/table-relation-extensions-are-additive-and-top-down.md @@ -17,10 +17,10 @@ A `tableextension` can add to an existing `TableRelation`, but the combined rela When a relation is designed to follow an extensible enum, express the base cases as conditional branches and leave no unconditional catch-all ahead of future extension branches. An enum extension can then append a condition for its new value. When extending a field you do not own, inspect the original `TableRelation`; do not claim that an appended condition overrides an unconditional relation. -See sample: `table-relation-extensions-are-additive-and-top-down.good.al`. +See sample: [`table-relation-extensions-are-additive-and-top-down.good.al`](table-relation-extensions-are-additive-and-top-down.good.al). ## Anti Pattern A base field has an unconditional `TableRelation = Customer;` and a `tableextension` adds `if (Type = const(Resource)) Resource`. The original unconditional branch always wins, so the new enum value still validates and looks up against Customer. The concern is evaluation order, not `ValidateTableRelation`; free-form input is covered separately by security guidance. -See sample: `table-relation-extensions-are-additive-and-top-down.bad.al`. +See sample: [`table-relation-extensions-are-additive-and-top-down.bad.al`](table-relation-extensions-are-additive-and-top-down.bad.al). diff --git a/microsoft/knowledge/data-modeling/transferfields-skip-type-mismatch-can-drop-data.md b/microsoft/knowledge/data-modeling/transferfields-skip-type-mismatch-can-drop-data.md index 7d1ea77..7f288b2 100644 --- a/microsoft/knowledge/data-modeling/transferfields-skip-type-mismatch-can-drop-data.md +++ b/microsoft/knowledge/data-modeling/transferfields-skip-type-mismatch-can-drop-data.md @@ -17,10 +17,10 @@ application-area: [all] Use `TransferFields(Source)` only when every field the destination requires, including primary key fields, is guaranteed to share a matching field number and type with the source; this form defaults `InitPrimaryKeyFields` to `true`. Fields with no matching field number, and fields whose types differ across extensions, are skipped regardless of `SkipFieldsNotMatchingType` โ€” that parameter only governs same-extension type mismatches. If the destination depends on a field that falls into either case, map and validate it explicitly in code rather than relying on `TransferFields` to catch the gap. Use `SkipFieldsNotMatchingType = true` only when skipping same-extension type mismatches is an intentional, documented part of the transfer contract. -See sample: `transferfields-skip-type-mismatch-can-drop-data.good.al`. +See sample: [`transferfields-skip-type-mismatch-can-drop-data.good.al`](transferfields-skip-type-mismatch-can-drop-data.good.al). ## Anti Pattern Using `TransferFields(Source, InitPrimaryKeyFields, true)` as a generic way to make two evolving table schemas transfer without errors, when the destination depends on every required source field being copied. A type change on either table can turn a previously transferred field into a silently skipped one without making the transfer itself fail. -See sample: `transferfields-skip-type-mismatch-can-drop-data.bad.al`. \ No newline at end of file +See sample: [`transferfields-skip-type-mismatch-can-drop-data.bad.al`](transferfields-skip-type-mismatch-can-drop-data.bad.al). \ No newline at end of file diff --git a/microsoft/knowledge/data-modeling/use-no-series-codeunit-not-noseriesmanagement.md b/microsoft/knowledge/data-modeling/use-no-series-codeunit-not-noseriesmanagement.md index b4d19b9..f80e35d 100644 --- a/microsoft/knowledge/data-modeling/use-no-series-codeunit-not-noseriesmanagement.md +++ b/microsoft/knowledge/data-modeling/use-no-series-codeunit-not-noseriesmanagement.md @@ -19,10 +19,10 @@ LLMs reproduce the legacy `NoSeriesManagement` pattern because it dominates pre- `OnInsert` assigns the number with `NoSeries.GetNextNo("No. Series")` where `NoSeries` is `Codeunit "No. Series"`. The `No.` field's `OnValidate` guards manual entry by calling `NoSeries.IsManual(...)` (or `TestManual`) before clearing `No. Series`. -See sample: `use-no-series-codeunit-not-noseriesmanagement.good.al`. +See sample: [`use-no-series-codeunit-not-noseriesmanagement.good.al`](use-no-series-codeunit-not-noseriesmanagement.good.al). ## Anti Pattern `NoSeriesMgt.InitSeries(...)` for assignment and `NoSeriesMgt.TestManual(...)` for the manual check, where `NoSeriesMgt` is `Codeunit NoSeriesManagement`. Both are obsolete-pending and emit compiler warnings. -See sample: `use-no-series-codeunit-not-noseriesmanagement.bad.al`. +See sample: [`use-no-series-codeunit-not-noseriesmanagement.bad.al`](use-no-series-codeunit-not-noseriesmanagement.bad.al). diff --git a/microsoft/knowledge/data-modeling/validate-table-relation-false-suppresses-rename-propagation.md b/microsoft/knowledge/data-modeling/validate-table-relation-false-suppresses-rename-propagation.md index ddd4856..0131edb 100644 --- a/microsoft/knowledge/data-modeling/validate-table-relation-false-suppresses-rename-propagation.md +++ b/microsoft/knowledge/data-modeling/validate-table-relation-false-suppresses-rename-propagation.md @@ -27,7 +27,7 @@ See also `owning-table-must-delete-dependents-in-ondelete.md` for the delete hal Leave `ValidateTableRelation` at its default wherever the stored value must stay correct across a rename. When it must be disabled, or when the relationship cannot be expressed as a `TableRelation` at all, the table owning the referenced key carries an explicit `OnRename` that repoints the dependents itself. -See sample: `validate-table-relation-false-suppresses-rename-propagation.good.al`. +See sample: [`validate-table-relation-false-suppresses-rename-propagation.good.al`](validate-table-relation-false-suppresses-rename-propagation.good.al). ## Anti Pattern @@ -35,4 +35,4 @@ See sample: `validate-table-relation-false-suppresses-rename-propagation.good.al Detection signal: any `ValidateTableRelation = false` on a field that also declares a `TableRelation`. Ask what repoints the value when the target is renamed; if the answer is "the platform", the finding stands. -See sample: `validate-table-relation-false-suppresses-rename-propagation.bad.al`. +See sample: [`validate-table-relation-false-suppresses-rename-propagation.bad.al`](validate-table-relation-false-suppresses-rename-propagation.bad.al). diff --git a/microsoft/knowledge/data-modeling/xrec-is-a-before-image-only-in-some-triggers.md b/microsoft/knowledge/data-modeling/xrec-is-a-before-image-only-in-some-triggers.md index 52c06bd..8c90095 100644 --- a/microsoft/knowledge/data-modeling/xrec-is-a-before-image-only-in-some-triggers.md +++ b/microsoft/knowledge/data-modeling/xrec-is-a-before-image-only-in-some-triggers.md @@ -25,7 +25,7 @@ See also `validate-table-relation-false-suppresses-rename-propagation.md`, which Use `xRec` for the previous key in `OnRename`, and for the record being removed in `OnDelete`. In `OnModify`, obtain the before-image by re-reading the stored row rather than trusting `xRec`, so the logic behaves identically whether a page, a job queue or an API drove the write. -See sample: `xrec-is-a-before-image-only-in-some-triggers.good.al`. +See sample: [`xrec-is-a-before-image-only-in-some-triggers.good.al`](xrec-is-a-before-image-only-in-some-triggers.good.al). ## Anti Pattern @@ -33,4 +33,4 @@ Comparing `Rec` against `xRec` inside `OnModify` (or `OnInsert`) to detect a cha Detection signal: any read of `xRec` inside `OnModify` or `OnInsert`. Treat "but it works when I test it on the page" as confirmation of the defect rather than a refutation. -See sample: `xrec-is-a-before-image-only-in-some-triggers.bad.al`. +See sample: [`xrec-is-a-before-image-only-in-some-triggers.bad.al`](xrec-is-a-before-image-only-in-some-triggers.bad.al). diff --git a/microsoft/knowledge/error-handling/collect-validation-errors-with-errorbehavior.md b/microsoft/knowledge/error-handling/collect-validation-errors-with-errorbehavior.md index b464fec..cdb87b6 100644 --- a/microsoft/knowledge/error-handling/collect-validation-errors-with-errorbehavior.md +++ b/microsoft/knowledge/error-handling/collect-validation-errors-with-errorbehavior.md @@ -17,10 +17,10 @@ By default a procedure stops on the first `Error`, so a user fixing ten bad rows Mark the orchestrating procedure `[ErrorBehavior(ErrorBehavior::Collect)]` and run each item's validation so one failure doesn't abandon the rest โ€” typically by calling the per-item routine through `Codeunit.Run`. When the run finishes, inspect `HasCollectedErrors()`, retrieve and clear the list with `GetCollectedErrors(true)`, and fail the operation with the collected messages. The sample intentionally produces a text aggregate and does not claim to retain record/field metadata in the final error. If that metadata is needed, map each `ErrorInfo` to a custom error UI before clearing, following the Microsoft Learn pattern. Do not replace validation failure with `Message`: clearing collected errors suppresses the platform failure, so the custom handler must still block the invalid operation. -See sample: `collect-validation-errors-with-errorbehavior.good.al`. +See sample: [`collect-validation-errors-with-errorbehavior.good.al`](collect-validation-errors-with-errorbehavior.good.al). ## Anti Pattern Three shapes signal trouble. Hand-rolled accumulation reimplements collection and prevents the handler from receiving individual `ErrorInfo` values. A `Collect` procedure that never handles the collection falls back to the concatenated platform dialog. Finally, code that calls parameterless `GetCollectedErrors()`, assumes it cleared the list, and only shows a `Message` can both leave the errors collected and allow invalid processing to continue. -See sample: `collect-validation-errors-with-errorbehavior.bad.al`. +See sample: [`collect-validation-errors-with-errorbehavior.bad.al`](collect-validation-errors-with-errorbehavior.bad.al). diff --git a/microsoft/knowledge/error-handling/errortype-internal-vs-client-for-diagnostics.md b/microsoft/knowledge/error-handling/errortype-internal-vs-client-for-diagnostics.md index 127fa50..ece3baf 100644 --- a/microsoft/knowledge/error-handling/errortype-internal-vs-client-for-diagnostics.md +++ b/microsoft/knowledge/error-handling/errortype-internal-vs-client-for-diagnostics.md @@ -17,10 +17,10 @@ application-area: [all] Reserve `ErrorType::Internal` for errors the user cannot act on: corrupted internal state, an unreachable branch, a contract a caller violated. Set a precise, detail-rich `Message` for telemetry, raise it via `Error(ErrorInfo)`, and let the platform show the user a generic dialog. Keep `ErrorType::Client` (or a plain `Error`) for failures the user is expected to read and resolve โ€” validation messages, missing setup, business-rule violations. The test is simple: if the message only makes sense to a developer, mark it `Internal`. -See sample: `errortype-internal-vs-client-for-diagnostics.good.al`. +See sample: [`errortype-internal-vs-client-for-diagnostics.good.al`](errortype-internal-vs-client-for-diagnostics.good.al). ## Anti Pattern Raising an internal failure with a plain `Error('Unexpected state: ledger bucket %1 not initialized', BucketId)`. The user is shown a technical message they can do nothing about, and the signal is buried in a generic error rather than carried as structured telemetry detail. Detection: an `Error` whose wording targets a developer ("unexpected", "should not happen", raw internal identifiers) raised with default `Client` visibility instead of an `ErrorInfo` marked `ErrorType::Internal`. -See sample: `errortype-internal-vs-client-for-diagnostics.bad.al`. +See sample: [`errortype-internal-vs-client-for-diagnostics.bad.al`](errortype-internal-vs-client-for-diagnostics.bad.al). diff --git a/microsoft/knowledge/error-handling/fielderror-default-message-logic.md b/microsoft/knowledge/error-handling/fielderror-default-message-logic.md index c02bb4f..51c116a 100644 --- a/microsoft/knowledge/error-handling/fielderror-default-message-logic.md +++ b/microsoft/knowledge/error-handling/fielderror-default-message-logic.md @@ -14,9 +14,9 @@ application-area: [all] ## Best Practice For a plain required-field check, prefer `TestField`, which tests the condition and raises the error in one call. When the condition is non-trivial and has already been evaluated, call `FieldError(FieldNo)` with no message to get the localized default (`must have a value`, `is not valid`, etc.), or pass a short lowercase predicate such as `FieldError(FieldNo, 'must be a positive number')`. Start the custom text with a lowercase letter so it reads as one sentence with the auto-inserted caption, and use a field-number reference (or the field token) rather than a hard-coded field name so captions and translations stay correct. Let the framework supply the caption, value, table, and key context for you. -See sample: `fielderror-default-message-logic.good.al`. +See sample: [`fielderror-default-message-logic.good.al`](fielderror-default-message-logic.good.al). ## Anti Pattern Re-testing a condition you already evaluated, or passing a fully formed sentence like `'The Amount field must be positive.'` to `FieldError`. The result reads as `Amount The Amount field must be positive. in Gen. Journal Line ...` โ€” capital letter mid-sentence, caption and value repeated, and a stray trailing clause. Reviewer signals: a `FieldError` argument that names the field, restates the current value, starts with a capital letter, or ends with a period. Each is a sign the author treated `FieldError` like `Error` instead of as a predicate slotted into framework-generated context. -See sample: `fielderror-default-message-logic.bad.al`. \ No newline at end of file +See sample: [`fielderror-default-message-logic.bad.al`](fielderror-default-message-logic.bad.al). \ No newline at end of file diff --git a/microsoft/knowledge/error-handling/fielderror-vs-testfield.md b/microsoft/knowledge/error-handling/fielderror-vs-testfield.md index 9b58b32..845a0db 100644 --- a/microsoft/knowledge/error-handling/fielderror-vs-testfield.md +++ b/microsoft/knowledge/error-handling/fielderror-vs-testfield.md @@ -16,9 +16,9 @@ Use `TestField` when the condition is a simple presence-or-equality check on a s A page action's `OnAction` trigger is a different case: a page action is only invocable through its own UI control, so when the action's `Enabled` property is already bound to the same condition the trigger would otherwise `TestField`, the control cannot be clicked while the field is blank and the field can never reach the trigger empty. Adding a `TestField` there is redundant defensive code, not a missing check โ€” flag it only when the trigger can run through a path `Enabled` does not cover (a shared procedure, an API, or a condition broader than what gates the action). -See sample: `fielderror-vs-testfield.good.al`. +See sample: [`fielderror-vs-testfield.good.al`](fielderror-vs-testfield.good.al). ## Anti Pattern Calling `FieldError` to "test" a field โ€” placing it on a path that is reached unconditionally and expecting it to validate โ€” terminates execution every time because `FieldError` never evaluates a condition. The inverse smell is reaching for `TestField` when the rule needs a tailored message, then bolting a vague generic string onto a check that cannot express the real business reason. A reviewer can spot the first by a `FieldError` that is not guarded by a preceding `if`, and the second by a `TestField` whose intent comment describes a condition more complex than presence or equality. -See sample: `fielderror-vs-testfield.bad.al`. +See sample: [`fielderror-vs-testfield.bad.al`](fielderror-vs-testfield.bad.al). diff --git a/microsoft/knowledge/error-handling/ignored-tryfunction-return-disables-try-semantics.md b/microsoft/knowledge/error-handling/ignored-tryfunction-return-disables-try-semantics.md index 52e3e40..902528b 100644 --- a/microsoft/knowledge/error-handling/ignored-tryfunction-return-disables-try-semantics.md +++ b/microsoft/knowledge/error-handling/ignored-tryfunction-return-disables-try-semantics.md @@ -17,13 +17,13 @@ A procedure marked `[TryFunction]` catches errors only when the caller uses its Consume the result directly: assign it to a Boolean or use the call in an `if` condition. Handle `false` immediately while the last-error state still describes that failure. -See sample: `ignored-tryfunction-return-disables-try-semantics.good.al`. +See sample: [`ignored-tryfunction-return-disables-try-semantics.good.al`](ignored-tryfunction-return-disables-try-semantics.good.al). ## Anti Pattern Calling a `[TryFunction]` procedure as a standalone statement and assuming the attribute suppresses its errors. The call has ordinary error semantics because its Boolean result is ignored. -See sample: `ignored-tryfunction-return-disables-try-semantics.bad.al`. +See sample: [`ignored-tryfunction-return-disables-try-semantics.bad.al`](ignored-tryfunction-return-disables-try-semantics.bad.al). ## See also diff --git a/microsoft/knowledge/error-handling/prefer-errorinfo-for-actionable-errors.md b/microsoft/knowledge/error-handling/prefer-errorinfo-for-actionable-errors.md index f774cc8..082e118 100644 --- a/microsoft/knowledge/error-handling/prefer-errorinfo-for-actionable-errors.md +++ b/microsoft/knowledge/error-handling/prefer-errorinfo-for-actionable-errors.md @@ -17,10 +17,10 @@ A plain `Error('text')` ends the operation with a dead-end dialog: the user read Build an `ErrorInfo`, set `Title`, `Message`, and `DetailedMessage`, then attach the action that matches the situation. For a Fix-it, call `AddAction(Caption, Codeunit::Handler, 'MethodName')` where the handler method (which receives the `ErrorInfo`) applies the known-good value; phrase the caption as "Set value to โ€ฆ". For a Show-it, set `PageNo := Page::"โ€ฆ"`, set `RecordId` so navigation opens the right record, and call `AddNavigationAction('Show โ€ฆ')`. Raise it with `Error(ErrorInfo)`. Reserve recommended actions for cases where the solution is genuinely known and the user has permission to apply it. -See sample: `prefer-errorinfo-for-actionable-errors.good.al`. +See sample: [`prefer-errorinfo-for-actionable-errors.good.al`](prefer-errorinfo-for-actionable-errors.good.al). ## Anti Pattern Surfacing a recoverable validation failure with `Error('You cannot invoice more than %1 units.', MaxQty)` and nothing else. The user is blocked with no offered remedy even though the code knows the maximum and could set it. The detection signal: an `Error` call in a validation or posting path whose message names a specific correct value or a specific related page, with no surrounding `ErrorInfo`, `AddAction`, or `AddNavigationAction`. Replace it with an `ErrorInfo` that carries the corresponding Fix-it or Show-it action. -See sample: `prefer-errorinfo-for-actionable-errors.bad.al`. +See sample: [`prefer-errorinfo-for-actionable-errors.bad.al`](prefer-errorinfo-for-actionable-errors.bad.al). diff --git a/microsoft/knowledge/events/add-new-event-parameters-at-the-end.md b/microsoft/knowledge/events/add-new-event-parameters-at-the-end.md index b05b020..418a50f 100644 --- a/microsoft/knowledge/events/add-new-event-parameters-at-the-end.md +++ b/microsoft/knowledge/events/add-new-event-parameters-at-the-end.md @@ -17,10 +17,10 @@ Event subscribers bind publisher parameters by name and can omit parameters they Add a parameter directly only when the shipped event publisher is `local` or `internal`. Place it where the signature is clearest; existing subscribers continue binding the parameters they name. For a public event, keep the original publisher unchanged and introduce a new event with the expanded contract. -See sample: `add-new-event-parameters-at-the-end.good.al`. +See sample: [`add-new-event-parameters-at-the-end.good.al`](add-new-event-parameters-at-the-end.good.al). ## Anti Pattern Appending a parameter to a public event and assuming its position makes the change compatible. Existing external callers still lack the new required argument. Conversely, do not flag a parameter inserted among existing parameters on a `local` or `internal` Business or Integration event merely because it was not appended. -See sample: `add-new-event-parameters-at-the-end.bad.al`. +See sample: [`add-new-event-parameters-at-the-end.bad.al`](add-new-event-parameters-at-the-end.bad.al). diff --git a/microsoft/knowledge/events/avoid-loosely-typed-event-parameters.md b/microsoft/knowledge/events/avoid-loosely-typed-event-parameters.md index 6c1e004..894b1ad 100644 --- a/microsoft/knowledge/events/avoid-loosely-typed-event-parameters.md +++ b/microsoft/knowledge/events/avoid-loosely-typed-event-parameters.md @@ -17,10 +17,10 @@ Passing `RecordRef` or `xRec` as event parameters weakens the contract. A `Recor Give events concrete record types and explicit values, such as `(SalesLine: Record "Sales Line"; PreviousQuantity: Decimal)`, instead of a `RecordRef` or an `xRec` parameter. Subscribers then get type safety, field access, and an unambiguous contract. -See sample: `avoid-loosely-typed-event-parameters.good.al`. +See sample: [`avoid-loosely-typed-event-parameters.good.al`](avoid-loosely-typed-event-parameters.good.al). ## Anti Pattern Event parameters typed as `RecordRef` (no table type) or an `xRec`-style "previous record" (ambiguous, possibly stale) without strong justification. Detection: an event signature containing a `RecordRef` parameter, or a passed-through `xRec` record, where a concrete typed record and explicit values would serve. -See sample: `avoid-loosely-typed-event-parameters.bad.al`. +See sample: [`avoid-loosely-typed-event-parameters.bad.al`](avoid-loosely-typed-event-parameters.bad.al). diff --git a/microsoft/knowledge/events/avoid-raising-events-inside-try-functions.md b/microsoft/knowledge/events/avoid-raising-events-inside-try-functions.md index 7e791fe..80b3b34 100644 --- a/microsoft/knowledge/events/avoid-raising-events-inside-try-functions.md +++ b/microsoft/knowledge/events/avoid-raising-events-inside-try-functions.md @@ -17,10 +17,10 @@ A `TryFunction` catches all errors โ€” including errors thrown by event subscrib Raise the integration event before entering the TryFunction scope. The event and its subscribers execute outside the error boundary, so subscriber errors propagate normally to the caller. Move only the operation that genuinely needs error isolation (such as an HTTP call or a posting step) inside the TryFunction. -See sample: `avoid-raising-events-inside-try-functions.good.al`. +See sample: [`avoid-raising-events-inside-try-functions.good.al`](avoid-raising-events-inside-try-functions.good.al). ## Anti Pattern Raising an integration event inside a TryFunction body. Subscriber failures are caught and discarded by the TryFunction. The subscriber contract โ€” that a subscriber can signal failure to the caller โ€” is silently broken. -See sample: `avoid-raising-events-inside-try-functions.bad.al`. +See sample: [`avoid-raising-events-inside-try-functions.bad.al`](avoid-raising-events-inside-try-functions.bad.al). diff --git a/microsoft/knowledge/events/choose-static-vs-manual-subscribers-deliberately.md b/microsoft/knowledge/events/choose-static-vs-manual-subscribers-deliberately.md index 9fe7312..68c0dc3 100644 --- a/microsoft/knowledge/events/choose-static-vs-manual-subscribers-deliberately.md +++ b/microsoft/knowledge/events/choose-static-vs-manual-subscribers-deliberately.md @@ -17,10 +17,10 @@ An `[EventSubscriber]` codeunit is static by default (`EventSubscriberInstance = Use a static subscriber for behaviour that genuinely applies all the time. For anything scoped, mark the codeunit `EventSubscriberInstance = Manual`, call `BindSubscription(SubscriberInstance)` at the start of the scope and `UnbindSubscription(SubscriberInstance)` at the end. A manual subscriber held only in a local variable unbinds automatically when that variable leaves scope, which suits test setup/teardown; a binding you intend to outlive a single call must be unbound explicitly. Keep subscriber methods `local` per CodeCop AA0207. -See sample: `choose-static-vs-manual-subscribers-deliberately.good.al`. +See sample: [`choose-static-vs-manual-subscribers-deliberately.good.al`](choose-static-vs-manual-subscribers-deliberately.good.al). ## Anti Pattern Two shapes. First, a static subscriber used for behaviour that should be scoped โ€” an always-on side effect (sending mail, writing extra records) that now fires for every event in every session and test with no way to disable it. Second, a manual subscriber that is bound with `BindSubscription` and never unbound: when the instance is held beyond the intended scope (for example on a `SingleInstance` codeunit), the binding leaks for the whole session and later unrelated operations keep hitting it. Detection: scoped side effects on a static subscriber, or a `BindSubscription` call with no matching `UnbindSubscription` and no scope that releases the instance. -See sample: `choose-static-vs-manual-subscribers-deliberately.bad.al`. +See sample: [`choose-static-vs-manual-subscribers-deliberately.bad.al`](choose-static-vs-manual-subscribers-deliberately.bad.al). diff --git a/microsoft/knowledge/events/declare-event-publishers-local-or-internal.md b/microsoft/knowledge/events/declare-event-publishers-local-or-internal.md index 9d097b4..ceca2ca 100644 --- a/microsoft/knowledge/events/declare-event-publishers-local-or-internal.md +++ b/microsoft/knowledge/events/declare-event-publishers-local-or-internal.md @@ -29,7 +29,7 @@ Give an event publisher the narrowest access modifier that still lets the code o Subscribers are unaffected by any of these choices. A non-public publisher also keeps the freedom to add a parameter later, which a public publisher gives up โ€” see `add-new-event-parameters-at-the-end`. -See sample: `declare-event-publishers-local-or-internal.good.al`. +See sample: [`declare-event-publishers-local-or-internal.good.al`](declare-event-publishers-local-or-internal.good.al). ## Anti Pattern @@ -39,4 +39,4 @@ Detection: an `[IntegrationEvent]` or `[BusinessEvent]` publisher that is public The mirror-image anti-pattern belongs to the reviewer, human or agent: recommending that a publisher be made public so extensions can subscribe, or reporting a `local`/`internal` publisher as unreachable dead code. Both readings mistake raising for subscribing. Neither should be raised as a finding. -See sample: `declare-event-publishers-local-or-internal.bad.al`. +See sample: [`declare-event-publishers-local-or-internal.bad.al`](declare-event-publishers-local-or-internal.bad.al). diff --git a/microsoft/knowledge/events/do-not-add-ishandled-to-an-existing-event.md b/microsoft/knowledge/events/do-not-add-ishandled-to-an-existing-event.md index bf563b4..df0c5e2 100644 --- a/microsoft/knowledge/events/do-not-add-ishandled-to-an-existing-event.md +++ b/microsoft/knowledge/events/do-not-add-ishandled-to-an-existing-event.md @@ -17,10 +17,10 @@ Adding a `var IsHandled: Boolean` parameter to an event that already shipped wit Keep the existing event as-is and add a separate `OnBeforeX(โ€ฆ; var IsHandled: Boolean)` before the logic you want to make overridable. Two events with distinct, stable contracts are safer than one event whose meaning and signature were changed under its subscribers. -See sample: `do-not-add-ishandled-to-an-existing-event.good.al`. +See sample: [`do-not-add-ishandled-to-an-existing-event.good.al`](do-not-add-ishandled-to-an-existing-event.good.al). ## Anti Pattern Mutating a shipped event โ€” for example adding `var IsHandled` to `OnAfterCalculateTotal` โ€” to retrofit override behaviour, which overloads the event's meaning and undermines existing subscribers. Detection: an `IsHandled` parameter added to a pre-existing event signature rather than introduced through a new dedicated `OnBefore` publisher. -See sample: `do-not-add-ishandled-to-an-existing-event.bad.al`. +See sample: [`do-not-add-ishandled-to-an-existing-event.bad.al`](do-not-add-ishandled-to-an-existing-event.bad.al). diff --git a/microsoft/knowledge/events/do-not-bypass-critical-operations-with-ishandled.md b/microsoft/knowledge/events/do-not-bypass-critical-operations-with-ishandled.md index e6941ce..7eff65f 100644 --- a/microsoft/knowledge/events/do-not-bypass-critical-operations-with-ishandled.md +++ b/microsoft/knowledge/events/do-not-bypass-critical-operations-with-ishandled.md @@ -17,10 +17,10 @@ The IsHandled override pattern lets a subscriber skip the guarded code entirely. Scope IsHandled to a safe value-calculation block and run the critical operations unconditionally afterwards; or expose a positive `OnAfterโ€ฆ` event for subscribers to adjust results, rather than a bypass around the commit. -See sample: `do-not-bypass-critical-operations-with-ishandled.good.al`. +See sample: [`do-not-bypass-critical-operations-with-ishandled.good.al`](do-not-bypass-critical-operations-with-ishandled.good.al). ## Anti Pattern An `OnBeforeโ€ฆ` IsHandled guard wrapping a posting or ledger routine โ€” `if IsHandled then exit;` around the code that creates ledger entries and updates document status โ€” letting subscribers skip the commit. Detection: an `if IsHandled then exit;` whose skipped body performs posting, ledger writes, number-series consumption, or integrity and permission validation. -See sample: `do-not-bypass-critical-operations-with-ishandled.bad.al`. +See sample: [`do-not-bypass-critical-operations-with-ishandled.bad.al`](do-not-bypass-critical-operations-with-ishandled.bad.al). diff --git a/microsoft/knowledge/events/do-not-change-shipped-event-attribute-flags.md b/microsoft/knowledge/events/do-not-change-shipped-event-attribute-flags.md index 98e0ca7..f613f3a 100644 --- a/microsoft/knowledge/events/do-not-change-shipped-event-attribute-flags.md +++ b/microsoft/knowledge/events/do-not-change-shipped-event-attribute-flags.md @@ -17,10 +17,10 @@ application-area: [all] Keep every available attribute argument exactly as shipped. If new subscribers need different sender/global exposure, publish a new event with the desired flags. Apply the same rule to `Isolated` only on BC20 or later, where that argument exists. Raise both events while the original contract is supported, and choose preferred flags only when designing a new event. -See sample: `do-not-change-shipped-event-attribute-flags.good.al`. +See sample: [`do-not-change-shipped-event-attribute-flags.good.al`](do-not-change-shipped-event-attribute-flags.good.al). ## Anti Pattern Changing a shipped event's `IncludeSender` or `GlobalVarAccess` to modernize its design, including replacing `IncludeSender` with an explicit parameter. On BC20 or later, adding, removing, or toggling `Isolated` is equally contract-significant. Even a change that leaves old subscribers compiling can alter observable execution or exposure; version the event instead. -See sample: `do-not-change-shipped-event-attribute-flags.bad.al`. +See sample: [`do-not-change-shipped-event-attribute-flags.bad.al`](do-not-change-shipped-event-attribute-flags.bad.al). diff --git a/microsoft/knowledge/events/do-not-publish-events-inside-loops.md b/microsoft/knowledge/events/do-not-publish-events-inside-loops.md index badbf28..ecebd19 100644 --- a/microsoft/knowledge/events/do-not-publish-events-inside-loops.md +++ b/microsoft/knowledge/events/do-not-publish-events-inside-loops.md @@ -17,10 +17,10 @@ Raising an event on every iteration of a loop multiplies the cost of every subsc Raise `OnBeforeProcessLines` before the loop and `OnAfterProcessLines` after it, outside the `repeat โ€ฆ until`, so each subscriber runs once per batch rather than once per row. Give those events the record or filters they need to operate on the whole set. -See sample: `do-not-publish-events-inside-loops.good.al`. +See sample: [`do-not-publish-events-inside-loops.good.al`](do-not-publish-events-inside-loops.good.al). ## Anti Pattern An event raised inside the loop body, fired once per iteration, so subscriber cost scales with the row count and large batches slow down or time out. Detection: an `OnBeforeโ€ฆ`/`OnAfterโ€ฆ`/`Onโ€ฆ` raise located between `repeat` and `until` in a record loop. -See sample: `do-not-publish-events-inside-loops.bad.al`. +See sample: [`do-not-publish-events-inside-loops.bad.al`](do-not-publish-events-inside-loops.bad.al). diff --git a/microsoft/knowledge/events/expose-process-context-via-manually-bound-flag.md b/microsoft/knowledge/events/expose-process-context-via-manually-bound-flag.md index a8e72be..802b7b0 100644 --- a/microsoft/knowledge/events/expose-process-context-via-manually-bound-flag.md +++ b/microsoft/knowledge/events/expose-process-context-via-manually-bound-flag.md @@ -25,7 +25,7 @@ Publish the context as a query and let the binding itself be the state. One proc Bind a fresh instance per run rather than reusing one: the platform refuses to bind the same instance twice but accepts several instances of the same codeunit, so nesting and re-entrancy need no counter. The binding is session-scoped, so work the process starts in another session โ€” a background session, a page background task, a job queue entry โ€” cannot see it; pass the context explicitly there. -See sample: `expose-process-context-via-manually-bound-flag.good.al`. +See sample: [`expose-process-context-via-manually-bound-flag.good.al`](expose-process-context-via-manually-bound-flag.good.al). ## Anti Pattern @@ -37,4 +37,4 @@ Second, the context kept private: the driving app arranges its own marker โ€” ty The mirror-image anti-pattern belongs to the reviewer: flagging the `BindSubscription` here as a leaked binding because no `UnbindSubscription` follows it. Scope release is the mechanism, not an omission โ€” see `microsoft/knowledge/events/choose-static-vs-manual-subscribers-deliberately.md`, whose leak case is an instance parked on a `SingleInstance` global that never leaves scope. -See sample: `expose-process-context-via-manually-bound-flag.bad.al`. +See sample: [`expose-process-context-via-manually-bound-flag.bad.al`](expose-process-context-via-manually-bound-flag.bad.al). diff --git a/microsoft/knowledge/events/name-event-parameters-without-abbreviations.md b/microsoft/knowledge/events/name-event-parameters-without-abbreviations.md index 539dd06..9985ae2 100644 --- a/microsoft/knowledge/events/name-event-parameters-without-abbreviations.md +++ b/microsoft/knowledge/events/name-event-parameters-without-abbreviations.md @@ -17,10 +17,10 @@ Event parameter names are part of the public contract a subscriber codes against Use full, unabbreviated names: `(SalesHeader: Record "Sales Header"; DocumentNo: Code[20]; Amount: Decimal)`. Record parameters mirror the table name without spaces, and value parameters read as whole words so the contract is unambiguous. -See sample: `name-event-parameters-without-abbreviations.good.al`. +See sample: [`name-event-parameters-without-abbreviations.good.al`](name-event-parameters-without-abbreviations.good.al). ## Anti Pattern Abbreviated parameter names (`SalesHdr`, `DocNo`, `Amt`) that obscure meaning and vary across publishers, so subscribers must guess what each one holds. Detection: event parameters whose names are truncated forms of the table name or contracted words rather than the full term. -See sample: `name-event-parameters-without-abbreviations.bad.al`. +See sample: [`name-event-parameters-without-abbreviations.bad.al`](name-event-parameters-without-abbreviations.bad.al). diff --git a/microsoft/knowledge/events/name-events-by-publisher-position.md b/microsoft/knowledge/events/name-events-by-publisher-position.md index cd8ac65..fc94098 100644 --- a/microsoft/knowledge/events/name-events-by-publisher-position.md +++ b/microsoft/knowledge/events/name-events-by-publisher-position.md @@ -17,10 +17,10 @@ An event name should tell a subscriber where in the publisher the event fires. T Name by position: `OnBeforePostSalesLine` and `OnAfterPostSalesLine` at the routine boundaries, and `OnPostSalesLineOnAfterCalcAmounts` for an event raised partway through `PostSalesLine` after an amount calculation. The name alone then tells a subscriber both the host routine and the exact point it runs. -See sample: `name-events-by-publisher-position.good.al`. +See sample: [`name-events-by-publisher-position.good.al`](name-events-by-publisher-position.good.al). ## Anti Pattern Ad-hoc event names that omit the host routine or the before/after position (`MyCustomSalesEvent`, `BeforePost`, `SalesLineEvent`), leaving subscribers unable to tell when the event fires relative to the publisher's logic. Detection: publisher names that do not follow the `OnBefore`/`OnAfter` or `OnOnBefore`/`OnAfter` patterns. -See sample: `name-events-by-publisher-position.bad.al`. +See sample: [`name-events-by-publisher-position.bad.al`](name-events-by-publisher-position.bad.al). diff --git a/microsoft/knowledge/events/prefer-reusing-or-extending-existing-events.md b/microsoft/knowledge/events/prefer-reusing-or-extending-existing-events.md index 3136023..d2e272b 100644 --- a/microsoft/knowledge/events/prefer-reusing-or-extending-existing-events.md +++ b/microsoft/knowledge/events/prefer-reusing-or-extending-existing-events.md @@ -17,10 +17,10 @@ Before adding a publisher, check whether an event already fires at that point in When the data you need is already exposed at an existing event, subscribe to it. When the event lacks a parameter, extend that event by appending the parameter at the end โ€” one publisher, one raise โ€” rather than adding a second event beside it. -See sample: `prefer-reusing-or-extending-existing-events.good.al`. +See sample: [`prefer-reusing-or-extending-existing-events.good.al`](prefer-reusing-or-extending-existing-events.good.al). ## Anti Pattern Adding a second event raise immediately after an existing one, or creating `OnBeforeProcessOrderWithCustomer` next to `OnBeforeProcessOrder` just to add a single parameter. Detection: two consecutive `OnBeforeโ€ฆ`/`OnAfterโ€ฆ` raises with no logic between them, or near-duplicate event names differing only by a parameter-describing suffix. -See sample: `prefer-reusing-or-extending-existing-events.bad.al`. +See sample: [`prefer-reusing-or-extending-existing-events.bad.al`](prefer-reusing-or-extending-existing-events.bad.al). diff --git a/microsoft/knowledge/events/prefer-this-over-includesender-in-codeunit-events.md b/microsoft/knowledge/events/prefer-this-over-includesender-in-codeunit-events.md index 35d75dc..2af8daa 100644 --- a/microsoft/knowledge/events/prefer-this-over-includesender-in-codeunit-events.md +++ b/microsoft/knowledge/events/prefer-this-over-includesender-in-codeunit-events.md @@ -17,10 +17,10 @@ When designing a new publisher, setting `IncludeSender` to `true` on `[Integrati For a new event, declare the publisher `[IntegrationEvent(false, false)]` with an explicit `Sender: Codeunit "โ€ฆ"` parameter and raise it with `this`, for example `OnBeforeProcessOrder(OrderNo, this);`. Subscribers then receive a typed sender they can call directly. -See sample: `prefer-this-over-includesender-in-codeunit-events.good.al`. +See sample: [`prefer-this-over-includesender-in-codeunit-events.good.al`](prefer-this-over-includesender-in-codeunit-events.good.al). ## Anti Pattern Designing a new codeunit event with `[IntegrationEvent(true, โ€ฆ)]` solely to hand subscribers the publisher instance, where `this` could be passed explicitly as a typed parameter. Do not apply this rule by mutating a shipped event's attribute flags. -See sample: `prefer-this-over-includesender-in-codeunit-events.bad.al`. +See sample: [`prefer-this-over-includesender-in-codeunit-events.bad.al`](prefer-this-over-includesender-in-codeunit-events.bad.al). diff --git a/microsoft/knowledge/events/prefix-temporary-record-event-parameters-with-temp.md b/microsoft/knowledge/events/prefix-temporary-record-event-parameters-with-temp.md index 0a952a5..08370bf 100644 --- a/microsoft/knowledge/events/prefix-temporary-record-event-parameters-with-temp.md +++ b/microsoft/knowledge/events/prefix-temporary-record-event-parameters-with-temp.md @@ -17,10 +17,10 @@ When a record passed to an event is a temporary record โ€” an in-memory buffer n Name temporary record parameters with a `Temp` prefix, for example `var TempSalesLineBuffer: Record "Sales Line" temporary`, so every subscriber sees immediately that the record is an in-memory buffer and treats writes accordingly. -See sample: `prefix-temporary-record-event-parameters-with-temp.good.al`. +See sample: [`prefix-temporary-record-event-parameters-with-temp.good.al`](prefix-temporary-record-event-parameters-with-temp.good.al). ## Anti Pattern A temporary record parameter named without the `Temp` prefix (`var SalesLineBuffer: Record "Sales Line" temporary`), so subscribers cannot tell the record is non-persistent and may rely on writes that are silently discarded. Detection: an event parameter declared `temporary` whose name does not start with `Temp`. -See sample: `prefix-temporary-record-event-parameters-with-temp.bad.al`. +See sample: [`prefix-temporary-record-event-parameters-with-temp.bad.al`](prefix-temporary-record-event-parameters-with-temp.bad.al). diff --git a/microsoft/knowledge/events/preserve-onafter-execution-when-ishandled-skips-the-body.md b/microsoft/knowledge/events/preserve-onafter-execution-when-ishandled-skips-the-body.md index 4ff958c..10b7fd1 100644 --- a/microsoft/knowledge/events/preserve-onafter-execution-when-ishandled-skips-the-body.md +++ b/microsoft/knowledge/events/preserve-onafter-execution-when-ishandled-skips-the-body.md @@ -17,10 +17,10 @@ A routine that exposes both an `OnBeforeโ€ฆ` event (with `var IsHandled`) and a Wrap only the default work in `if not IsHandled then begin โ€ฆ end;` and keep the `OnAfterX(โ€ฆ)` raise after that block, outside the guard, so it always fires regardless of whether a subscriber handled the OnBefore. This keeps the override seam and the after-notification independent, which is what subscribers expect. -See sample: `preserve-onafter-execution-when-ishandled-skips-the-body.good.al`. +See sample: [`preserve-onafter-execution-when-ishandled-skips-the-body.good.al`](preserve-onafter-execution-when-ishandled-skips-the-body.good.al). ## Anti Pattern Guarding with `if IsHandled then exit;` and placing the `OnAfterX` raise later in the same routine, so handling the OnBefore short-circuits the whole procedure and the OnAfter event is skipped along with the body. Detection: an `if IsHandled then exit;` in a routine that also raises a paired `OnAfterโ€ฆ` event after that point. -See sample: `preserve-onafter-execution-when-ishandled-skips-the-body.bad.al`. +See sample: [`preserve-onafter-execution-when-ishandled-skips-the-body.bad.al`](preserve-onafter-execution-when-ishandled-skips-the-body.bad.al). diff --git a/microsoft/knowledge/events/publish-thin-onbefore-onafter-integration-events.md b/microsoft/knowledge/events/publish-thin-onbefore-onafter-integration-events.md index 30c94df..526f136 100644 --- a/microsoft/knowledge/events/publish-thin-onbefore-onafter-integration-events.md +++ b/microsoft/knowledge/events/publish-thin-onbefore-onafter-integration-events.md @@ -17,10 +17,10 @@ A key operation โ€” a posting, release, or validation routine โ€” becomes a hard Wrap the operation's core with events: raise `OnBeforeX(var Rec, var IsHandled)` before the default work and `OnAfterX(var Rec)` once it succeeds, at the natural boundaries of the routine. Declare each publisher `[IntegrationEvent(false, false)] local procedure` with an empty body and let the calling routine โ€” never the publisher โ€” own the logic. Pass records by `var` so subscribers can read and adjust them, and include the parameters a subscriber would need to act. This gives partners a stable seam without touching base code. -See sample: `publish-thin-onbefore-onafter-integration-events.good.al`. +See sample: [`publish-thin-onbefore-onafter-integration-events.good.al`](publish-thin-onbefore-onafter-integration-events.good.al). ## Anti Pattern Business logic placed inside an `[IntegrationEvent]` publisher method, so the "event" actually mutates state every time it is raised โ€” defeating the hook and surprising every reader โ€” or a core operation that exposes no extension points at all, forcing partners to overwrite or duplicate it. Detection: an `[IntegrationEvent]`/`[BusinessEvent]` method whose body contains statements rather than being empty, or a posting/validation routine with no surrounding `OnBefore`/`OnAfter` publishers. -See sample: `publish-thin-onbefore-onafter-integration-events.bad.al`. +See sample: [`publish-thin-onbefore-onafter-integration-events.bad.al`](publish-thin-onbefore-onafter-integration-events.bad.al). diff --git a/microsoft/knowledge/events/reset-ishandled-only-when-the-value-can-carry-over.md b/microsoft/knowledge/events/reset-ishandled-only-when-the-value-can-carry-over.md index fba378b..67d5111 100644 --- a/microsoft/knowledge/events/reset-ishandled-only-when-the-value-can-carry-over.md +++ b/microsoft/knowledge/events/reset-ishandled-only-when-the-value-can-carry-over.md @@ -17,10 +17,10 @@ A routine that raises an `OnBeforeโ€ฆ` integration event with a `var IsHandled: Reset `IsHandled := false;` before a raise only when the value might otherwise carry over as `true`: the same variable is reused after an earlier raise without a control-flow proof that it is false, a raise is re-entered by a loop, the value comes from an input parameter, field, or global, or earlier code seeds it. Prefer separate fresh locals when independent event seams need independent handled state. A reset on a guaranteed-false fresh local used by one non-looping raise, or before a later raise reached only after a semantically valid `if IsHandled then exit;`, can be retained for readability, but its absence is not a correctness finding. -See sample: `reset-ishandled-only-when-the-value-can-carry-over.good.al`. +See sample: [`reset-ishandled-only-when-the-value-can-carry-over.good.al`](reset-ishandled-only-when-the-value-can-carry-over.good.al). ## Anti Pattern Raising `OnBeforeX(โ€ฆ, IsHandled)` when the variable can still be `true` from an earlier raise, an earlier loop iteration, or another source, so the publisher call starts with stale state. Do not match a single non-looping raise using a fresh local Boolean, or a later raise reached only after a semantically valid `if IsHandled then exit;` proves the value is false. -See sample: `reset-ishandled-only-when-the-value-can-carry-over.bad.al`. +See sample: [`reset-ishandled-only-when-the-value-can-carry-over.bad.al`](reset-ishandled-only-when-the-value-can-carry-over.bad.al). diff --git a/microsoft/knowledge/events/treat-local-and-internal-events-as-subscriber-contracts.md b/microsoft/knowledge/events/treat-local-and-internal-events-as-subscriber-contracts.md index 95a2617..004819b 100644 --- a/microsoft/knowledge/events/treat-local-and-internal-events-as-subscriber-contracts.md +++ b/microsoft/knowledge/events/treat-local-and-internal-events-as-subscriber-contracts.md @@ -17,10 +17,10 @@ The `local` and `internal` access modifiers on Business and Integration event pu Preserve a shipped Business or Integration event's identity and every existing parameter's name, type/subtype, and passing mode regardless of the procedure access modifier. AS0025 protects names and types, while AS0063 and AS0077 protect removal and addition of `var`. New parameters may be added at any position on a `local` or `internal` event because subscribers can omit them; public event procedures follow the stricter caller contract described by `add-new-event-parameters-at-the-end`. -See sample: `treat-local-and-internal-events-as-subscriber-contracts.good.al`. +See sample: [`treat-local-and-internal-events-as-subscriber-contracts.good.al`](treat-local-and-internal-events-as-subscriber-contracts.good.al). ## Anti Pattern Renaming or removing an existing parameter, changing its type/subtype, or adding/removing its `var` modifier because the event publisher procedure is `local` or `internal`. AppSourceCop checks these subscriber-breaking changes because dependent event subscribers can still bind to the event. Reordering unchanged parameters, or inserting a new parameter among them, is not this anti-pattern. -See sample: `treat-local-and-internal-events-as-subscriber-contracts.bad.al`. +See sample: [`treat-local-and-internal-events-as-subscriber-contracts.bad.al`](treat-local-and-internal-events-as-subscriber-contracts.bad.al). diff --git a/microsoft/knowledge/events/use-ishandled-to-make-base-behaviour-overridable.md b/microsoft/knowledge/events/use-ishandled-to-make-base-behaviour-overridable.md index d273589..6a6d6ab 100644 --- a/microsoft/knowledge/events/use-ishandled-to-make-base-behaviour-overridable.md +++ b/microsoft/knowledge/events/use-ishandled-to-make-base-behaviour-overridable.md @@ -17,10 +17,10 @@ AL has no method overriding, so a `procedure` that runs its body unconditionally Raise `OnBeforeX(โ€ฆ, IsHandled)` as the first step of the routine and guard with `if IsHandled then exit;` before any default logic runs. Declare the publisher `[IntegrationEvent(false, false)] local procedure OnBeforeX(โ€ฆ; var IsHandled: Boolean)` with an empty body, and keep `IsHandled` a `var` parameter so a subscriber can write to it. A subscriber that replaces the behaviour does its work and sets `IsHandled := true`; one that only augments leaves it untouched and guards with `if IsHandled then exit;` itself. Reserve the override hook for cases where a partner genuinely needs to replace logic โ€” when the goal is only to react, a positive `OnAfter` event is the better seam. -See sample: `use-ishandled-to-make-base-behaviour-overridable.good.al`. +See sample: [`use-ishandled-to-make-base-behaviour-overridable.good.al`](use-ishandled-to-make-base-behaviour-overridable.good.al). ## Anti Pattern Two shapes. First, a routine whose default logic always runs because there is no `OnBeforeโ€ฆ`/`IsHandled` hook at all โ€” extensions cannot change it without overwriting base code. Second, a routine that raises `OnBeforeX(IsHandled)` but omits the `if IsHandled then exit;` guard, so the default logic still executes after a subscriber set `IsHandled := true`, duplicating work and side effects. Detection: an `OnBefore` publisher with a `var IsHandled: Boolean` parameter whose caller never tests `IsHandled`, or a public routine doing non-trivial work with no overridable seam. -See sample: `use-ishandled-to-make-base-behaviour-overridable.bad.al`. +See sample: [`use-ishandled-to-make-base-behaviour-overridable.bad.al`](use-ishandled-to-make-base-behaviour-overridable.bad.al). diff --git a/microsoft/knowledge/interfaces/assign-codeunit-to-interface-for-testability.md b/microsoft/knowledge/interfaces/assign-codeunit-to-interface-for-testability.md index e0d50d7..8d3e7ce 100644 --- a/microsoft/knowledge/interfaces/assign-codeunit-to-interface-for-testability.md +++ b/microsoft/knowledge/interfaces/assign-codeunit-to-interface-for-testability.md @@ -17,10 +17,10 @@ An interface variable can hold any codeunit that `implements` the interface, ass Declare the dependency as an `Interface` variable on the consumer and supply the implementation from outside โ€” typically setter injection through a procedure that takes an `Interface` parameter, or a parameter on the entry method. Production passes the real implementation codeunit; a test passes a test-double codeunit that implements the same interface with deterministic behaviour. Because a codeunit assigns to an interface variable directly, no enum or factory is needed for the injectable case. The consumer's logic is then verifiable in isolation. -See sample: `assign-codeunit-to-interface-for-testability.good.al`. +See sample: [`assign-codeunit-to-interface-for-testability.good.al`](assign-codeunit-to-interface-for-testability.good.al). ## Anti Pattern A consumer that declares its dependency as a concrete `Codeunit "..."` variable and calls it directly. The collaborator cannot be substituted, so a unit test either runs the production side effects or cannot cover the consumer at all. Detection signal: a `var` of type `Codeunit ""` used for a collaborator that has โ€” or could have โ€” an interface, especially one that performs I/O, posting, or external calls. Extract an interface, depend on the interface variable, and inject the implementation. -See sample: `assign-codeunit-to-interface-for-testability.bad.al`. +See sample: [`assign-codeunit-to-interface-for-testability.bad.al`](assign-codeunit-to-interface-for-testability.bad.al). diff --git a/microsoft/knowledge/interfaces/extend-published-interfaces-dont-edit-them.md b/microsoft/knowledge/interfaces/extend-published-interfaces-dont-edit-them.md index 2aceec4..4f04d67 100644 --- a/microsoft/knowledge/interfaces/extend-published-interfaces-dont-edit-them.md +++ b/microsoft/knowledge/interfaces/extend-published-interfaces-dont-edit-them.md @@ -17,10 +17,10 @@ Adding a method to a shipped interface changes the contract every implementing c On BC25 or later, declare a new interface that `extends` the published interface and add the new method there. Existing implementers remain valid for the original contract, while new implementers opt in to the extended contract. For targets BC16 through BC24, where interface inheritance is unavailable, publish a new or versioned sibling interface instead. -See sample: `extend-published-interfaces-dont-edit-them.good.al`. +See sample: [`extend-published-interfaces-dont-edit-them.good.al`](extend-published-interfaces-dont-edit-them.good.al). ## Anti Pattern Adding a procedure directly to an interface that has already shipped. Every dependent implementation must immediately add that procedure, so an otherwise compatible app update breaks its implementers. -See sample: `extend-published-interfaces-dont-edit-them.bad.al`. +See sample: [`extend-published-interfaces-dont-edit-them.bad.al`](extend-published-interfaces-dont-edit-them.bad.al). diff --git a/microsoft/knowledge/interfaces/handle-unknown-enum-ordinals-with-unknownvalueimplementation.md b/microsoft/knowledge/interfaces/handle-unknown-enum-ordinals-with-unknownvalueimplementation.md index d3715e6..de50810 100644 --- a/microsoft/knowledge/interfaces/handle-unknown-enum-ordinals-with-unknownvalueimplementation.md +++ b/microsoft/knowledge/interfaces/handle-unknown-enum-ordinals-with-unknownvalueimplementation.md @@ -17,10 +17,10 @@ An enum ordinal can remain in persisted data after the enum extension that decla On BC18 or later, set `UnknownValueImplementation = = ;` on an enum that implements an interface and can be persisted. Use an implementation that reports a clear domain error or safely contains the unknown state. Keep `DefaultImplementation` separately when declared but unmapped values also need a fallback. -See sample: `handle-unknown-enum-ordinals-with-unknownvalueimplementation.good.al`. +See sample: [`handle-unknown-enum-ordinals-with-unknownvalueimplementation.good.al`](handle-unknown-enum-ordinals-with-unknownvalueimplementation.good.al). ## Anti Pattern Defining only `DefaultImplementation` and assuming it also handles a stored ordinal whose enum value has disappeared. After an enum extension is uninstalled, converting that unknown ordinal to the interface can produce a technical runtime error instead of controlled handling. -See sample: `handle-unknown-enum-ordinals-with-unknownvalueimplementation.bad.al`. +See sample: [`handle-unknown-enum-ordinals-with-unknownvalueimplementation.bad.al`](handle-unknown-enum-ordinals-with-unknownvalueimplementation.bad.al). diff --git a/microsoft/knowledge/interfaces/prefer-interface-over-case-branching.md b/microsoft/knowledge/interfaces/prefer-interface-over-case-branching.md index 337e0dc..b96b5e8 100644 --- a/microsoft/knowledge/interfaces/prefer-interface-over-case-branching.md +++ b/microsoft/knowledge/interfaces/prefer-interface-over-case-branching.md @@ -17,10 +17,10 @@ When behaviour varies by a discrete "type" โ€” a shipping method, a posting stra Declare an `interface` with the method signatures only (no bodies). Define an `enum` that `implements` the interface and set `Implementation = = ;` on each value, pointing at a codeunit that `implements` the same interface. In the consumer, declare a variable of the interface type, assign the enum value to it, and call the method โ€” the platform dispatches to the codeunit mapped to that value. New variants plug in by adding an enum value and its implementation; existing call sites are untouched. The open/closed boundary lives at the enum, not scattered across `case` blocks. -See sample: `prefer-interface-over-case-branching.good.al`. +See sample: [`prefer-interface-over-case-branching.good.al`](prefer-interface-over-case-branching.good.al). ## Anti Pattern A `case "Shipping Method" of` block that selects behaviour inline, duplicated across the call sites that need it. Each new method forces a synchronized edit to every block, and a missed branch is a silent gap. Detection signal: a `case` statement over an enum value whose branches choose between variant computations or strategies, especially when the same shape appears in more than one procedure. Replace the enum with one that `implements` an interface, move each branch body into an implementation codeunit, and let dispatch happen through an interface variable. -See sample: `prefer-interface-over-case-branching.bad.al`. +See sample: [`prefer-interface-over-case-branching.bad.al`](prefer-interface-over-case-branching.bad.al). diff --git a/microsoft/knowledge/interfaces/set-defaultimplementation-on-enum.md b/microsoft/knowledge/interfaces/set-defaultimplementation-on-enum.md index 7d2523e..a5c3bb6 100644 --- a/microsoft/knowledge/interfaces/set-defaultimplementation-on-enum.md +++ b/microsoft/knowledge/interfaces/set-defaultimplementation-on-enum.md @@ -17,10 +17,10 @@ An `enum` that `implements` an interface maps each declared value to a codeunit On any extensible enum that implements an interface, set `DefaultImplementation = = ;` at the enum level, pointing at a safe implementation. Values with their own `Implementation` keep using it; declared values without one resolve to the default. Do not rely on this property for persisted ordinals that match no declared enum value. -See sample: `set-defaultimplementation-on-enum.good.al`. +See sample: [`set-defaultimplementation-on-enum.good.al`](set-defaultimplementation-on-enum.good.al). ## Anti Pattern An extensible `enum ... implements ` where at least one value sets no `Implementation` and the enum declares no `DefaultImplementation`. Code that assigns that value to an interface variable and invokes a method throws at the call site, and because the enum is extensible the failing value can be introduced by a third party long after the consumer ships. Detection signal: an enum that implements an interface, has a `value(...)` with no `Implementation`, and no enum-level `DefaultImplementation`. Add a `DefaultImplementation` mapping to close the gap. -See sample: `set-defaultimplementation-on-enum.bad.al`. +See sample: [`set-defaultimplementation-on-enum.bad.al`](set-defaultimplementation-on-enum.bad.al). diff --git a/microsoft/knowledge/performance/addloadfields-in-report-onpredataitem.md b/microsoft/knowledge/performance/addloadfields-in-report-onpredataitem.md index 683a8a0..b72fc41 100644 --- a/microsoft/knowledge/performance/addloadfields-in-report-onpredataitem.md +++ b/microsoft/knowledge/performance/addloadfields-in-report-onpredataitem.md @@ -17,10 +17,10 @@ Report dataitem field selection is calculated at compile time and once per datai When a dataitem trigger needs an extra field, add that field in `OnPreDataItem` before iteration starts. This supplements the compiler-selected fields and avoids the first just-in-time load and enumerator update when the trigger reads the extra field. -See sample: `addloadfields-in-report-onpredataitem.good.al`. +See sample: [`addloadfields-in-report-onpredataitem.good.al`](addloadfields-in-report-onpredataitem.good.al). ## Anti Pattern Listing every dataset column in `AddLoadFields`, or omitting a known trigger-only field because the dataset already uses other fields. The former is redundant; the latter causes a just-in-time load on first access and can cause repeated loads when the record is copied or passed by value. -See sample: `addloadfields-in-report-onpredataitem.bad.al`. +See sample: [`addloadfields-in-report-onpredataitem.bad.al`](addloadfields-in-report-onpredataitem.bad.al). diff --git a/microsoft/knowledge/performance/apply-filters-before-iterating.md b/microsoft/knowledge/performance/apply-filters-before-iterating.md index 76d78ec..4c64be2 100644 --- a/microsoft/knowledge/performance/apply-filters-before-iterating.md +++ b/microsoft/knowledge/performance/apply-filters-before-iterating.md @@ -17,10 +17,10 @@ A `SetRange` or `SetFilter` placed before `FindSet` narrows the result set at th Move every predicate that can be expressed as an equality or range filter into a `SetRange` or `SetFilter` ahead of the find. Make sure a key (index) exists whose leading fields cover the filter so the optimizer can seek; note that `SetCurrentKey` only sets sort order and is not an index hint (see `setcurrentkey-sets-sort-order-not-index-hint.md`). The loop body should then contain only the work that depends on per-row state. -See sample: `apply-filters-before-iterating.good.al`. +See sample: [`apply-filters-before-iterating.good.al`](apply-filters-before-iterating.good.al). ## Anti Pattern `if Customer.FindSet() then repeat if Customer."Country/Region Code" = 'US' then ProcessCustomer(Customer); until Customer.Next() = 0;` โ€” the loop pays for every row in the table and discards the non-matching ones in AL. The intent is the same as a `SetRange("Country/Region Code", 'US')` ahead of the find, but the cost is not. -See sample: `apply-filters-before-iterating.bad.al`. +See sample: [`apply-filters-before-iterating.bad.al`](apply-filters-before-iterating.bad.al). diff --git a/microsoft/knowledge/performance/apply-guards-before-get.md b/microsoft/knowledge/performance/apply-guards-before-get.md index 9e77411..8d4a876 100644 --- a/microsoft/knowledge/performance/apply-guards-before-get.md +++ b/microsoft/knowledge/performance/apply-guards-before-get.md @@ -17,10 +17,10 @@ A `Get` (or any other database call) executed before a guard that may exit the p Read the procedure top-to-bottom and place every condition that can short-circuit ahead of every database call. The check `if SomeNo = '' then exit;` belongs above `Header.Get(...)`, not below. Each guard moved upward saves one wasted query on the path that exits. -See sample: `apply-guards-before-get.good.al`. +See sample: [`apply-guards-before-get.good.al`](apply-guards-before-get.good.al). ## Anti Pattern `Record.Get(...)` at the top of a procedure followed by `if SomeField = '' then exit;`. The code reads top-down as "load the record, then decide whether we needed it" โ€” exactly the order that wastes the query. The pattern is easy to introduce when guards are added later, defensively, without re-checking call ordering. -See sample: `apply-guards-before-get.bad.al`. +See sample: [`apply-guards-before-get.bad.al`](apply-guards-before-get.bad.al). diff --git a/microsoft/knowledge/performance/avoid-cloning-records-before-modify-delete-in-loops.md b/microsoft/knowledge/performance/avoid-cloning-records-before-modify-delete-in-loops.md index 66ee684..20f27ee 100644 --- a/microsoft/knowledge/performance/avoid-cloning-records-before-modify-delete-in-loops.md +++ b/microsoft/knowledge/performance/avoid-cloning-records-before-modify-delete-in-loops.md @@ -17,10 +17,10 @@ Microsoft's [AL database-method performance guidance](https://learn.microsoft.co Use `FindSet(true)` when the loop writes the traversed rows, and call `Modify` or `Delete` on that iterating record variable. If generic code is required, open and iterate the `RecordRef` directly instead of calling `GetTable` for each typed record. Keep a per-row loop when validation or row-specific behavior is required; this rule does not imply that `ModifyAll` or `DeleteAll` is equivalent. -See sample: `avoid-cloning-records-before-modify-delete-in-loops.good.al`. +See sample: [`avoid-cloning-records-before-modify-delete-in-loops.good.al`](avoid-cloning-records-before-modify-delete-in-loops.good.al). ## Anti Pattern Inside an active traversal, copy the current row, convert it with `RecordRef.GetTable`, or pass it without `var` to a helper, then call `Modify` or `Delete` on that clone. Do not flag read-only snapshots, temporary records, or copies used to write a different target table; the documented extra-statement concern is clone-before-write on the traversed table. -See sample: `avoid-cloning-records-before-modify-delete-in-loops.bad.al`. +See sample: [`avoid-cloning-records-before-modify-delete-in-loops.bad.al`](avoid-cloning-records-before-modify-delete-in-loops.bad.al). diff --git a/microsoft/knowledge/performance/avoid-commit-inside-loops.md b/microsoft/knowledge/performance/avoid-commit-inside-loops.md index 25ad958..011fd39 100644 --- a/microsoft/knowledge/performance/avoid-commit-inside-loops.md +++ b/microsoft/knowledge/performance/avoid-commit-inside-loops.md @@ -21,10 +21,10 @@ A durability checkpoint inside an outer batch loop can be valid only when the sa If the batch is large enough that a single transaction is untenable, use an ordered primary-key watermark and retrieve a bounded next-N key list. The sample uses a query capped by [`TopNumberOfRows`](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/query/queryinstance-topnumberofrows-method) to fill a temporary key buffer, then takes update locks and modifies only those exact keys. It does not reconstruct an inclusive first-to-last range that concurrent inserts could expand. Persist the last selected key in the same transaction as the completed chunk, then commit after the bounded helper returns. Use a stable key and define how a later run handles records inserted at or below an already committed watermark. Let errors escape so failed work is not recorded as complete. A `Codeunit.Run` boundary can also own a chunk when its implicit commit and error behavior fit the caller โ€” see `codeunit-run-as-atomic-sub-operation.md`. -See sample: `avoid-commit-inside-loops.good.al`. +See sample: [`avoid-commit-inside-loops.good.al`](avoid-commit-inside-loops.good.al). ## Anti Pattern Placing Commit inside `repeat ... until Next() = 0` without persisted progress is almost always a mistake: retries re-enter already committed work, while the cost of starting a transaction on every row dominates the operation. A progress variable held only in memory is not restart-safe. A full-tail `FindSet` with a commit every N rows is not bounded retrieval, even if a persisted watermark makes it restart-safe. A capped query that discovers only an upper key and then re-reads an inclusive key range is not exact batching either; concurrent inserts inside that range can enlarge the checkpoint. -See sample: `avoid-commit-inside-loops.bad.al`. +See sample: [`avoid-commit-inside-loops.bad.al`](avoid-commit-inside-loops.bad.al). diff --git a/microsoft/knowledge/performance/avoid-currpage-update-in-onaftergetrecord.md b/microsoft/knowledge/performance/avoid-currpage-update-in-onaftergetrecord.md index 20e6968..5790a6c 100644 --- a/microsoft/knowledge/performance/avoid-currpage-update-in-onaftergetrecord.md +++ b/microsoft/knowledge/performance/avoid-currpage-update-in-onaftergetrecord.md @@ -19,10 +19,10 @@ application-area: [all] Put display-only results in page variables assigned in `OnAfterGetRecord` without calling `Update`. If the page must refresh after an action, call `CurrPage.Update(false)` from `OnAction` once, not per row. -See sample: `avoid-currpage-update-in-onaftergetrecord.good.al`. +See sample: [`avoid-currpage-update-in-onaftergetrecord.good.al`](avoid-currpage-update-in-onaftergetrecord.good.al). ## Anti Pattern `trigger OnAfterGetRecord() begin ... CurrPage.Update(); end;` on a list. The signal is `CurrPage.Update` inside `OnAfterGetRecord` or `OnAfterGetCurrRecord` without an explicit user action. -See sample: `avoid-currpage-update-in-onaftergetrecord.bad.al`. +See sample: [`avoid-currpage-update-in-onaftergetrecord.bad.al`](avoid-currpage-update-in-onaftergetrecord.bad.al). diff --git a/microsoft/knowledge/performance/avoid-get-inside-loop-on-large-table.md b/microsoft/knowledge/performance/avoid-get-inside-loop-on-large-table.md index f77fbfe..db4f5e1 100644 --- a/microsoft/knowledge/performance/avoid-get-inside-loop-on-large-table.md +++ b/microsoft/knowledge/performance/avoid-get-inside-loop-on-large-table.md @@ -17,10 +17,10 @@ A `Get` or `FindFirst` against another persistent table inside a loop can produc Use a query object to join the outer and inner tables when the relationship and filters can be expressed as one query. If keys repeat, a dictionary cache can reduce lookups to one per distinct key. `SetLoadFields` can reduce the columns transferred by unavoidable inner reads, but it does not eliminate the N+1 shape and must not be presented as doing so. -See sample: `avoid-get-inside-loop-on-large-table.good.al`. +See sample: [`avoid-get-inside-loop-on-large-table.good.al`](avoid-get-inside-loop-on-large-table.good.al). ## Anti Pattern Iterating production BOM lines and calling `Item.Get(BOMLine."No.")` for each line when the same result can be produced by a query joining Production BOM Line to Item. Partial loading alone is only a payload mitigation for this pattern. -See sample: `avoid-get-inside-loop-on-large-table.bad.al`. +See sample: [`avoid-get-inside-loop-on-large-table.bad.al`](avoid-get-inside-loop-on-large-table.bad.al). diff --git a/microsoft/knowledge/performance/avoid-growing-globals-in-singleinstance-subscribers.md b/microsoft/knowledge/performance/avoid-growing-globals-in-singleinstance-subscribers.md index 966e0dd..82a3bb7 100644 --- a/microsoft/knowledge/performance/avoid-growing-globals-in-singleinstance-subscribers.md +++ b/microsoft/knowledge/performance/avoid-growing-globals-in-singleinstance-subscribers.md @@ -19,10 +19,10 @@ A codeunit with `SingleInstance = true` is allocated once per session and lives Keep the global footprint on a SingleInstance subscriber bounded and intentional: a handful of flags, a setup record, a bounded cache with a maximum size. When cross-event state is genuinely needed, define an explicit reset point โ€” end of a business process, arrival of a specific terminal event โ€” that clears the growing collection. -See sample: `avoid-growing-globals-in-singleinstance-subscribers.good.al`. +See sample: [`avoid-growing-globals-in-singleinstance-subscribers.good.al`](avoid-growing-globals-in-singleinstance-subscribers.good.al). ## Anti Pattern A SingleInstance subscriber that appends each event's payload to a global list, dictionary, or temporary record without a cap or cleanup trigger. The list grows for hours, memory pressure builds quietly, and debugging the root cause on a live environment is substantially harder than noticing the unbounded append in code review. -See sample: `avoid-growing-globals-in-singleinstance-subscribers.bad.al`. +See sample: [`avoid-growing-globals-in-singleinstance-subscribers.bad.al`](avoid-growing-globals-in-singleinstance-subscribers.bad.al). diff --git a/microsoft/knowledge/performance/avoid-recordref-in-hot-loop.md b/microsoft/knowledge/performance/avoid-recordref-in-hot-loop.md index b718449..6aae177 100644 --- a/microsoft/knowledge/performance/avoid-recordref-in-hot-loop.md +++ b/microsoft/knowledge/performance/avoid-recordref-in-hot-loop.md @@ -17,10 +17,10 @@ application-area: [all] Use `RecordRef`/`FieldRef` for genuinely generic code โ€” permission checks, field copying, table-agnostic export. When the loop target is known at compile time and the loop iterates a large number of rows, declare the typed record and access fields directly; the saved per-iteration overhead is measurable at the volumes the rule targets. -See sample: `avoid-recordref-in-hot-loop.good.al`. +See sample: [`avoid-recordref-in-hot-loop.good.al`](avoid-recordref-in-hot-loop.good.al). ## Anti Pattern `RecRef.Open(Database::Customer); if RecRef.FindSet() then repeat FldRef := RecRef.Field(Customer.FieldNo(Name)); ProcessName(FldRef.Value); until RecRef.Next() = 0;` โ€” the table is fixed at compile time, the field is fixed at compile time, and the loop pays the dynamic-resolution cost on every iteration. The direct `Customer.Name` form does the same work without the lookup. -See sample: `avoid-recordref-in-hot-loop.bad.al`. +See sample: [`avoid-recordref-in-hot-loop.bad.al`](avoid-recordref-in-hot-loop.bad.al). diff --git a/microsoft/knowledge/performance/avoid-redundant-get-when-record-already-loaded.md b/microsoft/knowledge/performance/avoid-redundant-get-when-record-already-loaded.md index 9684769..3859b7c 100644 --- a/microsoft/knowledge/performance/avoid-redundant-get-when-record-already-loaded.md +++ b/microsoft/knowledge/performance/avoid-redundant-get-when-record-already-loaded.md @@ -17,10 +17,10 @@ A list or card page's `OnAfterGetRecord` trigger fires *because* the platform ha Inside page triggers โ€” `OnAfterGetRecord`, `OnAfterGetCurrRecord`, validation triggers โ€” read from `Rec` (or the trigger's record parameter). The platform exposes the freshly loaded record there for exactly this purpose. Reach for `Get` only when the trigger needs a *different* record than the one being displayed. -See sample: `avoid-redundant-get-when-record-already-loaded.good.al`. +See sample: [`avoid-redundant-get-when-record-already-loaded.good.al`](avoid-redundant-get-when-record-already-loaded.good.al). ## Anti Pattern `AssemblyLineRec.Get("Document Type", "Document No.", "Line No.");` at the top of `OnAfterGetRecord`, when the trigger is on the `Assembly Line` page itself and `Rec` already holds that row. The pattern often appears when a helper that expects a record parameter is invoked from a page trigger and the author writes a `Get` to "freshen" `Rec` rather than passing `Rec` through. -See sample: `avoid-redundant-get-when-record-already-loaded.bad.al`. +See sample: [`avoid-redundant-get-when-record-already-loaded.bad.al`](avoid-redundant-get-when-record-already-loaded.bad.al). diff --git a/microsoft/knowledge/performance/avoid-user-prompts-inside-transactions.md b/microsoft/knowledge/performance/avoid-user-prompts-inside-transactions.md index 834641a..bb03f41 100644 --- a/microsoft/knowledge/performance/avoid-user-prompts-inside-transactions.md +++ b/microsoft/knowledge/performance/avoid-user-prompts-inside-transactions.md @@ -17,10 +17,10 @@ A `Confirm`, `StrMenu`, modal page, or other user prompt issued from inside a wr Sequence the operation so user confirmation happens *before* any database write that takes a lock the prompt holds open. The shape is: ask the user โ†’ if confirmed, acquire locks and post. `if Confirm(...) then begin SalesHeader.LockTable(); SalesHeader.Get(DocNo); PostSalesOrder(SalesHeader); end;` keeps the lock window down to the work itself. -See sample: `avoid-user-prompts-inside-transactions.good.al`. +See sample: [`avoid-user-prompts-inside-transactions.good.al`](avoid-user-prompts-inside-transactions.good.al). ## Anti Pattern `SalesHeader.LockTable(); SalesHeader.Get(DocNo); if Confirm('Post this order?') then ...;` โ€” the lock is held for as long as the dialog is up. A user who steps away to lunch holds the lock for an hour, and every other session that touches that row blocks for the duration. -See sample: `avoid-user-prompts-inside-transactions.bad.al`. +See sample: [`avoid-user-prompts-inside-transactions.bad.al`](avoid-user-prompts-inside-transactions.bad.al). diff --git a/microsoft/knowledge/performance/batch-number-series-instead-of-getnextno-per-row.md b/microsoft/knowledge/performance/batch-number-series-instead-of-getnextno-per-row.md index 616c789..cdb7405 100644 --- a/microsoft/knowledge/performance/batch-number-series-instead-of-getnextno-per-row.md +++ b/microsoft/knowledge/performance/batch-number-series-instead-of-getnextno-per-row.md @@ -19,10 +19,10 @@ application-area: [all] Inside a multi-row insert, call `"No. Series - Batch".GetNextNo` per row and `SaveState` once after the loop when the series must remain gapless. Use `NumberSequence.Next` when holes are allowed. Do not replace a single `OnInsert` `GetNextNo` for one master record; that path is not the hotspot. -See sample: `batch-number-series-instead-of-getnextno-per-row.good.al`. +See sample: [`batch-number-series-instead-of-getnextno-per-row.good.al`](batch-number-series-instead-of-getnextno-per-row.good.al). ## Anti Pattern `NoSeries.GetNextNo(...)` inside `repeat ... Insert ... until Next() = 0` where the series is **gapless** (Allow Gaps = false). Each iteration takes the series-line lock. The signal is `"No. Series"` (not `"No. Series - Batch"`) in a loop that inserts more than one row; do not flag the same pattern when the series has Allow Gaps enabled, as the `NumberSequence` path already avoids the lock. -See sample: `batch-number-series-instead-of-getnextno-per-row.bad.al`. +See sample: [`batch-number-series-instead-of-getnextno-per-row.bad.al`](batch-number-series-instead-of-getnextno-per-row.bad.al). diff --git a/microsoft/knowledge/performance/boolean-operators-do-not-short-circuit.md b/microsoft/knowledge/performance/boolean-operators-do-not-short-circuit.md index 7c4073b..9c840a7 100644 --- a/microsoft/knowledge/performance/boolean-operators-do-not-short-circuit.md +++ b/microsoft/knowledge/performance/boolean-operators-do-not-short-circuit.md @@ -23,13 +23,13 @@ For an `or`-shaped condition, do not nest: nesting `if A then if B then Action` Where a chain of `and`-guards runs past about three conditions, stop nesting and use a `case` statement instead โ€” see `case-true-of-for-long-condition-chains.md`. Keep `and` and `or` for operands that are independently safe and cheap โ€” in-memory field comparisons, enum tests, bound checks โ€” where combining them reads better and costs nothing. -See sample: `boolean-operators-do-not-short-circuit.good.al`. +See sample: [`boolean-operators-do-not-short-circuit.good.al`](boolean-operators-do-not-short-circuit.good.al). ## Anti Pattern A single condition that joins a guard with an operand depending on that guard, or with an expensive operand, using `and` or `or`. The consequence is either wasted work on every evaluation โ€” a database call or validation procedure invoked even when the outcome is already decided โ€” or a runtime error or silently wrong result that the guard was written to prevent. Applying the `and` fix to an `or` condition is a distinct mistake: rewriting `A or B` as nested `if`s drops the `A`-true/`B`-false case instead of preserving it. Detection signals: an operand that indexes an array or list with a variable whose bounds are checked in a sibling operand; `Record.Get(...)` or a `Find`/`IsEmpty` call as one operand of `and` with a field read of the same record as another; an expensive or unsafe operand combined with `or` next to a condition that alone already makes the result true; a boolean-returning procedure call combined with a cheap field test. The pattern is common in code ported from a language that does short-circuit, and in conditions grown by appending a clause to an existing `if`. -See sample: `boolean-operators-do-not-short-circuit.bad.al`. +See sample: [`boolean-operators-do-not-short-circuit.bad.al`](boolean-operators-do-not-short-circuit.bad.al). ## See also diff --git a/microsoft/knowledge/performance/calcsums-instead-of-calcfields-in-loop.md b/microsoft/knowledge/performance/calcsums-instead-of-calcfields-in-loop.md index 7d0752f..f4d7ad9 100644 --- a/microsoft/knowledge/performance/calcsums-instead-of-calcfields-in-loop.md +++ b/microsoft/knowledge/performance/calcsums-instead-of-calcfields-in-loop.md @@ -17,10 +17,10 @@ application-area: [all] When the procedure totals a FlowField (or several) across a filtered set, set the filters, then call `CalcSums("Field 1", "Field 2", ...)`. The platform issues one query; the result is read off the record's FlowField slot. Single `CalcFields` outside loops is fine, and `CalcFields` on the current row in a page's `OnAfterGetRecord` or in `OnValidate` is the standard pattern โ€” those are per-action, not per-row over a large set. -See sample: `calcsums-instead-of-calcfields-in-loop.good.al`. +See sample: [`calcsums-instead-of-calcfields-in-loop.good.al`](calcsums-instead-of-calcfields-in-loop.good.al). ## Anti Pattern `if CustLedgerEntry.FindSet() then repeat CustLedgerEntry.CalcFields("Remaining Amount"); Total += CustLedgerEntry."Remaining Amount"; until CustLedgerEntry.Next() = 0;` โ€” exactly the upstream-flagged shape. The iteration is the cheap part; the per-row `CalcFields` is what scales linearly with table size. -See sample: `calcsums-instead-of-calcfields-in-loop.bad.al`. +See sample: [`calcsums-instead-of-calcfields-in-loop.bad.al`](calcsums-instead-of-calcfields-in-loop.bad.al). diff --git a/microsoft/knowledge/performance/case-true-of-for-long-condition-chains.md b/microsoft/knowledge/performance/case-true-of-for-long-condition-chains.md index 1e0457e..1ec3863 100644 --- a/microsoft/knowledge/performance/case-true-of-for-long-condition-chains.md +++ b/microsoft/knowledge/performance/case-true-of-for-long-condition-chains.md @@ -17,13 +17,13 @@ Because AL gives no short-circuit guarantee for `and` and `or`, a chain of condi Sequence two or three dependent conditions with nested `if`. Beyond that, switch to `case`: use `case false of` for a chain of guards where every condition must hold, letting control fall past `end` when all of them pass; use `case true of` for first-match dispatch, where each later probe runs only if the earlier ones did not match. Comma-separate conditions into one value set only when every one of them is a pure, order-independent test with no side effect โ€” a field comparison, an enum check, a bound test โ€” so it makes no difference whether AL evaluates all of them or stops early; grouping these costs nothing and removes the repeated action. A condition that guards another, or that carries a side effect or a cost of its own โ€” a `Get`, a `Find`, a procedure call โ€” keeps its own value set, placed immediately after the value set it depends on, so the code relies only on the ordering the documentation actually states. A value set needs no parentheses around a comparison, unlike an operand of `and` or `or`: the AL operator hierarchy places `and` and `or` above the comparison operators, so parentheses are mandatory there and the chain fills up with them. This keeps every condition at one indentation level, makes evaluation order explicit rather than implied by nesting, and preserves the stop-at-first-match behaviour it relies on. It also aligns with the AL programming convention that more than two alternatives belong in a `case` statement rather than an `if-then-else`. -See sample: `case-true-of-for-long-condition-chains.good.al`. +See sample: [`case-true-of-for-long-condition-chains.good.al`](case-true-of-for-long-condition-chains.good.al). ## Anti Pattern An `if` ladder four or more levels deep whose only purpose is sequencing guards. Detection: a chain of nested `if` statements with no `else`, each condition guarding the one below it, terminating in a single action or `exit`; or the same `exit`/`error` duplicated at every level of such a nested chain, purely to escape it. The second, worse form is collapsing that ladder into one `and` chain to escape the nesting โ€” that trades indentation for a real defect, because the operands are still all evaluated. A third, subtler form is over-applying the comma-grouping itself: putting a guard and the condition it protects โ€” for example `Item.Get(...)` and a read of a field on that same record โ€” into one comma-separated value set. That relies on an evaluation order within a single value set that the documentation does not state; keep them in separate value sets instead. Reach for `case` over nested `if` or a collapsed `and` chain, and keep order-dependent conditions in their own value sets within it. -See sample: `case-true-of-for-long-condition-chains.bad.al`. +See sample: [`case-true-of-for-long-condition-chains.bad.al`](case-true-of-for-long-condition-chains.bad.al). ## See also diff --git a/microsoft/knowledge/performance/changecompany-in-loop-drops-caches.md b/microsoft/knowledge/performance/changecompany-in-loop-drops-caches.md index fddef76..5946e42 100644 --- a/microsoft/knowledge/performance/changecompany-in-loop-drops-caches.md +++ b/microsoft/knowledge/performance/changecompany-in-loop-drops-caches.md @@ -19,10 +19,10 @@ application-area: [all] Group work by company. Call `ChangeCompany` once per distinct company, then `FindSet`/`Get` that company's rows. If the record variable is reused afterward, call `ChangeCompany()` without a company name to redirect it back to the current company. -See sample: `changecompany-in-loop-drops-caches.good.al`. +See sample: [`changecompany-in-loop-drops-caches.good.al`](changecompany-in-loop-drops-caches.good.al). ## Anti Pattern `repeat Rec.ChangeCompany(Buffer.Company); Rec.Get(Buffer."No."); until Buffer.Next() = 0` when `Buffer` is not ordered by company, or even when it is โ€” if `ChangeCompany` still runs every row. The signal is `ChangeCompany` inside `repeat`/`while` keyed by a document line rather than by a company loop. -See sample: `changecompany-in-loop-drops-caches.bad.al`. +See sample: [`changecompany-in-loop-drops-caches.bad.al`](changecompany-in-loop-drops-caches.bad.al). diff --git a/microsoft/knowledge/performance/choose-maintainsiftindex-by-read-write-ratio.md b/microsoft/knowledge/performance/choose-maintainsiftindex-by-read-write-ratio.md index 3261a2c..1a60051 100644 --- a/microsoft/knowledge/performance/choose-maintainsiftindex-by-read-write-ratio.md +++ b/microsoft/knowledge/performance/choose-maintainsiftindex-by-read-write-ratio.md @@ -19,7 +19,7 @@ application-area: [all] Measure aggregate-read latency and write cost under realistic filters and volumes. Keep `MaintainSIFTIndex = true` when the maintained aggregate materially benefits frequent `CalcSums` or FlowField reads. Consider `false` when writes dominate and the less-frequent aggregate reads can tolerate calculation from the base table. -See sample: `choose-maintainsiftindex-by-read-write-ratio.good.al`. +See sample: [`choose-maintainsiftindex-by-read-write-ratio.good.al`](choose-maintainsiftindex-by-read-write-ratio.good.al). ## Anti Pattern diff --git a/microsoft/knowledge/performance/codeunit-run-as-atomic-sub-operation.md b/microsoft/knowledge/performance/codeunit-run-as-atomic-sub-operation.md index 89777f1..8abdd20 100644 --- a/microsoft/knowledge/performance/codeunit-run-as-atomic-sub-operation.md +++ b/microsoft/knowledge/performance/codeunit-run-as-atomic-sub-operation.md @@ -17,10 +17,10 @@ application-area: [all] When a piece of work must either complete fully or have no effect, put it in its own codeunit and invoke it via `Codeunit.Run`, capturing the return. Use `if not Codeunit.Run(X) then Error(...)` to abort and unwind; use the plain boolean branch to react to failure without aborting the caller. This replaces the SQL-style `BEGIN TRAN / COMMIT / ROLLBACK` habit with a pattern the AL runtime implements natively. Do not confuse `Codeunit.Run` with `[TryFunction]` โ€” both catch errors, but only `Codeunit.Run` rolls back database changes on failure (see `use-tryfunction-for-error-catching-not-rollback.md`). Note that if the caller is already in a write transaction, the platform requires a `Commit()` before `Codeunit.Run` โ€” the sub-operation cannot nest inside an open transaction (see `codeunit-run-requires-prior-commit-inside-transaction.md`). -See sample: `codeunit-run-as-atomic-sub-operation.good.al`. +See sample: [`codeunit-run-as-atomic-sub-operation.good.al`](codeunit-run-as-atomic-sub-operation.good.al). ## Anti Pattern Inlining the work in the caller and sprinkling `Commit()` to simulate sub-transaction boundaries. The caller's enclosing transaction is fused to the sub-work; any Commit between checkpoints survives subsequent errors, and any errors after a Commit cannot be cleanly unwound. Per-row Commits (see `avoid-commit-inside-loops.md`) are a frequent symptom. -See sample: `codeunit-run-as-atomic-sub-operation.bad.al`. +See sample: [`codeunit-run-as-atomic-sub-operation.bad.al`](codeunit-run-as-atomic-sub-operation.bad.al). diff --git a/microsoft/knowledge/performance/codeunit-run-requires-prior-commit-inside-transaction.md b/microsoft/knowledge/performance/codeunit-run-requires-prior-commit-inside-transaction.md index a07520f..936ae5b 100644 --- a/microsoft/knowledge/performance/codeunit-run-requires-prior-commit-inside-transaction.md +++ b/microsoft/knowledge/performance/codeunit-run-requires-prior-commit-inside-transaction.md @@ -17,10 +17,10 @@ application-area: [all] For the `Codeunit.Run` atomic-sub-operation pattern (see `codeunit-run-as-atomic-sub-operation.md`) to work in a loop, keep the outer scope **read-only**. Move per-iteration writes โ€” progress updates, logging, audit entries โ€” into the sub-codeunit so they commit or roll back together with the per-item work. If logging must live outside the atomic boundary, defer it: collect failure info in memory during the loop (a `List of [Text]`, a temporary record, local variables) and write it in one pass after the loop ends, when no outer write transaction is open. -See sample: `codeunit-run-requires-prior-commit-inside-transaction.good.al`. +See sample: [`codeunit-run-requires-prior-commit-inside-transaction.good.al`](codeunit-run-requires-prior-commit-inside-transaction.good.al). ## Anti Pattern Inserting `Commit()` before each `Codeunit.Run` to silence the runtime error. The error goes away, but the outer scope now commits per iteration โ€” the behavior `avoid-commit-inside-loops.md` exists to warn against. Attempting to silence the implicit commit inside the sub-codeunit with `[CommitBehavior(CommitBehavior::Ignore)]` also fails: the attribute does not apply to `Codeunit.Run`'s implicit commit. Conditioning the Commit on `Database.IsInWriteTransaction()` (runtime 11.0+) is another version of the same trap โ€” the method has legitimate uses for diagnostics and library code that genuinely cannot control its caller, but branching production flow on runtime transaction state typically signals unclear ownership that would be better fixed by restructuring the caller so transaction state is predictable. -See sample: `codeunit-run-requires-prior-commit-inside-transaction.bad.al`. +See sample: [`codeunit-run-requires-prior-commit-inside-transaction.bad.al`](codeunit-run-requires-prior-commit-inside-transaction.bad.al). diff --git a/microsoft/knowledge/performance/dataaccessintent-readonly-on-analytical-objects.md b/microsoft/knowledge/performance/dataaccessintent-readonly-on-analytical-objects.md index 89ee55e..943242d 100644 --- a/microsoft/knowledge/performance/dataaccessintent-readonly-on-analytical-objects.md +++ b/microsoft/knowledge/performance/dataaccessintent-readonly-on-analytical-objects.md @@ -19,10 +19,10 @@ application-area: [all] On report objects and `PageType = API` pages with `Editable = false` that never write, set `DataAccessIntent = ReadOnly`. For query objects, set it when the query is consumed via OData or an API endpoint. Keep the default on objects that insert, modify, or call a write codeunit from a processing-only report. -See sample: `dataaccessintent-readonly-on-analytical-objects.good.al`. +See sample: [`dataaccessintent-readonly-on-analytical-objects.good.al`](dataaccessintent-readonly-on-analytical-objects.good.al). ## Anti Pattern A listing report or API query with no `DataAccessIntent` that scans G/L or sales lines. The object is read-only in practice and still loads the primary. -See sample: `dataaccessintent-readonly-on-analytical-objects.bad.al`. +See sample: [`dataaccessintent-readonly-on-analytical-objects.bad.al`](dataaccessintent-readonly-on-analytical-objects.bad.al). diff --git a/microsoft/knowledge/performance/do-not-locktable-in-read-only-procedure.md b/microsoft/knowledge/performance/do-not-locktable-in-read-only-procedure.md index f25af2e..2580144 100644 --- a/microsoft/knowledge/performance/do-not-locktable-in-read-only-procedure.md +++ b/microsoft/knowledge/performance/do-not-locktable-in-read-only-procedure.md @@ -17,10 +17,10 @@ application-area: [all] Reserve `LockTable` for the read directly before a `Modify`, `Insert`, or `Delete` that depends on the read value. If a helper is sometimes called for reading and sometimes for writing, split it into separate read and write paths and call `LockTable` only on the write path. For read-only existence checks or lookups, the right primitive is `ReadIsolation` (see `prefer-readisolation-over-locktable-for-reads.md`). -See sample: `do-not-locktable-in-read-only-procedure.good.al`. +See sample: [`do-not-locktable-in-read-only-procedure.good.al`](do-not-locktable-in-read-only-procedure.good.al). ## Anti Pattern A pure getter that opens with `Rec.LockTable();`. Every caller's transaction now acquires `UPDLOCK` on that table for every subsequent read until commit. The contention shows up as blocking on unrelated sessions whose own code path looks innocent โ€” the locker is invisible to the blocked reader. -See sample: `do-not-locktable-in-read-only-procedure.bad.al`. +See sample: [`do-not-locktable-in-read-only-procedure.bad.al`](do-not-locktable-in-read-only-procedure.bad.al). diff --git a/microsoft/knowledge/performance/do-not-modify-in-onaftergetrecord.md b/microsoft/knowledge/performance/do-not-modify-in-onaftergetrecord.md index 9de0903..a30ada9 100644 --- a/microsoft/knowledge/performance/do-not-modify-in-onaftergetrecord.md +++ b/microsoft/knowledge/performance/do-not-modify-in-onaftergetrecord.md @@ -17,10 +17,10 @@ A list page's `OnAfterGetRecord` fires once per visible row, every time the user When the trigger needs to compute display-only state per row, write the result into a page variable (a global on the page object) rather than back to the database. Reserve `Modify` for triggers that fire on an explicit user action โ€” `OnAction`, validation triggers, `OnQueryClosePage` โ€” where one action maps to one write. -See sample: `do-not-modify-in-onaftergetrecord.good.al`. +See sample: [`do-not-modify-in-onaftergetrecord.good.al`](do-not-modify-in-onaftergetrecord.good.al). ## Anti Pattern `trigger OnAfterGetRecord() begin Rec."Warning Flag" := CalcWarning(); Rec.Modify(); end;` โ€” on a list page over a moderately sized table, scrolling through fifty rows produces fifty writes. The page feels slow, the table accumulates churn, and the warning flag โ€” which is recomputed on every refresh anyway โ€” never needed persistence. -See sample: `do-not-modify-in-onaftergetrecord.bad.al`. +See sample: [`do-not-modify-in-onaftergetrecord.bad.al`](do-not-modify-in-onaftergetrecord.bad.al). diff --git a/microsoft/knowledge/performance/do-not-remove-sourcetabletemporary-from-api-page.md b/microsoft/knowledge/performance/do-not-remove-sourcetabletemporary-from-api-page.md index a7215be..ef4b137 100644 --- a/microsoft/knowledge/performance/do-not-remove-sourcetabletemporary-from-api-page.md +++ b/microsoft/knowledge/performance/do-not-remove-sourcetabletemporary-from-api-page.md @@ -17,10 +17,10 @@ application-area: [all] If a page or record was declared temporary on purpose โ€” to buffer payloads, accept synthetic rows, or expose computed data through an API surface without persisting it โ€” keep it temporary. When removing the property looks necessary, audit the call sites first: a temporary API page is often consumed by integrations that issue many calls per minute, and the round-trip cost is paid per call. If persistence is genuinely required, weigh storage and lock cost against alternatives (a regular table the API page reads from, an event-driven write). -See sample: `do-not-remove-sourcetabletemporary-from-api-page.good.al`. +See sample: [`do-not-remove-sourcetabletemporary-from-api-page.good.al`](do-not-remove-sourcetabletemporary-from-api-page.good.al). ## Anti Pattern Dropping `SourceTableTemporary = true` from an API page to "simplify" it, without revisiting the access pattern. The page begins issuing real SQL on every request; locks now contend with other writers; bulk integrations slow proportionally. The same trap exists for a record that was `TableType = Temporary` and gets demoted to a persistent table to make a debugger view easier. -See sample: `do-not-remove-sourcetabletemporary-from-api-page.bad.al`. +See sample: [`do-not-remove-sourcetabletemporary-from-api-page.bad.al`](do-not-remove-sourcetabletemporary-from-api-page.bad.al). diff --git a/microsoft/knowledge/performance/findset-true-applies-updlock-on-read.md b/microsoft/knowledge/performance/findset-true-applies-updlock-on-read.md index 48a1deb..3ab780a 100644 --- a/microsoft/knowledge/performance/findset-true-applies-updlock-on-read.md +++ b/microsoft/knowledge/performance/findset-true-applies-updlock-on-read.md @@ -17,10 +17,10 @@ application-area: [all] Use `FindSet(true)` only when the loop body genuinely modifies the iterated rows; use `FindSet()` (or `FindSet(false)`) when the loop only reads. Do not write `FindSet(true, true)` or `FindSet(true, false)` โ€” the two-parameter form is the obsolete signature. -See sample: `findset-true-applies-updlock-on-read.good.al`. +See sample: [`findset-true-applies-updlock-on-read.good.al`](findset-true-applies-updlock-on-read.good.al). ## Anti Pattern `FindSet(true)` on a loop that does not modify the iterated rows takes an `UpdLock` the work does not need; competing readers and writers stall against a lock the loop never uses. The mirror anti-pattern is `FindSet()` (no parameter) on a loop that *does* modify each row โ€” the read takes a shared lock, the `Modify` then needs to upgrade, and the gap between them is a deadlock candidate. -See sample: `findset-true-applies-updlock-on-read.bad.al`. +See sample: [`findset-true-applies-updlock-on-read.bad.al`](findset-true-applies-updlock-on-read.bad.al). diff --git a/microsoft/knowledge/performance/flowfield-source-key-needs-sumindexfields.md b/microsoft/knowledge/performance/flowfield-source-key-needs-sumindexfields.md index 3a63613..fdedc42 100644 --- a/microsoft/knowledge/performance/flowfield-source-key-needs-sumindexfields.md +++ b/microsoft/knowledge/performance/flowfield-source-key-needs-sumindexfields.md @@ -17,10 +17,10 @@ A FlowField is computed by SQL on demand. CodeCop AA0232 โ€” "FlowFields should When introducing or changing a FlowField, walk the `CalcFormula`'s `WHERE` clause field by field and verify the source table has a key whose key fields cover those filters, with the aggregated field in `SumIndexFields`. The same applies when the destination side of the FlowField filter is a list-page column: the page filter triggers the FlowField on every visible row, and only SIFT keeps that affordable. -See sample: `flowfield-source-key-needs-sumindexfields.good.al`. +See sample: [`flowfield-source-key-needs-sumindexfields.good.al`](flowfield-source-key-needs-sumindexfields.good.al). ## Anti Pattern A `sum` FlowField against a large source table with no matching SIFT key. Each calculation aggregates rows directly; on a ledger-sized source the FlowField becomes the slowest column on every page that displays it. Pointing an existing FlowField's `CalcFormula` at a larger source table without verifying the new source's keys is the same trap a step removed โ€” the upstream review guidance flags it as "CalcFormula changed to larger source table". -See sample: `flowfield-source-key-needs-sumindexfields.bad.al`. +See sample: [`flowfield-source-key-needs-sumindexfields.bad.al`](flowfield-source-key-needs-sumindexfields.bad.al). diff --git a/microsoft/knowledge/performance/guard-event-subscribers-before-db-call.md b/microsoft/knowledge/performance/guard-event-subscribers-before-db-call.md index c466ffe..d56c716 100644 --- a/microsoft/knowledge/performance/guard-event-subscribers-before-db-call.md +++ b/microsoft/knowledge/performance/guard-event-subscribers-before-db-call.md @@ -17,10 +17,10 @@ Event subscribers fire on every event matching their signature โ€” for `OnAfterV Open the subscriber with an in-memory predicate that filters out the calls the subscriber does not handle โ€” record type, document type, status, parameter-passed flags. Only after the cheap guard passes should the body issue a database call, and only with `SetLoadFields` for the columns the body actually reads. -See sample: `guard-event-subscribers-before-db-call.good.al`. +See sample: [`guard-event-subscribers-before-db-call.good.al`](guard-event-subscribers-before-db-call.good.al). ## Anti Pattern `[EventSubscriber(...'OnAfterValidateEvent', 'Quantity', ...)] local procedure ... var Item: Record Item; begin Item.Get(Rec."No."); if Item.HasCustomPricing() then ...;` โ€” `Item.Get` runs on every quantity change, including changes to lines whose `Type` is not `Item`. A pre-check `if Rec.Type <> Rec.Type::Item then exit;` ahead of the `Get` removes most of the calls. -See sample: `guard-event-subscribers-before-db-call.bad.al`. +See sample: [`guard-event-subscribers-before-db-call.bad.al`](guard-event-subscribers-before-db-call.bad.al). diff --git a/microsoft/knowledge/performance/guiallowed-guard-on-pages-used-as-odata.md b/microsoft/knowledge/performance/guiallowed-guard-on-pages-used-as-odata.md index 01dc1f8..bf0bb7d 100644 --- a/microsoft/knowledge/performance/guiallowed-guard-on-pages-used-as-odata.md +++ b/microsoft/knowledge/performance/guiallowed-guard-on-pages-used-as-odata.md @@ -19,10 +19,10 @@ Pages exposed as OData, including Edit in Excel, still run AL page triggers for Wrap UI-only work โ€” FactBox refresh, notifications, defaulting that is not part of the web-service contract โ€” in `if GuiAllowed then`. Keep the OData path to field values the API actually returns. -See sample: `guiallowed-guard-on-pages-used-as-odata.good.al`. +See sample: [`guiallowed-guard-on-pages-used-as-odata.good.al`](guiallowed-guard-on-pages-used-as-odata.good.al). ## Anti Pattern Unconditional FactBox or calculation logic in `OnAfterGetRecord` / `OnAfterGetCurrRecord` on a page that is published as a web service or used with Edit in Excel. The signal is trigger work that calls `CurrPage` parts or extra queries without a `GuiAllowed` guard. -See sample: `guiallowed-guard-on-pages-used-as-odata.bad.al`. +See sample: [`guiallowed-guard-on-pages-used-as-odata.bad.al`](guiallowed-guard-on-pages-used-as-odata.bad.al). diff --git a/microsoft/knowledge/performance/hidden-flowfields-still-calculate-before-bc26-opt-in.md b/microsoft/knowledge/performance/hidden-flowfields-still-calculate-before-bc26-opt-in.md index b03ac51..7db17d3 100644 --- a/microsoft/knowledge/performance/hidden-flowfields-still-calculate-before-bc26-opt-in.md +++ b/microsoft/knowledge/performance/hidden-flowfields-still-calculate-before-bc26-opt-in.md @@ -17,10 +17,10 @@ By default, a FlowField used directly as a page control's source is calculated w On BC 26 and later, enable and verify the visible-only FlowField feature before relying on `Visible` to suppress calculation. When the target environment does not guarantee that option, avoid binding an expensive FlowField directly to a usually-hidden control: calculate it only in the branch that displays it and bind the page control to a variable. Do not flag a hidden FlowField when the v26 feature is known to be enabled or the FlowField is cheap and intentionally preloaded. -See sample: `hidden-flowfields-still-calculate-before-bc26-opt-in.good.al`. +See sample: [`hidden-flowfields-still-calculate-before-bc26-opt-in.good.al`](hidden-flowfields-still-calculate-before-bc26-opt-in.good.al). ## Anti Pattern Adding a costly Sum or Lookup FlowField to a page with `Visible = SomeRareMode` and assuming the hidden state prevents its query on all supported versions. The review signal is the direct FlowField source plus conditional or false visibility, not visibility alone. -See sample: `hidden-flowfields-still-calculate-before-bc26-opt-in.bad.al`. +See sample: [`hidden-flowfields-still-calculate-before-bc26-opt-in.bad.al`](hidden-flowfields-still-calculate-before-bc26-opt-in.bad.al). diff --git a/microsoft/knowledge/performance/httpclient-inside-write-transaction-holds-locks.md b/microsoft/knowledge/performance/httpclient-inside-write-transaction-holds-locks.md index 4c1d500..21a2c04 100644 --- a/microsoft/knowledge/performance/httpclient-inside-write-transaction-holds-locks.md +++ b/microsoft/knowledge/performance/httpclient-inside-write-transaction-holds-locks.md @@ -21,10 +21,10 @@ Defer the HTTP call to a separate session. When the external operation must corr A directly created scheduled task is suitable only when its work is independent of the caller's commit. An immediately ready task can run concurrently with the caller, so it must not assume that the caller's writes are already committed. Do **not** use `Commit()` as a general remedy: it irrevocably commits all prior writes in the current transaction, so any subsequent failure cannot roll them back. `Commit()` is appropriate only at top-level entry points where partial persistence is intentional and understood. -See sample: `httpclient-inside-write-transaction-holds-locks.good.al`. +See sample: [`httpclient-inside-write-transaction-holds-locks.good.al`](httpclient-inside-write-transaction-holds-locks.good.al). ## Anti Pattern `Modify`/`Insert` followed by `HttpClient` in the same procedure with no `Commit` between them. Detection signal: any `HttpClient` use after a write on the same execution path, especially in posting, page actions, or subscribers. -See sample: `httpclient-inside-write-transaction-holds-locks.bad.al`. +See sample: [`httpclient-inside-write-transaction-holds-locks.bad.al`](httpclient-inside-write-transaction-holds-locks.bad.al). diff --git a/microsoft/knowledge/performance/isempty-before-findset-is-extra-round-trip.md b/microsoft/knowledge/performance/isempty-before-findset-is-extra-round-trip.md index a8bf187..c88cdfe 100644 --- a/microsoft/knowledge/performance/isempty-before-findset-is-extra-round-trip.md +++ b/microsoft/knowledge/performance/isempty-before-findset-is-extra-round-trip.md @@ -19,10 +19,10 @@ application-area: [all] When the body iterates, open with `if Rec.FindSet() then repeat ... until Next() = 0`. Do not flag a bare `FindSet` loop as missing an `IsEmpty` precondition. Reserve `IsEmpty` for branches that never materialize the row set. -See sample: `isempty-before-findset-is-extra-round-trip.good.al`. +See sample: [`isempty-before-findset-is-extra-round-trip.good.al`](isempty-before-findset-is-extra-round-trip.good.al). ## Anti Pattern `if not Rec.IsEmpty() then if Rec.FindSet() then repeat`. Also a false-positive review comment that asks to add that guard. The second read does not avoid the first; it duplicates it. -See sample: `isempty-before-findset-is-extra-round-trip.bad.al`. +See sample: [`isempty-before-findset-is-extra-round-trip.bad.al`](isempty-before-findset-is-extra-round-trip.bad.al). diff --git a/microsoft/knowledge/performance/load-common-fields-before-branching-on-case.md b/microsoft/knowledge/performance/load-common-fields-before-branching-on-case.md index 257b0fb..6a48663 100644 --- a/microsoft/knowledge/performance/load-common-fields-before-branching-on-case.md +++ b/microsoft/knowledge/performance/load-common-fields-before-branching-on-case.md @@ -19,10 +19,10 @@ When a known input determines which fields a subsequent record read will use, a Call `SetLoadFields` with the common fields. In each branch, call `AddLoadFields` with that branch's normal fields and then perform the record read. This applies only when the discriminator is known before the read; branching on a field from an already-loaded row is too late to tailor that row's initial SQL projection. -See sample: `load-common-fields-before-branching-on-case.good.al`. +See sample: [`load-common-fields-before-branching-on-case.good.al`](load-common-fields-before-branching-on-case.good.al). ## Anti Pattern A single top-level `SetLoadFields` enumerating every branch's fields, or a branch-local `SetLoadFields` that accidentally discards the common selection. Both make the declared load plan differ from the fields the selected path actually uses. -See sample: `load-common-fields-before-branching-on-case.bad.al`. +See sample: [`load-common-fields-before-branching-on-case.bad.al`](load-common-fields-before-branching-on-case.bad.al). diff --git a/microsoft/knowledge/performance/load-only-primary-key-fields-for-reference-work.md b/microsoft/knowledge/performance/load-only-primary-key-fields-for-reference-work.md index 533ab8c..4e88b83 100644 --- a/microsoft/knowledge/performance/load-only-primary-key-fields-for-reference-work.md +++ b/microsoft/knowledge/performance/load-only-primary-key-fields-for-reference-work.md @@ -19,10 +19,10 @@ Work that uses a record only for its identity โ€” passing it to another procedur When the iterating code's body touches only primary key fields (or passes the record to another procedure that will apply its own `SetLoadFields`), declare `SetLoadFields` with just the primary key fields before applying filters and calling `FindSet`. Callers downstream that need more fields issue their own `Get` or extend the load explicitly. -See sample: `load-only-primary-key-fields-for-reference-work.good.al`. +See sample: [`load-only-primary-key-fields-for-reference-work.good.al`](load-only-primary-key-fields-for-reference-work.good.al). ## Anti Pattern Using the default full-record load in loops whose body only reads the primary key, or forwards the record to another codeunit that immediately re-queries. The non-key payload is fetched across the wire and held in memory for the duration of the loop, then discarded unread. -See sample: `load-only-primary-key-fields-for-reference-work.bad.al`. +See sample: [`load-only-primary-key-fields-for-reference-work.bad.al`](load-only-primary-key-fields-for-reference-work.bad.al). diff --git a/microsoft/knowledge/performance/maintainsqlindex-false-breaks-flowfield-sift.md b/microsoft/knowledge/performance/maintainsqlindex-false-breaks-flowfield-sift.md index e540859..8898e44 100644 --- a/microsoft/knowledge/performance/maintainsqlindex-false-breaks-flowfield-sift.md +++ b/microsoft/knowledge/performance/maintainsqlindex-false-breaks-flowfield-sift.md @@ -17,7 +17,7 @@ application-area: [all] When changing a key property to `MaintainSQLIndex = false`, find every FlowField whose `CalcFormula` filters on that key and verify another key covers the same fields. When adding a FlowField whose source table has only a `MaintainSQLIndex = false` key for its filter columns, add a fully-indexed key (or accept that the FlowField cannot ride SIFT and reshape the design โ€” see `flowfield-source-key-needs-sumindexfields.md`). -See sample: `maintainsqlindex-false-breaks-flowfield-sift.bad.al`. +See sample: [`maintainsqlindex-false-breaks-flowfield-sift.bad.al`](maintainsqlindex-false-breaks-flowfield-sift.bad.al). ## Anti Pattern diff --git a/microsoft/knowledge/performance/oncompanyopen-subscribers-must-not-do-io.md b/microsoft/knowledge/performance/oncompanyopen-subscribers-must-not-do-io.md index 3cb8459..e8da1c5 100644 --- a/microsoft/knowledge/performance/oncompanyopen-subscribers-must-not-do-io.md +++ b/microsoft/knowledge/performance/oncompanyopen-subscribers-must-not-do-io.md @@ -19,10 +19,10 @@ application-area: [all] Keep company-open subscribers to cheap in-memory work: set a flag, enqueue a job-queue entry, or `TaskScheduler.CreateTask`. Perform HTTP and large SQL after the session is running, in that background work. -See sample: `oncompanyopen-subscribers-must-not-do-io.good.al`. +See sample: [`oncompanyopen-subscribers-must-not-do-io.good.al`](oncompanyopen-subscribers-must-not-do-io.good.al). ## Anti Pattern An `OnAfterLogin` / `OnCompanyOpenCompleted` subscriber that calls `HttpClient` or scans a ledger. Detection signal: `HttpClient`, `FindSet`, or `CalcFields` inside a subscriber bound to those events. -See sample: `oncompanyopen-subscribers-must-not-do-io.bad.al`. +See sample: [`oncompanyopen-subscribers-must-not-do-io.bad.al`](oncompanyopen-subscribers-must-not-do-io.bad.al). diff --git a/microsoft/knowledge/performance/order-case-branches-by-frequency.md b/microsoft/knowledge/performance/order-case-branches-by-frequency.md index 1634475..dd327e9 100644 --- a/microsoft/knowledge/performance/order-case-branches-by-frequency.md +++ b/microsoft/knowledge/performance/order-case-branches-by-frequency.md @@ -19,10 +19,10 @@ AL documentation does not guarantee that a `case` statement uses a linear compar After profiling confirms the comparison path matters and the runtime frequency is known, list common branches first without changing the set of handled values, fallback behavior, or branch bodies. -See sample: `order-case-branches-by-frequency.good.al`. +See sample: [`order-case-branches-by-frequency.good.al`](order-case-branches-by-frequency.good.al). ## Anti Pattern Reordering branches based on assumed frequency without profiling, or changing an `else` arm or handled value while making the optimization. The good and bad forms must differ only in branch order. -See sample: `order-case-branches-by-frequency.bad.al`. +See sample: [`order-case-branches-by-frequency.bad.al`](order-case-branches-by-frequency.bad.al). diff --git a/microsoft/knowledge/performance/page-background-tasks-for-expensive-cues.md b/microsoft/knowledge/performance/page-background-tasks-for-expensive-cues.md index 48fae1c..8b2bb39 100644 --- a/microsoft/knowledge/performance/page-background-tasks-for-expensive-cues.md +++ b/microsoft/knowledge/performance/page-background-tasks-for-expensive-cues.md @@ -19,10 +19,10 @@ Role-center cues and CardPart totals that run `CalcFields`, scans, or HTTP on th Bind the cue to a page variable, enqueue a read-only calculation from `OnAfterGetCurrRecord` (not `OnAfterGetRecord` on a list), and apply the result in `OnPageBackgroundTaskCompleted`. Show a placeholder until then. -See sample: `page-background-tasks-for-expensive-cues.good.al`. +See sample: [`page-background-tasks-for-expensive-cues.good.al`](page-background-tasks-for-expensive-cues.good.al). ## Anti Pattern `CalcFields` or a ledger `Count` in `OnOpenPage` / `OnAfterGetCurrRecord` of a CueGroup CardPart with no background task. The Role Center waits on SQL the user may never look at. -See sample: `page-background-tasks-for-expensive-cues.bad.al`. +See sample: [`page-background-tasks-for-expensive-cues.bad.al`](page-background-tasks-for-expensive-cues.bad.al). diff --git a/microsoft/knowledge/performance/pair-findset-with-next-loop.md b/microsoft/knowledge/performance/pair-findset-with-next-loop.md index 80f855c..12d3bd4 100644 --- a/microsoft/knowledge/performance/pair-findset-with-next-loop.md +++ b/microsoft/knowledge/performance/pair-findset-with-next-loop.md @@ -17,10 +17,10 @@ Two CodeCop rules carve out the loop pattern. AA0181 says `FindSet()`/`Find()` " When the body executes `repeat ... until Next() = 0;`, open the iteration with `FindSet()`. When the body needs one record and does not call `Next`, use `FindFirst`, `FindLast`, or โ€” if the full primary key is known โ€” `Get` (see `use-get-instead-of-findfirst-on-full-primary-key.md`). The choice is per call site, not a global preference. -See sample: `pair-findset-with-next-loop.good.al`. +See sample: [`pair-findset-with-next-loop.good.al`](pair-findset-with-next-loop.good.al). ## Anti Pattern `if Customer.FindFirst() then repeat ... until Customer.Next() = 0;` โ€” AA0233 flags this. The single-row API does not prepare the runtime for iteration, so the loop pays a cost the FindSet path does not. The mirror anti-pattern is calling `FindSet` to read a single record (see `use-isempty-for-existence-check.md` when only existence is required). -See sample: `pair-findset-with-next-loop.bad.al`. +See sample: [`pair-findset-with-next-loop.bad.al`](pair-findset-with-next-loop.bad.al). diff --git a/microsoft/knowledge/performance/pass-false-to-insert-when-trigger-not-needed.md b/microsoft/knowledge/performance/pass-false-to-insert-when-trigger-not-needed.md index 45214b8..4fc83c4 100644 --- a/microsoft/knowledge/performance/pass-false-to-insert-when-trigger-not-needed.md +++ b/microsoft/knowledge/performance/pass-false-to-insert-when-trigger-not-needed.md @@ -17,7 +17,7 @@ application-area: [all] Reach for the `(false)` form when the calling code already enforces the invariants the trigger would, or when the trigger is empty for the current table/extension. Use `(true)` when the trigger does work the caller depends on (number-series allocation, validation, cascading writes). Decide per call, not by code style: a default of "always `true`" makes bulk writes pay for triggers they did not need, and a default of "always `false`" silently skips validation the trigger was put there to enforce. -See sample: `pass-false-to-insert-when-trigger-not-needed.good.al`. +See sample: [`pass-false-to-insert-when-trigger-not-needed.good.al`](pass-false-to-insert-when-trigger-not-needed.good.al). ## Anti Pattern diff --git a/microsoft/knowledge/performance/pass-var-record-to-preserve-partial-load-enumerator.md b/microsoft/knowledge/performance/pass-var-record-to-preserve-partial-load-enumerator.md index ce4c4bc..726ec41 100644 --- a/microsoft/knowledge/performance/pass-var-record-to-preserve-partial-load-enumerator.md +++ b/microsoft/knowledge/performance/pass-var-record-to-preserve-partial-load-enumerator.md @@ -19,10 +19,10 @@ A `FindSet`/`Next` loop builds an enumerator from the fields selected for load. Helpers that read extra fields on an in-flight iterator must take the record as `var`, or the caller must `AddLoadFields` those fields before the loop. Prefer declaring the extra fields up front so no JIT is needed. -See sample: `pass-var-record-to-preserve-partial-load-enumerator.good.al`. +See sample: [`pass-var-record-to-preserve-partial-load-enumerator.good.al`](pass-var-record-to-preserve-partial-load-enumerator.good.al). ## Anti Pattern A `SetLoadFields` loop that passes the iterator by value into a helper which then reads a field that was not loaded. The first row pays one JIT; every subsequent row pays it again because the enumerator never learned the extra field. -See sample: `pass-var-record-to-preserve-partial-load-enumerator.bad.al`. +See sample: [`pass-var-record-to-preserve-partial-load-enumerator.bad.al`](pass-var-record-to-preserve-partial-load-enumerator.bad.al). diff --git a/microsoft/knowledge/performance/prefer-modifyall-over-per-row-modify.md b/microsoft/knowledge/performance/prefer-modifyall-over-per-row-modify.md index 024aae1..ad7a6cf 100644 --- a/microsoft/knowledge/performance/prefer-modifyall-over-per-row-modify.md +++ b/microsoft/knowledge/performance/prefer-modifyall-over-per-row-modify.md @@ -17,10 +17,10 @@ application-area: [all] Use `ModifyAll` when the loop directly assigns the same value, does not call `Validate`, needs no per-row calculation, and does not depend on `OnModify` unless the equivalent `RunTrigger` value is supplied. Check whether table trigger code, related subscribers, security filtering, `Media`/`MediaSet`, or companion fields force row-by-row fallback (see `triggers-and-media-field-regress-modifyall.md`). A visible loop for progress UX is acceptable only when evidence shows the equivalent bulk call already executes as individual operations and the loop preserves trigger and business semantics. -See sample: `prefer-modifyall-over-per-row-modify.good.al`. +See sample: [`prefer-modifyall-over-per-row-modify.good.al`](prefer-modifyall-over-per-row-modify.good.al). ## Anti Pattern A loop that only assigns a constant and calls `Modify(false)` on a field with no validation side effects or bulk fallback condition. A progress dialog alone does not exempt this loop. Conversely, replacing `Validate(Field, Value); Modify(true)` with `ModifyAll(Field, Value)` is also an anti-pattern because it silently drops field validation and may drop table-trigger behavior. -See sample: `prefer-modifyall-over-per-row-modify.bad.al`. +See sample: [`prefer-modifyall-over-per-row-modify.bad.al`](prefer-modifyall-over-per-row-modify.bad.al). diff --git a/microsoft/knowledge/performance/prefer-readisolation-over-locktable-for-reads.md b/microsoft/knowledge/performance/prefer-readisolation-over-locktable-for-reads.md index f798636..a82ec8b 100644 --- a/microsoft/knowledge/performance/prefer-readisolation-over-locktable-for-reads.md +++ b/microsoft/knowledge/performance/prefer-readisolation-over-locktable-for-reads.md @@ -17,10 +17,10 @@ Without read scale-out, `LockTable` causes subsequent reads of that table in the For a read-only operation that specifically requires committed data, set `Rec.ReadIsolation := IsolationLevel::ReadCommitted` immediately before the read. If the default isolation is sufficient, set neither property. `ReadCommitted` can still block behind writers and does not guarantee that repeated reads stay unchanged; use the isolation level required by the operation. Reserve update locks for read-before-write logic, not read-only helpers. -See sample: `prefer-readisolation-over-locktable-for-reads.good.al`. +See sample: [`prefer-readisolation-over-locktable-for-reads.good.al`](prefer-readisolation-over-locktable-for-reads.good.al). ## Anti Pattern `Rec.LockTable();` at the top of a helper that only reads, perhaps to "make sure the read is consistent". It takes stronger isolation than the helper needs and changes later reads of that table in the surrounding transaction or read-scale-out session. -See sample: `prefer-readisolation-over-locktable-for-reads.bad.al`. +See sample: [`prefer-readisolation-over-locktable-for-reads.bad.al`](prefer-readisolation-over-locktable-for-reads.bad.al). diff --git a/microsoft/knowledge/performance/prefer-related-table-over-extension-on-hot-ledgers.md b/microsoft/knowledge/performance/prefer-related-table-over-extension-on-hot-ledgers.md index dd03115..9097b5a 100644 --- a/microsoft/knowledge/performance/prefer-related-table-over-extension-on-hot-ledgers.md +++ b/microsoft/knowledge/performance/prefer-related-table-over-extension-on-hot-ledgers.md @@ -20,10 +20,10 @@ Since v23, all extensions on the same base table share at most one companion-tab Put optional, sparse, or integration attributes in a related table with the ledger entry number as primary key. Show them from a FactBox or a FlowField. Use a tableextension stored field only when the value must appear as a native list column and is read on almost every access. -See sample: `prefer-related-table-over-extension-on-hot-ledgers.good.al`. +See sample: [`prefer-related-table-over-extension-on-hot-ledgers.good.al`](prefer-related-table-over-extension-on-hot-ledgers.good.al). ## Anti Pattern `tableextension` on `"G/L Entry"` (or another posting table) that adds several stored `Text`/`Blob` fields used only by one integration. The companion join is paid on every posting and on any AL code path that loads extension fields, even when those columns are not needed for the current operation. -See sample: `prefer-related-table-over-extension-on-hot-ledgers.bad.al`. +See sample: [`prefer-related-table-over-extension-on-hot-ledgers.bad.al`](prefer-related-table-over-extension-on-hot-ledgers.bad.al). diff --git a/microsoft/knowledge/performance/query-results-bypass-primary-key-cache.md b/microsoft/knowledge/performance/query-results-bypass-primary-key-cache.md index 1f3205f..5bcfbd8 100644 --- a/microsoft/knowledge/performance/query-results-bypass-primary-key-cache.md +++ b/microsoft/knowledge/performance/query-results-bypass-primary-key-cache.md @@ -19,10 +19,10 @@ The Business Central server caches primary-key `Get` calls within a transaction. Keep `Record.Get` for repeated lookups of the same primary keys in one transaction. Use a Query when the work is a true join or aggregation that the record API would express as nested scans. Do not flag a guarded `Get` on a repeating key as an N+1 solely because a Query could express the same columns. -See sample: `query-results-bypass-primary-key-cache.good.al`. +See sample: [`query-results-bypass-primary-key-cache.good.al`](query-results-bypass-primary-key-cache.good.al). ## Anti Pattern Rewriting a helper that `Get`s Customer by `No.` on every sales line into a Query opened inside that helper. Distinct line customers still need a lookup; repeating customers were already served from the PK cache. The Query pays SQL every time. -See sample: `query-results-bypass-primary-key-cache.bad.al`. +See sample: [`query-results-bypass-primary-key-cache.bad.al`](query-results-bypass-primary-key-cache.bad.al). diff --git a/microsoft/knowledge/performance/reset-clears-partial-record-selection.md b/microsoft/knowledge/performance/reset-clears-partial-record-selection.md index c99f8d2..d53aecb 100644 --- a/microsoft/knowledge/performance/reset-clears-partial-record-selection.md +++ b/microsoft/knowledge/performance/reset-clears-partial-record-selection.md @@ -19,10 +19,10 @@ application-area: [all] Call `Reset` (or empty `SetLoadFields()`) first when the variable must be reused, then call `SetLoadFields` with the fields the next read actually uses, then apply filters and read. After `Reset`, a new `SetLoadFields` is required; the previous list is gone. -See sample: `reset-clears-partial-record-selection.good.al`. +See sample: [`reset-clears-partial-record-selection.good.al`](reset-clears-partial-record-selection.good.al). ## Anti Pattern `SetLoadFields(...)` followed by `Reset()` (or by parameterless `SetLoadFields()`) and then `FindSet` without restoring the load list. The filters look correct; the SQL still selects every column. -See sample: `reset-clears-partial-record-selection.bad.al`. +See sample: [`reset-clears-partial-record-selection.bad.al`](reset-clears-partial-record-selection.bad.al). diff --git a/microsoft/knowledge/performance/setcurrentkey-sets-sort-order-not-index-hint.md b/microsoft/knowledge/performance/setcurrentkey-sets-sort-order-not-index-hint.md index 40428ca..ad0bb40 100644 --- a/microsoft/knowledge/performance/setcurrentkey-sets-sort-order-not-index-hint.md +++ b/microsoft/knowledge/performance/setcurrentkey-sets-sort-order-not-index-hint.md @@ -26,10 +26,10 @@ Decide `SetCurrentKey` on one question only: **do I need the result set in a spe To make a filtered read fast, ensure a key (index) exists on the table whose leading fields cover the filter, and filter on those fields with `SetRange`/`SetFilter`. That is what lets the optimizer seek. Defining the key creates the index; `SetCurrentKey` is not required to make the optimizer use it. -See sample: `setcurrentkey-sets-sort-order-not-index-hint.good.al`. +See sample: [`setcurrentkey-sets-sort-order-not-index-hint.good.al`](setcurrentkey-sets-sort-order-not-index-hint.good.al). ## Anti Pattern Adding `SetCurrentKey` to a filtered read purely in the belief that it forces SQL Server to seek a particular index, when the code never uses the resulting order. This does nothing for index selection and only appends an `ORDER BY` the query does not need, risking an unnecessary sort. Remove the `SetCurrentKey`; rely on the filters and an existing covering key instead. -See sample: `setcurrentkey-sets-sort-order-not-index-hint.bad.al`. +See sample: [`setcurrentkey-sets-sort-order-not-index-hint.bad.al`](setcurrentkey-sets-sort-order-not-index-hint.bad.al). diff --git a/microsoft/knowledge/performance/skip-setloadfields-on-write-and-transferfields.md b/microsoft/knowledge/performance/skip-setloadfields-on-write-and-transferfields.md index 8077f28..41a0ad8 100644 --- a/microsoft/knowledge/performance/skip-setloadfields-on-write-and-transferfields.md +++ b/microsoft/knowledge/performance/skip-setloadfields-on-write-and-transferfields.md @@ -19,10 +19,10 @@ application-area: [all] Omit `SetLoadFields` on loops whose body performs a documented full-load operation (`Insert`, `Delete`, `Rename`, `TransferFields`, or assignment into a temporary record) on the same record variable, so the initial read already materializes every field those operations need. -See sample: `skip-setloadfields-on-write-and-transferfields.good.al`. +See sample: [`skip-setloadfields-on-write-and-transferfields.good.al`](skip-setloadfields-on-write-and-transferfields.good.al). ## Anti Pattern Calling `SetLoadFields` immediately before a `FindSet` whose body performs `Delete`, `Rename`, `TransferFields`, or copies the record into a temporary table. The review signal is a partial-record setup on a record variable that feeds one of these documented full-load operations in the same iteration. -See sample: `skip-setloadfields-on-write-and-transferfields.bad.al`. +See sample: [`skip-setloadfields-on-write-and-transferfields.bad.al`](skip-setloadfields-on-write-and-transferfields.bad.al). diff --git a/microsoft/knowledge/performance/use-dedicated-lookup-pages-not-full-lists.md b/microsoft/knowledge/performance/use-dedicated-lookup-pages-not-full-lists.md index 191568e..6a9793f 100644 --- a/microsoft/knowledge/performance/use-dedicated-lookup-pages-not-full-lists.md +++ b/microsoft/knowledge/performance/use-dedicated-lookup-pages-not-full-lists.md @@ -19,10 +19,10 @@ A `TableRelation` lookup opens the table's `LookupPageId`. If that is the full l Give master tables a slim lookup page (`PageType = List`, few columns, no FactBoxes, no heavy `OnAfterGetRecord`) and assign it to `LookupPageId`. Keep the full list for `DrillDownPageId` and the role-explorer entry. -See sample: `use-dedicated-lookup-pages-not-full-lists.good.al`. +See sample: [`use-dedicated-lookup-pages-not-full-lists.good.al`](use-dedicated-lookup-pages-not-full-lists.good.al). ## Anti Pattern `LookupPageId = Page::"... List"` on a table that already has (or should have) a lookup page. Opening a field lookup then pays list-page cost. The signal is `LookupPageId` pointing at a page that declares FactBoxes or a wide repeater. -See sample: `use-dedicated-lookup-pages-not-full-lists.bad.al`. +See sample: [`use-dedicated-lookup-pages-not-full-lists.bad.al`](use-dedicated-lookup-pages-not-full-lists.bad.al). diff --git a/microsoft/knowledge/performance/use-deleteall-for-filtered-bulk-deletion.md b/microsoft/knowledge/performance/use-deleteall-for-filtered-bulk-deletion.md index 41ad41f..8c847b4 100644 --- a/microsoft/knowledge/performance/use-deleteall-for-filtered-bulk-deletion.md +++ b/microsoft/knowledge/performance/use-deleteall-for-filtered-bulk-deletion.md @@ -19,10 +19,10 @@ application-area: [all] Use filtered `DeleteAll(false)` for purpose-built staging or cleanup tables only after verifying that base-table `OnDelete` logic is unnecessary and that trigger code, related subscribers, security filtering, media fields, and companion fields do not add required per-row behavior or regress the bulk path. If deletion requires per-row business logic, keep an explicit triggered operation instead of simulating trigger execution separately. -See sample: `use-deleteall-for-filtered-bulk-deletion.good.al`. +See sample: [`use-deleteall-for-filtered-bulk-deletion.good.al`](use-deleteall-for-filtered-bulk-deletion.good.al). ## Anti Pattern Iterating with `FindSet` + `Delete(false)` to clear a filtered staging batch that has no delete logic or fallback condition. The reverse mistake is assuming `DeleteAll` is always one SQL statement without checking the documented fallback conditions. -See sample: `use-deleteall-for-filtered-bulk-deletion.bad.al`. +See sample: [`use-deleteall-for-filtered-bulk-deletion.bad.al`](use-deleteall-for-filtered-bulk-deletion.bad.al). diff --git a/microsoft/knowledge/performance/use-get-instead-of-findfirst-on-full-primary-key.md b/microsoft/knowledge/performance/use-get-instead-of-findfirst-on-full-primary-key.md index 06c0383..e74614b 100644 --- a/microsoft/knowledge/performance/use-get-instead-of-findfirst-on-full-primary-key.md +++ b/microsoft/knowledge/performance/use-get-instead-of-findfirst-on-full-primary-key.md @@ -17,10 +17,10 @@ application-area: [all] When all primary-key fields are available at the call site, call `Get` (or `GetBySystemId`) with them. Reserve `FindFirst` for cases where the filter is on something other than the full primary key โ€” a unique secondary field, a partial composite key, a sort that the caller cares about. -See sample: `use-get-instead-of-findfirst-on-full-primary-key.good.al`. +See sample: [`use-get-instead-of-findfirst-on-full-primary-key.good.al`](use-get-instead-of-findfirst-on-full-primary-key.good.al). ## Anti Pattern Composing `SetRange` calls that exactly cover the primary key and then calling `FindFirst`. The result is correct but the call site reads as "search the table" rather than "look up by key", which obscures both the intent and the access pattern from later reviewers. -See sample: `use-get-instead-of-findfirst-on-full-primary-key.bad.al`. +See sample: [`use-get-instead-of-findfirst-on-full-primary-key.bad.al`](use-get-instead-of-findfirst-on-full-primary-key.bad.al). diff --git a/microsoft/knowledge/performance/use-isempty-for-existence-check.md b/microsoft/knowledge/performance/use-isempty-for-existence-check.md index ab574ec..cfa2dc1 100644 --- a/microsoft/knowledge/performance/use-isempty-for-existence-check.md +++ b/microsoft/knowledge/performance/use-isempty-for-existence-check.md @@ -17,10 +17,10 @@ When the caller only needs to know whether any row matches a filter, `IsEmpty()` Phrase existence checks as `if not Record.IsEmpty() then ...` (or `if Record.IsEmpty() then ...` for the negative). Apply filters via `SetRange`/`SetFilter` before the call so the existence check runs against the intended subset. Reserve `Count` for cases where the actual number matters and `FindFirst` for cases where the record fields are read. -See sample: `use-isempty-for-existence-check.good.al`. +See sample: [`use-isempty-for-existence-check.good.al`](use-isempty-for-existence-check.good.al). ## Anti Pattern `if Customer.Count() > 0 then ...` and `if Customer.FindFirst() then ...` (when the record is discarded) โ€” both are flagged by the upstream guidance as the wrong tool. The first asks the database for the full count; the second asks for a row's fields. Both answers go unused. -See sample: `use-isempty-for-existence-check.bad.al`. +See sample: [`use-isempty-for-existence-check.bad.al`](use-isempty-for-existence-check.bad.al). diff --git a/microsoft/knowledge/performance/use-setautocalcfields-for-per-row-flowfields.md b/microsoft/knowledge/performance/use-setautocalcfields-for-per-row-flowfields.md index 0c749ce..b5f9d6a 100644 --- a/microsoft/knowledge/performance/use-setautocalcfields-for-per-row-flowfields.md +++ b/microsoft/knowledge/performance/use-setautocalcfields-for-per-row-flowfields.md @@ -17,10 +17,10 @@ application-area: [all] Call `SetAutoCalcFields` before `FindSet` when every returned row needs the same FlowField for a comparison, branch, or per-record action. Use `CalcSums` instead when the required result is one aggregate over the filtered set (see `calcsums-instead-of-calcfields-in-loop.md`). -See sample: `use-setautocalcfields-for-per-row-flowfields.good.al`. +See sample: [`use-setautocalcfields-for-per-row-flowfields.good.al`](use-setautocalcfields-for-per-row-flowfields.good.al). ## Anti Pattern Calling `CalcFields` inside the loop when every iteration reads the same FlowField. Each `CalcFields` request requires a separate SQL statement unless a compatible recent result is cached. Do not replace row-specific decisions with `CalcSums`; an aggregate cannot preserve which rows met the condition. -See sample: `use-setautocalcfields-for-per-row-flowfields.bad.al`. +See sample: [`use-setautocalcfields-for-per-row-flowfields.bad.al`](use-setautocalcfields-for-per-row-flowfields.bad.al). diff --git a/microsoft/knowledge/performance/use-setloadfields-for-partial-records.md b/microsoft/knowledge/performance/use-setloadfields-for-partial-records.md index a8d134f..06b2d14 100644 --- a/microsoft/knowledge/performance/use-setloadfields-for-partial-records.md +++ b/microsoft/knowledge/performance/use-setloadfields-for-partial-records.md @@ -17,7 +17,7 @@ application-area: [all] Before a `Get`, `FindSet`, or `FindFirst` that the procedure follows by reading only a handful of the table's fields, call `SetLoadFields` listing exactly those fields. The pattern `SetLoadFields(...); if Record.Get(...) then ...` is the upstream-endorsed shape. Place the call immediately before the read, after any `SetRange`/`SetFilter`, so a reader can see at a glance which read the selection governs and any projection-changing operation is easy to spot. Skip `SetLoadFields` when the table has few fields (under ten), when the code reads most of them (above 60 %), when the loop runs ten or fewer iterations, or when the table is exempt for other reasons (`singleton-setup-tables-need-no-access-optimization.md`, `temporary-tables-have-no-database-cost.md`). For report dataitems, use `AddLoadFields` in `OnPreDataItem` instead (see `addloadfields-in-report-onpredataitem.md`). -See sample: `use-setloadfields-for-partial-records.good.al`. +See sample: [`use-setloadfields-for-partial-records.good.al`](use-setloadfields-for-partial-records.good.al). ## Anti Pattern @@ -25,4 +25,4 @@ Loading a wide table and reading one field per row in a loop. The bytes transfer Statement order is not part of this anti pattern. `SetLoadFields` placed ahead of `SetRange`/`SetFilter` materializes exactly the same columns as the reverse order, so a reviewer reports it as a readability observation at most โ€” never as a performance defect. -See sample: `use-setloadfields-for-partial-records.bad.al`. +See sample: [`use-setloadfields-for-partial-records.bad.al`](use-setloadfields-for-partial-records.bad.al). diff --git a/microsoft/knowledge/performance/use-tryfunction-for-error-catching-not-rollback.md b/microsoft/knowledge/performance/use-tryfunction-for-error-catching-not-rollback.md index 6070b76..41f751f 100644 --- a/microsoft/knowledge/performance/use-tryfunction-for-error-catching-not-rollback.md +++ b/microsoft/knowledge/performance/use-tryfunction-for-error-catching-not-rollback.md @@ -19,13 +19,13 @@ Reach for `[TryFunction]` when you want to catch a failure without unwinding the Use `[TryFunction]` sparingly. Each caught error writes to the session-wide `GetLastErrorText` and `GetLastErrorCallStack` buffers, and every subsequent catch overwrites the earlier state โ€” a helper that reads `GetLastErrorText` later may see a different error than the one it intended to inspect. Prefer explicit checks (non-throwing predicates, guard conditions, upfront validation) for operations with predictable failure modes; reserve `[TryFunction]` for genuinely unpredictable failures such as network calls, third-party interop, or evaluation of user-supplied expressions. When you do catch, read `GetLastErrorText` immediately after the failed call, and call `ClearLastError` before the call if an earlier catch in the same scope could have left state behind โ€” per the platform reference, "If you call the GetLastErrorText method immediately after you call the ClearLastError method, then an empty string is returned." -See sample: `use-tryfunction-for-error-catching-not-rollback.good.al`. +See sample: [`use-tryfunction-for-error-catching-not-rollback.good.al`](use-tryfunction-for-error-catching-not-rollback.good.al). ## Anti Pattern Wrapping database writes in `[TryFunction]` and expecting successful writes before the error to roll back. They remain, the caller receives `false`, and partially applied state can escape. Defensive sprinkling is also unsafe: every catch overwrites the session error buffer and can hide the failure a later helper intended to inspect. -See sample: `use-tryfunction-for-error-catching-not-rollback.bad.al`. +See sample: [`use-tryfunction-for-error-catching-not-rollback.bad.al`](use-tryfunction-for-error-catching-not-rollback.bad.al). ## See also diff --git a/microsoft/knowledge/performance/validate-on-partial-record-forces-jit.md b/microsoft/knowledge/performance/validate-on-partial-record-forces-jit.md index 00b9657..2b87c2a 100644 --- a/microsoft/knowledge/performance/validate-on-partial-record-forces-jit.md +++ b/microsoft/knowledge/performance/validate-on-partial-record-forces-jit.md @@ -19,10 +19,10 @@ application-area: [all] In a partial-record loop, assign fields directly when trigger side effects are not required. If `Validate` is required, do not use `SetLoadFields` on that iterator, or `AddLoadFields` every field the validate path can touch before the read. -See sample: `validate-on-partial-record-forces-jit.good.al`. +See sample: [`validate-on-partial-record-forces-jit.good.al`](validate-on-partial-record-forces-jit.good.al). ## Anti Pattern `SetLoadFields` on a handful of columns, then `Validate` inside the loop. The load list looks optimal; runtime JIT and TableRelation I/O dominate. The signal is `Validate(` on a record that still has a `SetLoadFields` in the same procedure. -See sample: `validate-on-partial-record-forces-jit.bad.al`. +See sample: [`validate-on-partial-record-forces-jit.bad.al`](validate-on-partial-record-forces-jit.bad.al). diff --git a/microsoft/knowledge/privacy/avoid-strsubstno-prebuild-before-error.md b/microsoft/knowledge/privacy/avoid-strsubstno-prebuild-before-error.md index f9245b1..f4a25f3 100644 --- a/microsoft/knowledge/privacy/avoid-strsubstno-prebuild-before-error.md +++ b/microsoft/knowledge/privacy/avoid-strsubstno-prebuild-before-error.md @@ -17,10 +17,10 @@ Error method trace telemetry includes the AL error string only when the first `E Declare the complete message as a `Label` or `TextConst` and pass it directly to `Error`, followed by substitution values. The client receives the formatted message while telemetry retains the static message template without using the dynamic values as its message. Independently review whether each substitution value is appropriate to show to the current user. -See sample: `avoid-strsubstno-prebuild-before-error.good.al`. +See sample: [`avoid-strsubstno-prebuild-before-error.good.al`](avoid-strsubstno-prebuild-before-error.good.al). ## Anti Pattern `Error(StrSubstNo(CustomerInvalidErr, Customer."No."))` and `Error(HeaderErr + DetailErr)` both make the first argument dynamic. They reduce error telemetry quality; they do not cause that composed string to be logged verbatim as the telemetry message. -See sample: `avoid-strsubstno-prebuild-before-error.bad.al`. +See sample: [`avoid-strsubstno-prebuild-before-error.bad.al`](avoid-strsubstno-prebuild-before-error.bad.al). diff --git a/microsoft/knowledge/privacy/data-classification-required-on-pii-fields.md b/microsoft/knowledge/privacy/data-classification-required-on-pii-fields.md index b431c12..6bad6f6 100644 --- a/microsoft/knowledge/privacy/data-classification-required-on-pii-fields.md +++ b/microsoft/knowledge/privacy/data-classification-required-on-pii-fields.md @@ -17,10 +17,10 @@ application-area: [all] Set `DataClassification` to the value that matches the data the field actually stores. A `Customer."E-Mail"`-style field is `CustomerContent` (data belonging to the tenant's customers); a personal identifier such as an employee number or user ID is `EndUserIdentifiableInformation` or `EndUserPseudonymousIdentifiers` depending on whether it is directly identifying. A field that identifies an organization rather than a person โ€” a company registration or VAT registration number โ€” is `OrganizationIdentifiableInformation`, and a financial account identifier such as a bank account number or IBAN is `AccountData`. Choose the classification at field definition time โ€” fixing it later is a schema change. -See sample: `data-classification-required-on-pii-fields.good.al`. +See sample: [`data-classification-required-on-pii-fields.good.al`](data-classification-required-on-pii-fields.good.al). ## Anti Pattern Declaring a field that stores PII with `DataClassification = SystemMetadata` to silence the compiler warning. The field compiles but the platform now treats customer data as system metadata in telemetry, GDPR exports and admin reports. -See sample: `data-classification-required-on-pii-fields.bad.al`. +See sample: [`data-classification-required-on-pii-fields.bad.al`](data-classification-required-on-pii-fields.bad.al). diff --git a/microsoft/knowledge/privacy/errorinfo-telemetry-classification-and-errortype.md b/microsoft/knowledge/privacy/errorinfo-telemetry-classification-and-errortype.md index ce1b684..d83131f 100644 --- a/microsoft/knowledge/privacy/errorinfo-telemetry-classification-and-errortype.md +++ b/microsoft/knowledge/privacy/errorinfo-telemetry-classification-and-errortype.md @@ -17,10 +17,10 @@ Runtime 3.0 (BC 14) provides `ErrorInfo.Message`, `DataClassification`, and `Err Keep `Message` stable and classify its actual content. Choose `ErrorType` for client usability, not as a telemetry privacy boundary. On BC 19 and later, put only support-safe technical context in `DetailedMessage`, because a user can copy it from the dialog. The samples use only members available at the BC 14 article floor. -See sample: `errorinfo-telemetry-classification-and-errortype.good.al`. +See sample: [`errorinfo-telemetry-classification-and-errortype.good.al`](errorinfo-telemetry-classification-and-errortype.good.al). ## Anti Pattern Marking a dynamic customer-bearing `Message` as `SystemMetadata`, or assuming `ErrorType::Internal` keeps it out of telemetry. On BC 19 and later, the same anti-pattern includes placing secrets or personal data in `DetailedMessage` because it is not the primary dialog text. -See sample: `errorinfo-telemetry-classification-and-errortype.bad.al`. +See sample: [`errorinfo-telemetry-classification-and-errortype.bad.al`](errorinfo-telemetry-classification-and-errortype.bad.al). diff --git a/microsoft/knowledge/privacy/featuretelemetry-customdimensions-no-pii.md b/microsoft/knowledge/privacy/featuretelemetry-customdimensions-no-pii.md index 6d8bfb5..f34804b 100644 --- a/microsoft/knowledge/privacy/featuretelemetry-customdimensions-no-pii.md +++ b/microsoft/knowledge/privacy/featuretelemetry-customdimensions-no-pii.md @@ -17,10 +17,10 @@ application-area: [all] Pass only non-personal context through `CustomDimensions` โ€” feature names, status enums, counts, error codes, durations. For uptake or usage signals that do not need per-call context, prefer the parameterless overload of `LogUptake`/`LogUsage` over a `CustomDimensions` dictionary that risks accreting PII over time. -See sample: `featuretelemetry-customdimensions-no-pii.good.al`. +See sample: [`featuretelemetry-customdimensions-no-pii.good.al`](featuretelemetry-customdimensions-no-pii.good.al). ## Anti Pattern `CustomDimensions.Add('EmployeeNo', ExpenseHeader."Employee No.")` followed by `FeatureTelemetry.LogUsage(...)` โ€” the employee number is a pseudonymous user identifier (EUPI) and is now in telemetry. Same pattern with `'UserName'`, `'CustomerEmail'`, `'AttachmentName'` etc. -See sample: `featuretelemetry-customdimensions-no-pii.bad.al`. +See sample: [`featuretelemetry-customdimensions-no-pii.bad.al`](featuretelemetry-customdimensions-no-pii.bad.al). diff --git a/microsoft/knowledge/privacy/featuretelemetry-logerror-implicit-errortext.md b/microsoft/knowledge/privacy/featuretelemetry-logerror-implicit-errortext.md index 863f3a8..64a9b12 100644 --- a/microsoft/knowledge/privacy/featuretelemetry-logerror-implicit-errortext.md +++ b/microsoft/knowledge/privacy/featuretelemetry-logerror-implicit-errortext.md @@ -17,10 +17,10 @@ application-area: [all] Review the dedicated error arguments as telemetry payload. Capture `GetLastErrorText(true)` when scrubbed platform error text is sufficient, and pass `GetLastErrorCallStack()` only as a call stack. Keep custom dimensions non-personal too. -See sample: `featuretelemetry-logerror-implicit-errortext.good.al`. +See sample: [`featuretelemetry-logerror-implicit-errortext.good.al`](featuretelemetry-logerror-implicit-errortext.good.al). ## Anti Pattern Approving a `LogError` call because its explicit dictionary contains only safe values while it passes unsanitized `GetLastErrorText()` or arbitrary context through `ErrorText` or `ErrorCallStack`. Those arguments become telemetry dimensions outside the dictionary. -See sample: `featuretelemetry-logerror-implicit-errortext.bad.al`. +See sample: [`featuretelemetry-logerror-implicit-errortext.bad.al`](featuretelemetry-logerror-implicit-errortext.bad.al). diff --git a/microsoft/knowledge/privacy/flowfield-flowfilter-classification-systemmetadata.md b/microsoft/knowledge/privacy/flowfield-flowfilter-classification-systemmetadata.md index d3a1b7b..659b775 100644 --- a/microsoft/knowledge/privacy/flowfield-flowfilter-classification-systemmetadata.md +++ b/microsoft/knowledge/privacy/flowfield-flowfilter-classification-systemmetadata.md @@ -17,7 +17,7 @@ application-area: [all] Do not declare `DataClassification` on `FieldClass = FlowField` or `FieldClass = FlowFilter` fields โ€” the inherited `SystemMetadata` is correct and the property is redundant. If a FlowField exposes sensitive data, ensure the underlying source field has the right `DataClassification`; that is where the platform reads classification from for GDPR and telemetry purposes. -See sample: `flowfield-flowfilter-classification-systemmetadata.good.al`. +See sample: [`flowfield-flowfilter-classification-systemmetadata.good.al`](flowfield-flowfilter-classification-systemmetadata.good.al). ## Anti Pattern diff --git a/microsoft/knowledge/privacy/getlasterrortext-customer-content-in-errors.md b/microsoft/knowledge/privacy/getlasterrortext-customer-content-in-errors.md index 8ff26a8..fd541a1 100644 --- a/microsoft/knowledge/privacy/getlasterrortext-customer-content-in-errors.md +++ b/microsoft/knowledge/privacy/getlasterrortext-customer-content-in-errors.md @@ -17,10 +17,10 @@ Parameterless `GetLastErrorText()` can contain customer content such as field va Use a generic label when the user does not need the underlying detail. If showing unsanitized detail is appropriate, put `%1` in a label and pass parameterless `GetLastErrorText()` as a separate argument. This preserves a useful static telemetry message while keeping the dynamic value out of the telemetry message field. -See sample: `getlasterrortext-customer-content-in-errors.good.al`. +See sample: [`getlasterrortext-customer-content-in-errors.good.al`](getlasterrortext-customer-content-in-errors.good.al). ## Anti Pattern `Error(StrSubstNo(AttachmentFailedErr, GetLastErrorText()))` or `Error(AttachmentPrefixErr + GetLastErrorText())`. Both lose the static first argument and trigger AA0231; neither causes the composed text to be logged verbatim as the Error telemetry message. -See sample: `getlasterrortext-customer-content-in-errors.bad.al`. +See sample: [`getlasterrortext-customer-content-in-errors.bad.al`](getlasterrortext-customer-content-in-errors.bad.al). diff --git a/microsoft/knowledge/privacy/no-pii-in-telemetry-message-string.md b/microsoft/knowledge/privacy/no-pii-in-telemetry-message-string.md index e27d1e4..8192c84 100644 --- a/microsoft/knowledge/privacy/no-pii-in-telemetry-message-string.md +++ b/microsoft/knowledge/privacy/no-pii-in-telemetry-message-string.md @@ -19,7 +19,7 @@ Keep the telemetry message a static, non-personal string ("Customer record proce If a pseudonymous identifier (record `No.`, primary key value) genuinely belongs in the diagnostic, prefer attaching it through a custom dimension and set the call's `DataClassification` to match the data actually shipped โ€” `EndUserPseudonymousIdentifiers` for pseudonymous IDs, `CustomerContent` for content-bearing telemetry. Changing the `DataClassification` alone does **not** make embedding a customer name into the message string acceptable; the data still ships in the message, and downstream consumers still see the literal string. -See sample: `no-pii-in-telemetry-message-string.good.al`. +See sample: [`no-pii-in-telemetry-message-string.good.al`](no-pii-in-telemetry-message-string.good.al). ## Related @@ -30,4 +30,4 @@ See sample: `no-pii-in-telemetry-message-string.good.al`. `Session.LogMessage('0000', StrSubstNo('Processed %1', Customer.Name), ...)` โ€” the customer name is in telemetry the moment the line runs. Detection signal: a `StrSubstNo` whose result is the second argument of `Session.LogMessage`. The same shape with `FileName`, `EmployeeCode`, or any record field is the same problem. -See sample: `no-pii-in-telemetry-message-string.bad.al`. +See sample: [`no-pii-in-telemetry-message-string.bad.al`](no-pii-in-telemetry-message-string.bad.al). diff --git a/microsoft/knowledge/privacy/privacy-notice-consent-for-external-data-transfer.md b/microsoft/knowledge/privacy/privacy-notice-consent-for-external-data-transfer.md index d95acef..8c3898d 100644 --- a/microsoft/knowledge/privacy/privacy-notice-consent-for-external-data-transfer.md +++ b/microsoft/knowledge/privacy/privacy-notice-consent-for-external-data-transfer.md @@ -17,10 +17,10 @@ Business Central's `Codeunit "Privacy Notice"` creates notices and records per-i Register the custom notice with `CreatePrivacyNotice` during setup or through `OnRegisterPrivacyNotices`. Before sending data, call `ConfirmPrivacyNoticeApproval()` outside a write transaction, or check `GetPrivacyNoticeApprovalState()` when the flow must not show UI. No path should issue the request without approval. -See sample: `privacy-notice-consent-for-external-data-transfer.good.al`. +See sample: [`privacy-notice-consent-for-external-data-transfer.good.al`](privacy-notice-consent-for-external-data-transfer.good.al). ## Anti Pattern A custom integration that posts data without checking its own notice, or that gates the call with a built-in ID such as the Exchange privacy notice ID. Consent for one service does not authorize another. -See sample: `privacy-notice-consent-for-external-data-transfer.bad.al`. +See sample: [`privacy-notice-consent-for-external-data-transfer.bad.al`](privacy-notice-consent-for-external-data-transfer.bad.al). diff --git a/microsoft/knowledge/privacy/register-integration-in-privacy-notice-registrations.md b/microsoft/knowledge/privacy/register-integration-in-privacy-notice-registrations.md index 4e76779..a9f12ff 100644 --- a/microsoft/knowledge/privacy/register-integration-in-privacy-notice-registrations.md +++ b/microsoft/knowledge/privacy/register-integration-in-privacy-notice-registrations.md @@ -17,7 +17,7 @@ The current extension point is `Codeunit "Privacy Notice"`. Extensions can subsc Choose a stable ID owned by the extension. Register it through `OnRegisterPrivacyNotices`, or call `PrivacyNotice.CreatePrivacyNotice` during an intentional setup or upgrade path. Use that same ID for consent checks described in `privacy-notice-consent-for-external-data-transfer.md`. -See sample: `register-integration-in-privacy-notice-registrations.good.al`. +See sample: [`register-integration-in-privacy-notice-registrations.good.al`](register-integration-in-privacy-notice-registrations.good.al). ## Anti Pattern diff --git a/microsoft/knowledge/privacy/session-logmessage-requires-dataclassification.md b/microsoft/knowledge/privacy/session-logmessage-requires-dataclassification.md index 67381f7..2febcae 100644 --- a/microsoft/knowledge/privacy/session-logmessage-requires-dataclassification.md +++ b/microsoft/knowledge/privacy/session-logmessage-requires-dataclassification.md @@ -17,10 +17,10 @@ application-area: [all] Use the overload that takes `Verbosity`, `DataClassification`, and `TelemetryScope`. For payload-free operational telemetry that does not embed customer data, `DataClassification::SystemMetadata` is the right value. Choose `TelemetryScope::ExtensionPublisher` for telemetry meant for the publishing partner only; `TelemetryScope::All` also forwards to the customer's tenant telemetry. -See sample: `session-logmessage-requires-dataclassification.good.al`. +See sample: [`session-logmessage-requires-dataclassification.good.al`](session-logmessage-requires-dataclassification.good.al). ## Anti Pattern Calling `Session.LogMessage('0003', 'Operation completed', Verbosity::Normal)` โ€” the overload omits `DataClassification` and leaves the platform without the information needed to classify the entry. Detection signal: a `Session.LogMessage` call whose argument list ends at `Verbosity`. -See sample: `session-logmessage-requires-dataclassification.bad.al`. +See sample: [`session-logmessage-requires-dataclassification.bad.al`](session-logmessage-requires-dataclassification.bad.al). diff --git a/microsoft/knowledge/privacy/table-level-data-classification-cascades.md b/microsoft/knowledge/privacy/table-level-data-classification-cascades.md index 253d2cc..f41dc11 100644 --- a/microsoft/knowledge/privacy/table-level-data-classification-cascades.md +++ b/microsoft/knowledge/privacy/table-level-data-classification-cascades.md @@ -17,7 +17,7 @@ A valid table-level `DataClassification` is the effective default for the Normal Use a table-level classification when it accurately describes the table's fields, and add a field-level classification only where a field stores a different kind of data. Do not flag a Normal field solely because it omits an explicit property when its own table supplies a valid default; verify whether the inherited value matches the field's data instead. A `tableextension` has no default to inherit, so require an explicit `DataClassification` on every Normal field it adds. -See sample: `table-level-data-classification-cascades.good.al`. +See sample: [`table-level-data-classification-cascades.good.al`](table-level-data-classification-cascades.good.al). ## Anti Pattern diff --git a/microsoft/knowledge/query/reopening-query-resets-cursor-but-keeps-filters.md b/microsoft/knowledge/query/reopening-query-resets-cursor-but-keeps-filters.md index 4bc8816..0d7ca47 100644 --- a/microsoft/knowledge/query/reopening-query-resets-cursor-but-keeps-filters.md +++ b/microsoft/knowledge/query/reopening-query-resets-cursor-but-keeps-filters.md @@ -17,10 +17,10 @@ Calling `Open()` on an already open query first closes the current dataset and o Open once for one read pass. Close after the pass, and call `Clear(QueryVariable)` before reusing the variable for a logically independent query whose filters must start empty. Set the next pass's filters explicitly before reopening. -See sample: `reopening-query-resets-cursor-but-keeps-filters.good.al`. +See sample: [`reopening-query-resets-cursor-but-keeps-filters.good.al`](reopening-query-resets-cursor-but-keeps-filters.good.al). ## Anti Pattern Calling `Open()` inside or between reads to "advance" or "start fresh", or reusing the same query variable for a new operation while assuming `Open()` cleared old filters. The code compiles but can repeatedly process the first row or silently omit rows behind a retained filter. -See sample: `reopening-query-resets-cursor-but-keeps-filters.bad.al`. +See sample: [`reopening-query-resets-cursor-but-keeps-filters.bad.al`](reopening-query-resets-cursor-but-keeps-filters.bad.al). diff --git a/microsoft/knowledge/query/set-query-filters-before-open.md b/microsoft/knowledge/query/set-query-filters-before-open.md index f999456..bb82e9d 100644 --- a/microsoft/knowledge/query/set-query-filters-before-open.md +++ b/microsoft/knowledge/query/set-query-filters-before-open.md @@ -17,10 +17,10 @@ application-area: [all] Apply every filter before `Open()`, then read the dataset to completion and call `Close()`. When a later branch needs different filters, close or clear the query, set the new filters, and open a new dataset deliberately. -See sample: `set-query-filters-before-open.good.al`. +See sample: [`set-query-filters-before-open.good.al`](set-query-filters-before-open.good.al). ## Anti Pattern `Query.Open()` followed by `SetFilter` or `SetRange` and then `Read()` under the assumption that the filter updates the open cursor. Refiltering after `Open()` is valid only when the code intentionally opens a fresh dataset afterward. -See sample: `set-query-filters-before-open.bad.al`. +See sample: [`set-query-filters-before-open.bad.al`](set-query-filters-before-open.bad.al). diff --git a/microsoft/knowledge/security/al-has-no-built-in-htmlencode.md b/microsoft/knowledge/security/al-has-no-built-in-htmlencode.md index 7441712..649c384 100644 --- a/microsoft/knowledge/security/al-has-no-built-in-htmlencode.md +++ b/microsoft/knowledge/security/al-has-no-built-in-htmlencode.md @@ -15,8 +15,8 @@ AL does not ship a built-in `HtmlEncode` (or equivalent) function. Code that bui ## Best Practice -Replace the four characters by hand before concatenating user content into HTML: `&` โ†’ `&` first, then `<` โ†’ `<`, `>` โ†’ `>`, `"` โ†’ `"`. Centralize the substitution in one helper so every HTML producer in the extension uses the same encoder. Better still, do not build raw HTML at all โ€” use a structured format (JSON for an API payload, a report layout for a printed document) and let the renderer do the encoding. See sample: `al-has-no-built-in-htmlencode.good.al`. +Replace the four characters by hand before concatenating user content into HTML: `&` โ†’ `&` first, then `<` โ†’ `<`, `>` โ†’ `>`, `"` โ†’ `"`. Centralize the substitution in one helper so every HTML producer in the extension uses the same encoder. Better still, do not build raw HTML at all โ€” use a structured format (JSON for an API payload, a report layout for a printed document) and let the renderer do the encoding. See sample: [`al-has-no-built-in-htmlencode.good.al`](al-has-no-built-in-htmlencode.good.al). ## Anti Pattern -`HtmlContent := '
Welcome ' + UserName + '!
'` โ€” any record-field value or user input concatenated directly into an HTML string. Reviewers should flag any string concatenation whose right-hand operand is a field, a parameter, or any non-literal value, and whose surrounding context contains HTML tags (`<`, `Welcome ' + UserName + '!'` โ€” any record-field value or user input concatenated directly into an HTML string. Reviewers should flag any string concatenation whose right-hand operand is a field, a parameter, or any non-literal value, and whose surrounding context contains HTML tags (`<`, `` column headers, so a captionless field that is mean Reserve `ShowCaption = false` in a layout-table grid for non-editable, free-standing content cells. If a field's role is to label or annotate another field in the same grid, restructure the grid to meet the data-table conditions (see `grid-data-table-heuristic.md`) instead of hiding the caption. -See sample: `standalone-content-in-layout-table.good.al`. +See sample: [`standalone-content-in-layout-table.good.al`](standalone-content-in-layout-table.good.al). diff --git a/microsoft/knowledge/ui/style-expr-text-vs-boolean.md b/microsoft/knowledge/ui/style-expr-text-vs-boolean.md index 5040099..720423b 100644 --- a/microsoft/knowledge/ui/style-expr-text-vs-boolean.md +++ b/microsoft/knowledge/ui/style-expr-text-vs-boolean.md @@ -22,4 +22,4 @@ When `StyleExpr` is Text, you must trace the variable's assignments โ€” typicall Inspect the declared type of the symbol referenced by `StyleExpr` before drawing conclusions. If it is Boolean, evaluate the `Style` property. If it is Text, follow every assignment to the variable and check the full set of possible style values against `cosmetic-styles-need-no-textual-context.md` and `semantic-styles-need-independent-textual-meaning.md`. -See sample: `style-expr-text-vs-boolean.good.al`. +See sample: [`style-expr-text-vs-boolean.good.al`](style-expr-text-vs-boolean.good.al). diff --git a/microsoft/knowledge/ui/tabular-intent-requires-data-table-conditions.md b/microsoft/knowledge/ui/tabular-intent-requires-data-table-conditions.md index b56aadb..c02bbd9 100644 --- a/microsoft/knowledge/ui/tabular-intent-requires-data-table-conditions.md +++ b/microsoft/knowledge/ui/tabular-intent-requires-data-table-conditions.md @@ -24,4 +24,4 @@ Both manifestations have the same root cause: tabular semantics were intended bu A single field that keeps its visible caption is enough to demote an entire would-be data-table grid into a layout table โ€” and silently strip the labels off its sibling captionless fields. Either restructure to meet all three conditions, or restore captions on every editable field. -See sample: `tabular-intent-requires-data-table-conditions.bad.al`. +See sample: [`tabular-intent-requires-data-table-conditions.bad.al`](tabular-intent-requires-data-table-conditions.bad.al). diff --git a/microsoft/knowledge/ui/validate-request-page-input-in-onqueryclosepage.md b/microsoft/knowledge/ui/validate-request-page-input-in-onqueryclosepage.md index 75c8a09..d796827 100644 --- a/microsoft/knowledge/ui/validate-request-page-input-in-onqueryclosepage.md +++ b/microsoft/knowledge/ui/validate-request-page-input-in-onqueryclosepage.md @@ -17,13 +17,13 @@ application-area: [all] ## Best Practice -Put the validation in one local procedure and call it from both places: from the request page's `OnQueryClosePage`, so an interactive user can correct the input where they entered it, and from `OnPreReport` (or the relevant `OnPreDataItem`), so a run without a request page is still refused. Guard the interactive call on the close action โ€” validate only when the user confirmed the run, for example `if CloseAction = Action::OK then`. The base application uses this shape; report 292, `Copy Sales Document`, validates its request-page input in `OnQueryClosePage` behind a close-action check. Mark the control with `ShowMandatory` as well, so the requirement is visible before the user submits โ€” see `showmandatory-on-code-required-page-fields.md`. See sample: `validate-request-page-input-in-onqueryclosepage.good.al`. +Put the validation in one local procedure and call it from both places: from the request page's `OnQueryClosePage`, so an interactive user can correct the input where they entered it, and from `OnPreReport` (or the relevant `OnPreDataItem`), so a run without a request page is still refused. Guard the interactive call on the close action โ€” validate only when the user confirmed the run, for example `if CloseAction = Action::OK then`. The base application uses this shape; report 292, `Copy Sales Document`, validates its request-page input in `OnQueryClosePage` behind a close-action check. Mark the control with `ShowMandatory` as well, so the requirement is visible before the user submits โ€” see `showmandatory-on-code-required-page-fields.md`. See sample: [`validate-request-page-input-in-onqueryclosepage.good.al`](validate-request-page-input-in-onqueryclosepage.good.al). ## Anti Pattern Validating mandatory request-page input only in `OnPreReport`. The check is correct and the report is never run with bad input, but every interactive mistake costs the user the whole request page: the error arrives after the page is gone, and filters, dates, and options all have to be entered again. Reviewer signal: a `TestField`, `Error`, or blank/zero-value check in `OnPreReport` or `OnPreDataItem` against a variable that is bound to a request-page control, in a report whose request page declares no `OnQueryClosePage`. -The mirror defect is an `OnQueryClosePage` that validates without inspecting `CloseAction`: because an error prevents the page from closing, a user who presses Cancel or Esc to abandon the report is trapped in a request page that errors on every attempt to leave it. Validating only in `OnQueryClosePage` is the third variant โ€” the interactive path behaves well, and a job queue entry runs the report with unchecked input. See sample: `validate-request-page-input-in-onqueryclosepage.bad.al`. +The mirror defect is an `OnQueryClosePage` that validates without inspecting `CloseAction`: because an error prevents the page from closing, a user who presses Cancel or Esc to abandon the report is trapped in a request page that errors on every attempt to leave it. Validating only in `OnQueryClosePage` is the third variant โ€” the interactive path behaves well, and a job queue entry runs the report with unchecked input. See sample: [`validate-request-page-input-in-onqueryclosepage.bad.al`](validate-request-page-input-in-onqueryclosepage.bad.al). ## See also diff --git a/microsoft/knowledge/upgrade/breaking-changes-only-on-tables-without-data.md b/microsoft/knowledge/upgrade/breaking-changes-only-on-tables-without-data.md index 9ba7e8a..5efc751 100644 --- a/microsoft/knowledge/upgrade/breaking-changes-only-on-tables-without-data.md +++ b/microsoft/knowledge/upgrade/breaking-changes-only-on-tables-without-data.md @@ -17,10 +17,10 @@ Primary-key changes and field-type changes (for example widening `Integer` to `B Treat primary-key and field-type changes as restricted to tables introduced in the same change. For changes on tables with existing data, design and ship the corresponding upgrade procedure (typically backed by `DataTransfer` and an upgrade tag) that guarantees the new layout is achievable for every row, and verify with concrete evidence that the existing values fit the new constraint (no PK collisions, no value-range overflow). -See sample: `breaking-changes-only-on-tables-without-data.good.al`. +See sample: [`breaking-changes-only-on-tables-without-data.good.al`](breaking-changes-only-on-tables-without-data.good.al). ## Anti Pattern Changing the primary key on a base-app table, or widening / narrowing a field type on a table that has been shipping for releases, with no accompanying upgrade plan. The change compiles cleanly and may even deploy on an empty-ish tenant, then fails on customers who actually have data. -See sample: `breaking-changes-only-on-tables-without-data.bad.al`. +See sample: [`breaking-changes-only-on-tables-without-data.bad.al`](breaking-changes-only-on-tables-without-data.bad.al). diff --git a/microsoft/knowledge/upgrade/check-only-triggers-do-not-migrate-data.md b/microsoft/knowledge/upgrade/check-only-triggers-do-not-migrate-data.md index 30f6ca7..8770f93 100644 --- a/microsoft/knowledge/upgrade/check-only-triggers-do-not-migrate-data.md +++ b/microsoft/knowledge/upgrade/check-only-triggers-do-not-migrate-data.md @@ -17,10 +17,10 @@ application-area: [all] Have check triggers call query-only helpers that raise an error when an invariant fails. Put every `Insert`, `Modify`, `Delete`, `Rename`, `DataTransfer`, and other migration write behind helpers called from the matching `OnUpgrade...` trigger. -See sample: `check-only-triggers-do-not-migrate-data.good.al`. +See sample: [`check-only-triggers-do-not-migrate-data.good.al`](check-only-triggers-do-not-migrate-data.good.al). ## Anti Pattern Repairing data in `OnCheckPreconditions...` or finishing migration in `OnValidateUpgrade...`. Those writes blur the phase contract and make a check alter the state it is supposed to assess. -See sample: `check-only-triggers-do-not-migrate-data.bad.al`. +See sample: [`check-only-triggers-do-not-migrate-data.bad.al`](check-only-triggers-do-not-migrate-data.bad.al). diff --git a/microsoft/knowledge/upgrade/datatransfer-for-bulk-init.md b/microsoft/knowledge/upgrade/datatransfer-for-bulk-init.md index 988dfa4..830dbdb 100644 --- a/microsoft/knowledge/upgrade/datatransfer-for-bulk-init.md +++ b/microsoft/knowledge/upgrade/datatransfer-for-bulk-init.md @@ -17,13 +17,13 @@ Tables that can contain more than 300,000 records, and any newly added field on For a bulk update use a `DataTransfer` variable: call `SetTables(Database::"...", Database::"...")` (source and destination may be the same table), add filters with `AddSourceFilter`, set the target value with `AddConstantValue` (or copy a source field with `AddFieldValue`), and execute with `CopyFields()`. To express multiple distinct updates against the same table, `Clear` the `DataTransfer` between executions and configure the next one. -See sample: `datatransfer-for-bulk-init.good.al`. +See sample: [`datatransfer-for-bulk-init.good.al`](datatransfer-for-bulk-init.good.al). ## Anti Pattern Iterating with `FindSet(true) ... repeat ... Modify() ... until Next() = 0` to set a single field across an entire large table. On 300k+ rows this is the canonical slow-upgrade footgun. -See sample: `datatransfer-for-bulk-init.bad.al`. +See sample: [`datatransfer-for-bulk-init.bad.al`](datatransfer-for-bulk-init.bad.al). ## See also diff --git a/microsoft/knowledge/upgrade/datatransfer-skips-triggers-and-subscribers.md b/microsoft/knowledge/upgrade/datatransfer-skips-triggers-and-subscribers.md index 34a406b..33173da 100644 --- a/microsoft/knowledge/upgrade/datatransfer-skips-triggers-and-subscribers.md +++ b/microsoft/knowledge/upgrade/datatransfer-skips-triggers-and-subscribers.md @@ -19,10 +19,10 @@ For *new fields and tables added in the same change* this is fine: nothing yet d Use `DataTransfer` when set-based transfer is safe and row-level business logic is intentionally unnecessary โ€” initial population of a new field is the canonical case. When an existing field's validation must run, loop through records and call `Validate(Field, Value)`; if the table's modify trigger must also run, follow with `Modify(true)`. If performance requires `DataTransfer`, document exactly which field-validation and row-modification triggers or subscribers are intentionally bypassed and verify that derived data remains correct. -See sample: `datatransfer-skips-triggers-and-subscribers.good.al`. +See sample: [`datatransfer-skips-triggers-and-subscribers.good.al`](datatransfer-skips-triggers-and-subscribers.good.al). ## Anti Pattern Reaching for `DataTransfer` to update an existing field with non-trivial `OnValidate` or `OnModify` logic, without confirming that both validation and row-modification subscribers can be skipped. Replacing it with only `Modify(true)` is also incomplete when field validation is required; call `Validate` for that field first. -See sample: `datatransfer-skips-triggers-and-subscribers.bad.al`. +See sample: [`datatransfer-skips-triggers-and-subscribers.bad.al`](datatransfer-skips-triggers-and-subscribers.bad.al). diff --git a/microsoft/knowledge/upgrade/do-not-block-upgrade-on-data-errors.md b/microsoft/knowledge/upgrade/do-not-block-upgrade-on-data-errors.md index cf585eb..868b61e 100644 --- a/microsoft/knowledge/upgrade/do-not-block-upgrade-on-data-errors.md +++ b/microsoft/knowledge/upgrade/do-not-block-upgrade-on-data-errors.md @@ -17,10 +17,10 @@ When upgrade code encounters unexpected data โ€” a record it expected to find, a When an upgrade procedure detects something missing, call `Session.LogMessage` with a stable event ID, classify the message verbosity (typically `Warning`), and `exit` the procedure so the rest of the upgrade can proceed. The platform telemetry then surfaces the situation to the partner without breaking the customer. -See sample: `do-not-block-upgrade-on-data-errors.good.al`. +See sample: [`do-not-block-upgrade-on-data-errors.good.al`](do-not-block-upgrade-on-data-errors.good.al). ## Anti Pattern Calling `Record.Get(Key)` (or any other erroring API) and letting the error propagate out of the upgrade trigger. The first tenant with imperfect data fails to upgrade, and the failure surfaces as a hard upgrade error rather than as a telemetry signal. -See sample: `do-not-block-upgrade-on-data-errors.bad.al`. +See sample: [`do-not-block-upgrade-on-data-errors.bad.al`](do-not-block-upgrade-on-data-errors.bad.al). diff --git a/microsoft/knowledge/upgrade/enum-values-additive-at-end.md b/microsoft/knowledge/upgrade/enum-values-additive-at-end.md index 4de929b..332efa6 100644 --- a/microsoft/knowledge/upgrade/enum-values-additive-at-end.md +++ b/microsoft/knowledge/upgrade/enum-values-additive-at-end.md @@ -17,13 +17,13 @@ An AL `enum` is a fixed list of ordinal-named values. Persisted rows reference e When adding an enum value, place it after the last existing `value(N; ...)` entry, with an ordinal strictly greater than every existing one. Never renumber existing entries. To retire a value, do not delete it: mark it `ObsoleteState = Pending` (and later `Removed`) with `ObsoleteReason` and `ObsoleteTag` so the ordinal remains taken. -See sample: `enum-values-additive-at-end.good.al`. +See sample: [`enum-values-additive-at-end.good.al`](enum-values-additive-at-end.good.al). ## Anti Pattern Inserting a value between existing entries ("just put `NewMiddleValue` between `First` and `Second`"), or removing a value from the enum without first going through `ObsoleteState = Pending` โ†’ `Removed`. Every row whose persisted ordinal matched the removed or shifted value now reads as a different member. -See sample: `enum-values-additive-at-end.bad.al`. +See sample: [`enum-values-additive-at-end.bad.al`](enum-values-additive-at-end.bad.al). ## See also diff --git a/microsoft/knowledge/upgrade/first-install-dataversion-zero-check.md b/microsoft/knowledge/upgrade/first-install-dataversion-zero-check.md index 6324836..5cbdec8 100644 --- a/microsoft/knowledge/upgrade/first-install-dataversion-zero-check.md +++ b/microsoft/knowledge/upgrade/first-install-dataversion-zero-check.md @@ -17,13 +17,13 @@ On the first install of an extension on a tenant the platform records a zero dat In `OnInstallAppPerCompany`, fetch the current `ModuleInfo` via `NavApp.GetCurrentModuleInfo`, compare `AppInfo.DataVersion()` to `Version.Create('0.0.0.0')`, and run first-install seed logic only when they match. On a non-zero data version, follow the reinstall path or exit. -See sample: `first-install-dataversion-zero-check.good.al`. +See sample: [`first-install-dataversion-zero-check.good.al`](first-install-dataversion-zero-check.good.al). ## Anti Pattern Treating `OnInstallAppPerCompany` as if it always implies "fresh tenant". The trigger also fires when reinstalling over an existing data set; without the `0.0.0.0` guard, first-install seed code can run again and duplicate rows. -See sample: `first-install-dataversion-zero-check.bad.al`. +See sample: [`first-install-dataversion-zero-check.bad.al`](first-install-dataversion-zero-check.bad.al). ## See also diff --git a/microsoft/knowledge/upgrade/guard-database-reads.md b/microsoft/knowledge/upgrade/guard-database-reads.md index c5bc206..09a99e3 100644 --- a/microsoft/knowledge/upgrade/guard-database-reads.md +++ b/microsoft/knowledge/upgrade/guard-database-reads.md @@ -17,10 +17,10 @@ Inside an upgrade codeunit (or any procedure transitively invoked from `OnUpgrad Wrap every read in an `if`. `if Item.Get(No) then ...`, `if Customer.FindSet() then;`, `if not Vendor.FindLast() then exit;`. The empty-then form `if Customer.FindSet() then;` is the idiomatic way to attempt a read whose only purpose is to position a record, while swallowing the "not found" case. -See sample: `guard-database-reads.good.al`. +See sample: [`guard-database-reads.good.al`](guard-database-reads.good.al). ## Anti Pattern Calling `Item.Get()`, `Customer.FindSet()`, or `Vendor.FindLast()` bare in upgrade code. The first tenant whose data does not match the upgrade's assumptions will fail to upgrade. -See sample: `guard-database-reads.bad.al`. +See sample: [`guard-database-reads.bad.al`](guard-database-reads.bad.al). diff --git a/microsoft/knowledge/upgrade/initvalue-does-not-update-existing-rows.md b/microsoft/knowledge/upgrade/initvalue-does-not-update-existing-rows.md index 4733ef2..bfa0f03 100644 --- a/microsoft/knowledge/upgrade/initvalue-does-not-update-existing-rows.md +++ b/microsoft/knowledge/upgrade/initvalue-does-not-update-existing-rows.md @@ -23,10 +23,10 @@ Several legitimate cases do NOT need upgrade code: When a new field on an existing table has an `InitValue` that matters, ship an upgrade procedure that walks the existing rows and sets the field to the same value โ€” typically via `DataTransfer.AddConstantValue` for performance โ€” guarded by an upgrade tag. -See sample: `initvalue-does-not-update-existing-rows.good.al`. +See sample: [`initvalue-does-not-update-existing-rows.good.al`](initvalue-does-not-update-existing-rows.good.al). ## Anti Pattern Adding a field with `InitValue = true;` (or any non-default `InitValue`) and shipping no upgrade code. Existing rows silently carry the datatype default, leaving the table in two states: rows created before the upgrade with the wrong value, and rows created after with the right one. -See sample: `initvalue-does-not-update-existing-rows.bad.al`. +See sample: [`initvalue-does-not-update-existing-rows.bad.al`](initvalue-does-not-update-existing-rows.bad.al). diff --git a/microsoft/knowledge/upgrade/install-code-does-not-run-on-version-upgrade.md b/microsoft/knowledge/upgrade/install-code-does-not-run-on-version-upgrade.md index 12f432f..52c3989 100644 --- a/microsoft/knowledge/upgrade/install-code-does-not-run-on-version-upgrade.md +++ b/microsoft/knowledge/upgrade/install-code-does-not-run-on-version-upgrade.md @@ -17,10 +17,10 @@ An install codeunit runs when an extension is installed for the first time or an Use `Subtype = Install` for first-install and reinstall initialization. Put version migration in a separate `Subtype = Upgrade` codeunit and enter it from `OnUpgradePerCompany` or `OnUpgradePerDatabase`. -See sample: `install-code-does-not-run-on-version-upgrade.good.al`. +See sample: [`install-code-does-not-run-on-version-upgrade.good.al`](install-code-does-not-run-on-version-upgrade.good.al). ## Anti Pattern Putting a schema or data migration only in an install trigger and expecting it to run when a higher app version is upgraded. The migration is never invoked on that path. -See sample: `install-code-does-not-run-on-version-upgrade.bad.al`. +See sample: [`install-code-does-not-run-on-version-upgrade.bad.al`](install-code-does-not-run-on-version-upgrade.bad.al). diff --git a/microsoft/knowledge/upgrade/minimize-onvalidate-upgrade-triggers.md b/microsoft/knowledge/upgrade/minimize-onvalidate-upgrade-triggers.md index b9e5e13..3095655 100644 --- a/microsoft/knowledge/upgrade/minimize-onvalidate-upgrade-triggers.md +++ b/microsoft/knowledge/upgrade/minimize-onvalidate-upgrade-triggers.md @@ -17,10 +17,10 @@ Triggers such as `OnValidateUpgradePerCompany` run on every upgrade pass. A full Filter directly to invalid rows and use `IsEmpty` or another bounded existence check where possible. If a broad validation is unavoidable, document the invariant that requires it and keep all data changes in `OnUpgrade...`. -See sample: `minimize-onvalidate-upgrade-triggers.good.al`. +See sample: [`minimize-onvalidate-upgrade-triggers.good.al`](minimize-onvalidate-upgrade-triggers.good.al). ## Anti Pattern Reading every record in `OnValidateUpgradePerCompany` when a filtered existence check can prove the same invariant. The scan repeats on every upgrade. -See sample: `minimize-onvalidate-upgrade-triggers.bad.al`. +See sample: [`minimize-onvalidate-upgrade-triggers.bad.al`](minimize-onvalidate-upgrade-triggers.bad.al). diff --git a/microsoft/knowledge/upgrade/no-external-calls-in-upgrade.md b/microsoft/knowledge/upgrade/no-external-calls-in-upgrade.md index eb644b6..f04fb40 100644 --- a/microsoft/knowledge/upgrade/no-external-calls-in-upgrade.md +++ b/microsoft/knowledge/upgrade/no-external-calls-in-upgrade.md @@ -19,10 +19,10 @@ The rule applies inside any codeunit with `Subtype = Upgrade` and to any procedu Defer external calls to runtime code. If a piece of upgrade work conceptually needs data from an external service, set a flag or write a queue row during upgrade and have the runtime code make the call later (for example on first user sign-in or via job queue), where retries and degraded modes are tractable. -See sample: `no-external-calls-in-upgrade.good.al`. +See sample: [`no-external-calls-in-upgrade.good.al`](no-external-calls-in-upgrade.good.al). ## Anti Pattern Calling `HttpClient.Get`, `HttpClient.Post`, or DotNet interop methods from `OnUpgradePerCompany`, `OnUpgradePerDatabase`, or any procedure they invoke. -See sample: `no-external-calls-in-upgrade.bad.al`. +See sample: [`no-external-calls-in-upgrade.bad.al`](no-external-calls-in-upgrade.bad.al). diff --git a/microsoft/knowledge/upgrade/obsolete-pending-to-removed-staging.md b/microsoft/knowledge/upgrade/obsolete-pending-to-removed-staging.md index cb008ac..64a54f2 100644 --- a/microsoft/knowledge/upgrade/obsolete-pending-to-removed-staging.md +++ b/microsoft/knowledge/upgrade/obsolete-pending-to-removed-staging.md @@ -17,10 +17,10 @@ application-area: [all] Stage the deprecation across releases. Step 1: mark `Pending` with reason and tag; consumers are warned but data and code keep working. Step 2: in a later release, transition to `Removed` and (if persisted data references the element) ship an upgrade procedure that migrates that data โ€” gated by an upgrade tag. The standard mechanic for retiring the actual implementation body is to remove the `#if not CLEAN` block in the same release that flips the state to `Removed`. -See sample: `obsolete-pending-to-removed-staging.good.al`. +See sample: [`obsolete-pending-to-removed-staging.good.al`](obsolete-pending-to-removed-staging.good.al). ## Anti Pattern Jumping straight to `ObsoleteState = Removed` without a prior `Pending` release. Consumers have no deprecation window to migrate and any data still referencing the element is stranded. Equally wrong: leaving an element `Pending` indefinitely and never staging its removal โ€” the deprecation never completes. -See sample: `obsolete-pending-to-removed-staging.bad.al`. +See sample: [`obsolete-pending-to-removed-staging.bad.al`](obsolete-pending-to-removed-staging.bad.al). diff --git a/microsoft/knowledge/upgrade/obsoletion-requires-reason-and-tag.md b/microsoft/knowledge/upgrade/obsoletion-requires-reason-and-tag.md index d6ec37a..b468437 100644 --- a/microsoft/knowledge/upgrade/obsoletion-requires-reason-and-tag.md +++ b/microsoft/knowledge/upgrade/obsoletion-requires-reason-and-tag.md @@ -22,13 +22,13 @@ In both forms, the reason should name the replacement and the tag should identif For an object or field, set all three properties together. For a method, variable, or event, provide both `[Obsolete]` arguments. Keep the original tag stable through the lifecycle rather than changing it to a planned removal version. -See sample: `obsoletion-requires-reason-and-tag.good.al`. +See sample: [`obsoletion-requires-reason-and-tag.good.al`](obsoletion-requires-reason-and-tag.good.al). ## Anti Pattern Setting only `ObsoleteState = Pending`/`Removed` on an object or field, or using `[Obsolete('', '')]` on a method, variable, or event. Both forms produce deprecation metadata without useful replacement guidance or traceability. -See sample: `obsoletion-requires-reason-and-tag.bad.al`. +See sample: [`obsoletion-requires-reason-and-tag.bad.al`](obsoletion-requires-reason-and-tag.bad.al). ## See also diff --git a/microsoft/knowledge/upgrade/register-upgrade-tags-with-subscribers.md b/microsoft/knowledge/upgrade/register-upgrade-tags-with-subscribers.md index e5e1983..0bba7c2 100644 --- a/microsoft/knowledge/upgrade/register-upgrade-tags-with-subscribers.md +++ b/microsoft/knowledge/upgrade/register-upgrade-tags-with-subscribers.md @@ -19,10 +19,10 @@ Registration is not install-time seeding. When an extension is installed into an In the upgrade codeunit, guard work with `HasUpgradeTag` and call `SetUpgradeTag` only after successful completion. Seed the same tag explicitly from `OnInstallAppPerCompany` when first-install logic should not run as a later upgrade. Also add historical per-company tags to `OnGetPerCompanyUpgradeTags` so `SetAllUpgradeTags` marks them complete for newly created companies. Keep the tag definition shared so all paths use the exact same value. -See sample: `register-upgrade-tags-with-subscribers.good.al`. +See sample: [`register-upgrade-tags-with-subscribers.good.al`](register-upgrade-tags-with-subscribers.good.al). ## Anti Pattern Assuming an `OnGetPerCompanyUpgradeTags` subscriber sets tags during extension installation, or omitting the subscriber and allowing old upgrade steps to run when `SetAllUpgradeTags` initializes a new company. The subscriber supplies a list; only `SetAllUpgradeTags` or an explicit `SetUpgradeTag` call persists it. -See sample: `register-upgrade-tags-with-subscribers.bad.al`. +See sample: [`register-upgrade-tags-with-subscribers.bad.al`](register-upgrade-tags-with-subscribers.bad.al). diff --git a/microsoft/knowledge/upgrade/skip-nonessential-work-via-execution-context.md b/microsoft/knowledge/upgrade/skip-nonessential-work-via-execution-context.md index 0b441e6..c4558b7 100644 --- a/microsoft/knowledge/upgrade/skip-nonessential-work-via-execution-context.md +++ b/microsoft/knowledge/upgrade/skip-nonessential-work-via-execution-context.md @@ -19,10 +19,10 @@ This is the opposite of a load-bearing concern: code that MUST run during the up In a runtime procedure that performs non-essential side effects, guard the side-effect block with `if GetExecutionContext() = ExecutionContext::Upgrade then exit;` and include a brief comment explaining what is being skipped and why. -See sample: `skip-nonessential-work-via-execution-context.good.al`. +See sample: [`skip-nonessential-work-via-execution-context.good.al`](skip-nonessential-work-via-execution-context.good.al). ## Anti Pattern Using `GetExecutionContext()` to *enable* upgrade behaviour from outside an upgrade codeunit. Upgrade behaviour belongs in a codeunit with `Subtype = Upgrade`; runtime code should only use the check to *suppress* optional work. -See sample: `skip-nonessential-work-via-execution-context.bad.al`. +See sample: [`skip-nonessential-work-via-execution-context.bad.al`](skip-nonessential-work-via-execution-context.bad.al). diff --git a/microsoft/knowledge/upgrade/triggers-call-helpers-not-implementations.md b/microsoft/knowledge/upgrade/triggers-call-helpers-not-implementations.md index dcc21e3..078e109 100644 --- a/microsoft/knowledge/upgrade/triggers-call-helpers-not-implementations.md +++ b/microsoft/knowledge/upgrade/triggers-call-helpers-not-implementations.md @@ -19,10 +19,10 @@ Empty `OnUpgradePerCompany` / `OnUpgradePerDatabase` triggers are acceptable โ€” Each upgrade trigger contains an ordered list of procedure calls, one per feature: `UpgradeFeatureA();` `UpgradeFeatureB();`. Each procedure handles its own upgrade tag, its own data work, and can be added or removed independently. -See sample: `triggers-call-helpers-not-implementations.good.al`. +See sample: [`triggers-call-helpers-not-implementations.good.al`](triggers-call-helpers-not-implementations.good.al). ## Anti Pattern Implementing record loops, `ModifyAll`, or other data work directly in the trigger body. The trigger then mixes orchestration with implementation, and adding a second feature requires editing the trigger rather than appending one line. -See sample: `triggers-call-helpers-not-implementations.bad.al`. +See sample: [`triggers-call-helpers-not-implementations.bad.al`](triggers-call-helpers-not-implementations.bad.al). diff --git a/microsoft/knowledge/upgrade/upgrade-codeunit-subtype.md b/microsoft/knowledge/upgrade/upgrade-codeunit-subtype.md index a9fee7c..8bf558d 100644 --- a/microsoft/knowledge/upgrade/upgrade-codeunit-subtype.md +++ b/microsoft/knowledge/upgrade/upgrade-codeunit-subtype.md @@ -17,10 +17,10 @@ A codeunit only participates in the upgrade pipeline when it sets `Subtype = Upg Place every piece of upgrade logic in a codeunit declared with `Subtype = Upgrade;` and expose entry points via the two triggers `OnUpgradePerCompany` and `OnUpgradePerDatabase`. Helper procedures may live in normal codeunits, but they inherit the upgrade-context rules (guarded reads, no external calls, upgrade tags, etc.) when called from an upgrade trigger. -See sample: `upgrade-codeunit-subtype.good.al`. +See sample: [`upgrade-codeunit-subtype.good.al`](upgrade-codeunit-subtype.good.al). ## Anti Pattern Putting upgrade-style logic in a regular codeunit that the platform never invokes during upgrade โ€” for example a normal codeunit with a manually invented "RunUpgrade" procedure that nothing wires to the upgrade pipeline. The migration code will simply not run. -See sample: `upgrade-codeunit-subtype.bad.al`. +See sample: [`upgrade-codeunit-subtype.bad.al`](upgrade-codeunit-subtype.bad.al). diff --git a/microsoft/knowledge/upgrade/use-upgrade-tags-not-version-checks.md b/microsoft/knowledge/upgrade/use-upgrade-tags-not-version-checks.md index 62347d1..bb7649f 100644 --- a/microsoft/knowledge/upgrade/use-upgrade-tags-not-version-checks.md +++ b/microsoft/knowledge/upgrade/use-upgrade-tags-not-version-checks.md @@ -17,13 +17,13 @@ Each piece of upgrade logic must run exactly once per company (or database) acro Every upgrade procedure starts with a `HasUpgradeTag` guard and ends with `SetUpgradeTag` once the work is committed. Each feature gets its own tag string so features can be re-run independently if needed. -See sample: `use-upgrade-tags-not-version-checks.good.al`. +See sample: [`use-upgrade-tags-not-version-checks.good.al`](use-upgrade-tags-not-version-checks.good.al). ## Anti Pattern Branching on `MyApp.DataVersion().Major > N`, or chains of `< N` / `< M` to decide which upgrade step to run. Such code becomes unmaintainable after a few releases and silently does the wrong thing on tenants that skip versions. -See sample: `use-upgrade-tags-not-version-checks.bad.al`. +See sample: [`use-upgrade-tags-not-version-checks.bad.al`](use-upgrade-tags-not-version-checks.bad.al). ## See also diff --git a/microsoft/knowledge/web-services/api-enum-values-are-a-contract-by-name-not-ordinal.md b/microsoft/knowledge/web-services/api-enum-values-are-a-contract-by-name-not-ordinal.md index 7988326..66fe36e 100644 --- a/microsoft/knowledge/web-services/api-enum-values-are-a-contract-by-name-not-ordinal.md +++ b/microsoft/knowledge/web-services/api-enum-values-are-a-contract-by-name-not-ordinal.md @@ -21,7 +21,7 @@ LLMs treat one carrier as universal. Some assume the caption is serialised and r Establish which schema versions the field is served under before changing anything about its enum. Under schema 2.0 (Microsoft's API v2.0, an explicit `$schemaversion=2.0` in the consumer contract, or another reliable context signal) the member name is the contract: keep names stable, put wording changes in `Caption`, add a value by appending a new name with an ordinal above every existing one, and retire a value through `ObsoleteState` rather than by deleting it. For a custom API that clients may still call as schema 1.0, any install of BC 17 to 23 or a caller that pins 1.0, the caption is a contract as well: change neither name nor caption in place, or publish the change as a new `APIVersion` on a new page object. A rename is out in every case: AppSourceCop AS0082 rejects it against a baseline, and dependent extensions bind to the name. -See sample: `api-enum-values-are-a-contract-by-name-not-ordinal.good.al`. +See sample: [`api-enum-values-are-a-contract-by-name-not-ordinal.good.al`](api-enum-values-are-a-contract-by-name-not-ordinal.good.al). ## Anti Pattern @@ -31,7 +31,7 @@ Detection signal: a diff hunk that changes the name in a `value(...)` line while The mirror image is a review defect: suppressing a caption-change finding because "the API serialises names". That holds only under schema 2.0. Do not flag a `Caption` change when the reviewer can establish schema 2.0 for every consumer; on a custom API where clients may select schema 1.0, report a caption change on an exposed value as a consumer-visible change and ask for versioning. A value appended at the end changes no contract under either schema and is never a finding. -See sample: `api-enum-values-are-a-contract-by-name-not-ordinal.bad.al`. +See sample: [`api-enum-values-are-a-contract-by-name-not-ordinal.bad.al`](api-enum-values-are-a-contract-by-name-not-ordinal.bad.al). ## See also diff --git a/microsoft/knowledge/web-services/disable-write-operations-on-read-only-api-pages.md b/microsoft/knowledge/web-services/disable-write-operations-on-read-only-api-pages.md index 2358a6b..8f6a73c 100644 --- a/microsoft/knowledge/web-services/disable-write-operations-on-read-only-api-pages.md +++ b/microsoft/knowledge/web-services/disable-write-operations-on-read-only-api-pages.md @@ -17,10 +17,10 @@ An API meant purely for reading โ€” a reporting or lookup endpoint โ€” is not re For a read-only / reporting API page set all three CRUD guards off โ€” `InsertAllowed = false`, `ModifyAllowed = false`, `DeleteAllowed = false` โ€” and mark the page `Editable = false`. The endpoint then serves GET requests and rejects any insert, modify, or delete, matching the read-only contract regardless of the caller. Make the read-only stance explicit rather than depending on the writable default. -See sample: `disable-write-operations-on-read-only-api-pages.good.al`. +See sample: [`disable-write-operations-on-read-only-api-pages.good.al`](disable-write-operations-on-read-only-api-pages.good.al). ## Anti Pattern An API intended for read-only consumption that omits the CRUD guards, leaving `InsertAllowed`, `ModifyAllowed`, and `DeleteAllowed` at their writable defaults. The endpoint silently accepts POST, PATCH, and DELETE, so a client can mutate or remove data the API was never meant to expose for writing. The detection signal: a read-only/reporting `PageType = API` page that does not set the three `*Allowed = false` properties. -See sample: `disable-write-operations-on-read-only-api-pages.bad.al`. +See sample: [`disable-write-operations-on-read-only-api-pages.bad.al`](disable-write-operations-on-read-only-api-pages.bad.al). diff --git a/microsoft/knowledge/web-services/expose-only-committed-data-from-api-reads.md b/microsoft/knowledge/web-services/expose-only-committed-data-from-api-reads.md index 739b5aa..4337b41 100644 --- a/microsoft/knowledge/web-services/expose-only-committed-data-from-api-reads.md +++ b/microsoft/knowledge/web-services/expose-only-committed-data-from-api-reads.md @@ -17,10 +17,10 @@ This is about the data-consistency contract of an API endpoint: what a consumer For an API page that must expose only committed data, set the endpoint's read isolation once as the page opens: in the `OnOpenPage` trigger write `Rec.ReadIsolation := IsolationLevel::ReadCommitted;`. Every read the endpoint then serves ignores uncommitted writes from concurrent transactions, so a consumer never receives a row that another transaction might still roll back. -See sample: `expose-only-committed-data-from-api-reads.good.al`. +See sample: [`expose-only-committed-data-from-api-reads.good.al`](expose-only-committed-data-from-api-reads.good.al). ## Anti Pattern An API intended to return committed-only data that sets no isolation level, leaving reads at the default that can observe in-flight, uncommitted writes. A consumer can fetch a row created by a concurrent transaction that is later rolled back โ€” a dirty read that surfaces data which never durably existed. The detection signal: a committed-only read API with no `Rec.ReadIsolation := IsolationLevel::ReadCommitted` in `OnOpenPage`. -See sample: `expose-only-committed-data-from-api-reads.bad.al`. +See sample: [`expose-only-committed-data-from-api-reads.bad.al`](expose-only-committed-data-from-api-reads.bad.al). diff --git a/microsoft/knowledge/web-services/expose-operations-as-bound-actions.md b/microsoft/knowledge/web-services/expose-operations-as-bound-actions.md index 7f0ed87..18908a7 100644 --- a/microsoft/knowledge/web-services/expose-operations-as-bound-actions.md +++ b/microsoft/knowledge/web-services/expose-operations-as-bound-actions.md @@ -17,10 +17,10 @@ An API consumer that needs to *do* something to a record โ€” post it, ship it, r Declare the operation as `[ServiceEnabled] procedure Post(var ActionContext: WebServiceActionContext)` on the API page. Inside, perform the operation against `Rec`, then call a `SetActionResponse` helper that writes the result โ€” the bound record and its id โ€” back into the `WebServiceActionContext` so the caller receives a well-formed response. The operation is now an explicit, named endpoint action separate from ordinary field writes. -See sample: `expose-operations-as-bound-actions.good.al`. +See sample: [`expose-operations-as-bound-actions.good.al`](expose-operations-as-bound-actions.good.al). ## Anti Pattern Exposing a writable Boolean (for example `posted`) whose `OnValidate` performs the posting. A client that PATCHes the field to `true` โ€” an action indistinguishable from any other data edit โ€” silently triggers a side-effecting business operation. The detection signal: an API page field whose `OnValidate` posts, ships, or releases, instead of a `[ServiceEnabled]` bound action. -See sample: `expose-operations-as-bound-actions.bad.al`. +See sample: [`expose-operations-as-bound-actions.bad.al`](expose-operations-as-bound-actions.bad.al). diff --git a/microsoft/knowledge/web-services/expose-systemid-as-the-api-key.md b/microsoft/knowledge/web-services/expose-systemid-as-the-api-key.md index 93d2dcd..f34e9a4 100644 --- a/microsoft/knowledge/web-services/expose-systemid-as-the-api-key.md +++ b/microsoft/knowledge/web-services/expose-systemid-as-the-api-key.md @@ -17,10 +17,10 @@ Every BC table carries a `SystemId` โ€” an immutable GUID assigned at insert and Set `ODataKeyFields = SystemId` so OData routes records by the stable GUID, and expose it as `field(id; Rec.SystemId)` marked `Editable = false`. Clients then address a record at `.../customers()`, an identity that survives any rename of the business key. Keep the business key (for example `No.`) as an ordinary exposed field, not as the OData key. -See sample: `expose-systemid-as-the-api-key.good.al`. +See sample: [`expose-systemid-as-the-api-key.good.al`](expose-systemid-as-the-api-key.good.al). ## Anti Pattern Setting `ODataKeyFields = "No."` so the endpoint addresses records by a renamable business field. As soon as a user changes that `No.`, every external reference built on the old value points at nothing, silently breaking integrations. The detection signal: `ODataKeyFields` set to a business field rather than `SystemId`, or an API page that exposes no `id` field bound to `Rec.SystemId`. -See sample: `expose-systemid-as-the-api-key.bad.al`. +See sample: [`expose-systemid-as-the-api-key.bad.al`](expose-systemid-as-the-api-key.bad.al). diff --git a/microsoft/knowledge/web-services/link-api-parts-on-systemid-and-set-multiplicity.md b/microsoft/knowledge/web-services/link-api-parts-on-systemid-and-set-multiplicity.md index 4cf81d6..2f92c0c 100644 --- a/microsoft/knowledge/web-services/link-api-parts-on-systemid-and-set-multiplicity.md +++ b/microsoft/knowledge/web-services/link-api-parts-on-systemid-and-set-multiplicity.md @@ -17,13 +17,13 @@ application-area: [all] Define the child foreign key as `Guid` with a `TableRelation` to the parent table's `SystemId`, then use `SubPageLink = "" = Field(SystemId)` on the parent API page. A child collection may omit `Multiplicity` and rely on the default 1:N relationship, or declare `Multiplicity = Many` explicitly. Set `Multiplicity = ZeroOrOne` when the intended navigation metadata is a singleton. -See sample: `link-api-parts-on-systemid-and-set-multiplicity.good.al`. +See sample: [`link-api-parts-on-systemid-and-set-multiplicity.good.al`](link-api-parts-on-systemid-and-set-multiplicity.good.al). ## Anti Pattern On a parent API with `ODataKeyFields = SystemId`, linking a child business field such as `"Order No."` to the parent's `"No."` creates a second identity scheme for navigation instead of using the contract's stable GUID. A separate defect is an explicit `Multiplicity` that conflicts with the intended shape, such as `ZeroOrOne` on an order-lines collection or `Many` on a singleton. Do not treat omission alone as a defect: it is valid for a collection because the default is 1:N, while an intended singleton must explicitly use `Multiplicity = ZeroOrOne`. -See sample: `link-api-parts-on-systemid-and-set-multiplicity.bad.al`. +See sample: [`link-api-parts-on-systemid-and-set-multiplicity.bad.al`](link-api-parts-on-systemid-and-set-multiplicity.bad.al). ## Source diff --git a/microsoft/knowledge/web-services/set-required-api-page-properties.md b/microsoft/knowledge/web-services/set-required-api-page-properties.md index 496db1a..24edc5d 100644 --- a/microsoft/knowledge/web-services/set-required-api-page-properties.md +++ b/microsoft/knowledge/web-services/set-required-api-page-properties.md @@ -17,10 +17,10 @@ An API page needs `APIPublisher`, `APIGroup`, `EntityName`, `EntitySetName`, and Declare the five routing/entity properties required by the API page and set `APIVersion` explicitly for a stable published contract, for example `'v1.0'`. Expose the record's fields inside a repeater under `area(content)`. Review missing routing metadata as a malformed API definition, but review a missing `APIVersion` as unintended publication under `beta`, not as an unpublished endpoint. -See sample: `set-required-api-page-properties.good.al`. +See sample: [`set-required-api-page-properties.good.al`](set-required-api-page-properties.good.al). ## Anti Pattern Leaving out `APIPublisher`, `APIGroup`, `EntityName`, `EntitySetName`, or `SourceTable` leaves the API definition incomplete. A subtler contract defect is declaring all of those but omitting `APIVersion`: the page is exposed as `beta`, which is valid runtime behavior but not the explicit stable route a production client expects. -See sample: `set-required-api-page-properties.bad.al`. +See sample: [`set-required-api-page-properties.bad.al`](set-required-api-page-properties.bad.al). diff --git a/microsoft/knowledge/web-services/version-apis-by-adding-not-mutating-published-versions.md b/microsoft/knowledge/web-services/version-apis-by-adding-not-mutating-published-versions.md index bfd2c9f..1e8417b 100644 --- a/microsoft/knowledge/web-services/version-apis-by-adding-not-mutating-published-versions.md +++ b/microsoft/knowledge/web-services/version-apis-by-adding-not-mutating-published-versions.md @@ -17,10 +17,10 @@ Once an API version is published, external clients depend on its exact shape โ€” Keep the existing page object and its `APIVersion = 'v1.0'` contract unchanged. Copy the page to a new object ID, set that object's `APIVersion = 'v2.0'`, and make the v2-only shape changes there. A multi-value `APIVersion` list is appropriate only when the exact same page shape is supported under each listed version. -See sample: `version-apis-by-adding-not-mutating-published-versions.good.al`. +See sample: [`version-apis-by-adding-not-mutating-published-versions.good.al`](version-apis-by-adding-not-mutating-published-versions.good.al). ## Anti Pattern Editing the published `v1.0` page in place breaks its clients. So does adding `v2.0` to that same page and assuming subsequent field changes apply only to v2: both routes use one object shape. The detection signal is a breaking shape change without a separate API page object retaining the old version. -See sample: `version-apis-by-adding-not-mutating-published-versions.bad.al`. +See sample: [`version-apis-by-adding-not-mutating-published-versions.bad.al`](version-apis-by-adding-not-mutating-published-versions.bad.al). diff --git a/microsoft/knowledge/web-services/webhook-eligibility-and-validationtoken-renewal.md b/microsoft/knowledge/web-services/webhook-eligibility-and-validationtoken-renewal.md index b35a05c..2aad620 100644 --- a/microsoft/knowledge/web-services/webhook-eligibility-and-validationtoken-renewal.md +++ b/microsoft/knowledge/web-services/webhook-eligibility-and-validationtoken-renewal.md @@ -17,13 +17,13 @@ Business Central can subscribe only to eligible API pages, not every endpoint th Before creating a subscription, confirm the resource appears in `webhookSupportedResources` and that a custom endpoint is an API page with a single stable key over an eligible persistent table. Use one validation path that echoes `validationToken` for both create (`POST`) and renew (`PATCH`) handshakes. Track `expirationDateTime` and renew before expiry: online subscriptions expire after three days, while on-premises lifetime defaults to three days and can be changed with `ApiSubscriptionExpiration`. -See samples: `webhook-eligibility-and-validationtoken-renewal.good.al` and `webhook-eligibility-and-validationtoken-renewal.good.js`. +See samples: [`webhook-eligibility-and-validationtoken-renewal.good.al`](webhook-eligibility-and-validationtoken-renewal.good.al) and [`webhook-eligibility-and-validationtoken-renewal.good.js`](webhook-eligibility-and-validationtoken-renewal.good.js). ## Anti Pattern Attempting to subscribe to an API query, temporary/composite/system-table/Job Queue Entry API page, or assuming a successful create handshake makes renewal automatic. Composite includes an explicit multi-field `ODataKeyFields` and a missing `ODataKeyFields` when the source table's primary key has multiple fields. A renewal issues the same validation challenge; a notification handler that ignores the query-string token cannot create or renew the subscription. -See samples: `webhook-eligibility-and-validationtoken-renewal.bad.al` and `webhook-eligibility-and-validationtoken-renewal.bad.js`. +See samples: [`webhook-eligibility-and-validationtoken-renewal.bad.al`](webhook-eligibility-and-validationtoken-renewal.bad.al) and [`webhook-eligibility-and-validationtoken-renewal.bad.js`](webhook-eligibility-and-validationtoken-renewal.bad.js). ## Source diff --git a/microsoft/skills/review/al-breaking-changes-review.md b/microsoft/skills/review/al-breaking-changes-review.md index 767c9af..1ddd810 100644 --- a/microsoft/skills/review/al-breaking-changes-review.md +++ b/microsoft/skills/review/al-breaking-changes-review.md @@ -134,7 +134,7 @@ Output conforms to the DO output contract. Every finding this skill emits MUST s } ``` -The empty-corpus case โ€” BCQuality's state until breaking-changes knowledge files land โ€” produces: +When no applicable breaking-changes knowledge is available, the report is: ```json { diff --git a/microsoft/skills/review/al-code-review.md b/microsoft/skills/review/al-code-review.md index 41f0f78..9972aca 100644 --- a/microsoft/skills/review/al-code-review.md +++ b/microsoft/skills/review/al-code-review.md @@ -300,7 +300,9 @@ Output conforms to the DO output contract, extended with `sub-results` and `skip } ``` -The empty-corpus case โ€” BCQuality's state until knowledge files land โ€” rolls up to `no-knowledge`: +When the selected leaves find no applicable knowledge, the result rolls up to +`no-knowledge`. This example shows two leaf results; a full run includes every +invoked leaf: ```json { diff --git a/microsoft/skills/review/al-error-handling-review.md b/microsoft/skills/review/al-error-handling-review.md index bc4598a..63c4d5e 100644 --- a/microsoft/skills/review/al-error-handling-review.md +++ b/microsoft/skills/review/al-error-handling-review.md @@ -132,7 +132,7 @@ Output conforms to the DO output contract. Every finding this skill emits MUST s } ``` -The empty-corpus case โ€” BCQuality's state until error-handling knowledge files land โ€” produces: +When no applicable error-handling knowledge is available, the report is: ```json { diff --git a/microsoft/skills/review/al-events-review.md b/microsoft/skills/review/al-events-review.md index 7248a45..559d036 100644 --- a/microsoft/skills/review/al-events-review.md +++ b/microsoft/skills/review/al-events-review.md @@ -141,7 +141,7 @@ Output conforms to the DO output contract. Every finding this skill emits MUST s } ``` -The empty-corpus case โ€” BCQuality's state until events knowledge files land โ€” produces: +When no applicable events knowledge is available, the report is: ```json { diff --git a/microsoft/skills/review/al-performance-review.md b/microsoft/skills/review/al-performance-review.md index d4738ac..f2fa80d 100644 --- a/microsoft/skills/review/al-performance-review.md +++ b/microsoft/skills/review/al-performance-review.md @@ -134,7 +134,7 @@ Output conforms to the DO output contract. Every finding this skill emits MUST s } ``` -The empty-corpus case โ€” BCQuality's state until performance knowledge files land โ€” produces: +When no applicable performance knowledge is available, the report is: ```json { diff --git a/microsoft/skills/review/al-security-review.md b/microsoft/skills/review/al-security-review.md index 5d998c9..e3e4049 100644 --- a/microsoft/skills/review/al-security-review.md +++ b/microsoft/skills/review/al-security-review.md @@ -129,7 +129,7 @@ Output conforms to the DO output contract. Every finding this skill emits MUST s } ``` -The empty-corpus case โ€” BCQuality's state until security knowledge files land โ€” produces: +When no applicable security knowledge is available, the report is: ```json { diff --git a/skills/do.md b/skills/do.md index 5234437..4ac433b 100644 --- a/skills/do.md +++ b/skills/do.md @@ -19,7 +19,12 @@ An action skill is a single markdown file with YAML frontmatter. It lives inside - `/community/skills/` โ€” community-contributed action skills. - `/custom/skills/` โ€” partner or customer action skills (typically in a consumer repo, not in BCQuality itself). -Action skills do not live at the repo root. The files in `/skills/` โ€” the three meta-skill contracts (READ, DO, WRITE) and the entry-point skill (`entry.md`, `kind: entry-point`) โ€” are the only skills that sit outside a layer. The entry-point skill structurally follows this same four-step pattern but produces a dispatch record rather than a findings-report; see `skills/entry.md` for its contract. +Action skills do not live at the repo root. Layer-independent files in +`/skills/` contain the three meta-skill contracts (READ, DO, WRITE), the +entry-point skill (`entry.md`, `kind: entry-point`), and host-format adapters. +Adapters are not action skills. Entry structurally follows the same +four-step pattern but produces a dispatch record rather than a findings-report; +see [entry.md](entry.md) for its contract. ## Skills hold mechanics; knowledge files hold BC facts @@ -319,15 +324,16 @@ A super-skill's top-level `suppressed[]` remains knowledge-file-only and is typi ## Worked example -A minimal action skill that cites applicable guidance for a changed AL file, without generating findings of its own: +A minimal action skill that reviews a changed AL file against applicable +guidance. Relevance alone never produces a finding: ```yaml --- kind: action-skill -id: cite-applicable-guidance +id: review-applicable-guidance version: 1 -title: Cite applicable guidance -description: Lists knowledge files relevant to a changed AL file. +title: Review applicable guidance +description: Reviews a changed AL file against applicable knowledge. inputs: [file-path] outputs: [findings-report] technologies: [al] @@ -345,7 +351,12 @@ Filter by `technologies: [al]` and `bc-version` matching the target environment. Intersect `keywords` with tokens derived from the target file's object name and changed members. ## Action -For each worklist entry, emit one finding with severity `info`, a message naming the concern, and a reference object pointing to the knowledge file. +Read each worklisted article in full and compare its normative guidance to the +input. Emit a finding only for a concrete violation or an observation the +article explicitly defines, with justified severity, evidence, and a reference +copied from the discovered article path. Do not report an article merely +because it was relevant. If every item was evaluated and none warrants a +finding, return `completed` with an empty `findings` array. ## Output Conforms to the DO output contract. diff --git a/skills/read.md b/skills/read.md index 6a2080d..f2e9c1e 100644 --- a/skills/read.md +++ b/skills/read.md @@ -7,7 +7,9 @@ title: Schema + Use โ€” how to read a knowledge file # READ -Every consumer of BCQuality โ€” an agent, an action skill, a human reviewer โ€” reads this file first. It defines what a knowledge file is, what fields it contains, what they mean, and how to reconcile multiple files. +Read this contract before interpreting knowledge files. Task execution starts +at [Entry](entry.md); READ is loaded on demand when a dispatched skill needs +it. It defines knowledge fields, their meaning, and how to reconcile files. This contract is stable. Changes require a PR approved by both maintainers. @@ -131,7 +133,7 @@ Rules: - A sample file is identified by the article's slug followed by a `..` suffix. The supported kinds are `good` and `bad`. Additional kinds MAY be introduced by a layer; consumers MUST ignore unknown kinds without failing. - The extension matches the technology (`al`, `ps1`, `js`, `kql`, โ€ฆ). A single article MAY carry samples in multiple technologies if the article's frontmatter `technologies` lists them. -- Articles MAY have a `good` sample only, a `bad` sample only, both, or neither. The article text SHOULD reference each sample it ships, using a relative path like `` `.good.al` ``. +- Articles MAY have a `good` sample only, a `bad` sample only, both, or neither. The article text SHOULD reference each sample it ships with a relative Markdown link whose label retains the backticked filename, like `` [`.good.al`](.good.al) ``. - Samples are **demonstration-only**. They are not deployed, not compiled as part of a published app, and not derived from the Business Central base application source. Each sample is self-contained and exists purely to make the accompanying article concrete for humans and agents. - Layer precedence applies to sample files the same way it applies to articles: a `/custom/knowledge//.good.al` overrides a `/microsoft/knowledge//.good.al` for the same article in the same layer hierarchy. diff --git a/skills/write.md b/skills/write.md index 8096f1a..c2edab5 100644 --- a/skills/write.md +++ b/skills/write.md @@ -16,7 +16,7 @@ Before authoring anything, confirm a knowledge file is the right artifact. BCQua - **Skills** (`*/skills/**`) hold only finder/applier mechanics โ€” how to discover, filter, worklist, and emit findings. See `skills/do.md`. - **Knowledge files** (`*/knowledge/**`) hold every Business-Central-specific fact a skill acts on. -A new BC fact is therefore a knowledge file, never a skill edit. In particular, if you arrived here because a review agent flagged something it should not have (a false positive) or missed something it should have caught, the remedy is a knowledge file โ€” apply the admission test in the [README](../README.md#what-belongs-here): *would a capable LLM get this wrong without the file?* If you find yourself editing a skill to stop it flagging something, stop and write a knowledge file instead. +A new BC fact is therefore a knowledge file, never a skill edit. In particular, if you arrived here because a review agent flagged something it should not have (a false positive) or missed something it should have caught, the remedy is a knowledge file โ€” apply the [admission test](../docs/contributing.md#what-belongs-here): *would a capable LLM get this wrong without the file?* If you find yourself editing a skill to stop it flagging something, stop and write a knowledge file instead. ### Negative knowledge is first-class @@ -56,6 +56,13 @@ Target under 100 lines. Ideal under 50. Long files almost always mean two concer Custom `##` sections are permitted when they serve the concern (for example, `## Applies to` for scope caveats or `## See also` for related files). Consumers are not required to understand them, so do not put load-bearing content there. +When adding or changing a platform claim, cite an authoritative public source +where available. A short `## References` section can link the relevant API, +property documentation, or public source definition. If no such source is +available, identify the evidence or policy basis explicitly; do not imply an +official guarantee. Keep the actual rule and its exceptions in normative +sections, not only in references. See [sources and examples](../docs/contributing.md#sources-and-examples). + ## No fenced code blocks Knowledge files do not contain code. Samples live as **sibling files** next to the article โ€” `.good.al`, `.bad.al`, etc. โ€” in the same knowledge-layer folder. See `skills/read.md` for the full convention. This keeps knowledge files retrieval-friendly and prevents code from drifting out of sync with BC platform changes buried inside prose. @@ -93,7 +100,7 @@ The `/custom/` layer is **empty by default** in the upstream `microsoft/BCQualit Before authoring or scaffolding any file under `/custom/knowledge/` or `/custom/skills/`, an author โ€” human or agent โ€” MUST confirm the working repository is **not** `microsoft/BCQuality`: - Check the `origin` remote: `git remote get-url origin`. If it points at `github.com/microsoft/BCQuality`, stop โ€” you are in the upstream repo, not a fork. -- If you are in the upstream repo, do not write the file. Either fork the repository (or clone it into your organization's own repo) and add the custom content there, or โ€” if the guidance is genuinely shareable โ€” author it in `/community/knowledge/` instead. +- If you are in the upstream repo, do not write the custom file. Either fork the repository (or clone it into your organization's own repo) and add the custom content there, or โ€” if the guidance is genuinely shareable โ€” use the shared layer that owns the domain, following *Choosing a layer* above. Community is not a staging area for Microsoft-owned domains. A pull request that adds `/custom/` content to `microsoft/BCQuality` will be **automatically closed** by the `Guard custom layer` workflow. Validate the fork precondition first so authoring effort is not wasted on a PR that cannot be merged. @@ -109,7 +116,8 @@ Before opening a pull request: - Frontmatter `domain` exactly matches the containing domain folder. - File is in the correct layer and domain folder. - Name is kebab-case and descriptive. -- Every companion sample is referenced by filename from the article, and every referenced sample exists. +- Every companion sample has a clickable relative link retaining its backticked filename, and every referenced sample exists. +- Platform claims link supporting sources where available; policy or empirical guidance is identified as such. - Every review-leaf domain has at least one article with both `.good.al` and `.bad.al` companions; the evaluation harness derives positive and clean controls from that convention automatically. Agents scaffolding new files SHOULD run this checklist programmatically before emitting the file.