* Add 18 community AL/BC patterns across style, data-modeling, web-services, appsource, breaking-changes, performance, and testing
Contributed by CURABIS ApS, generalized from patterns observed across real AppSource/PTE development. Each article follows the knowledge file format (frontmatter, Description/Best Practice/Anti Pattern, sibling .good.al/.bad.al samples).
* Address Jesper Schulz-Wedde's review on PR #156
- Rename 3 articles so their .good.al/.bad.al companion stems match
(do-not-change-primary-key, testfield-required-setup-field,
al-identifiers-english), fixing the R14 orphan-sample errors.
- do-not-change-primary-key.good.al: include Flow in the new table's
own primary key so it actually models the discriminating dimension.
- al-build-output-must-not-pollute-project-root.md: drop the
unsubstantiated AL0197 causal claim and the non-existent
al.outputPath setting; reframe as build-artifact hygiene sourced
from ALTool --outfolder / al_build outputPath.
- prefer-email-module.md: Email Message is Codeunit 8904, not a table;
distinguish it from the underlying Sent/Outbox/Draft storage.
- file-datatype-saas.md: File.Open/Create/Read/Write fails to compile
against a Cloud-scoped project, it does not compile and silently
fail at runtime.
- namespace-must-be-verified-from-source.md: narrow to "resolve from
the referenced object's source or symbols," since source-file line
one is not the only authoritative source (symbol packages, comments
before the namespace line).
- test-data-must-be-random-and-complete.md: drop "assume an empty
database" and "collision-free" absolutes; reframe around
independence from unrelated business records and reserving explicit
values for scenario-defining inputs.
- binary-choice-must-be-boolean.md: scope to genuine true/false
semantics, not mechanical two-member-enum-to-boolean conversion.
- document-report-word-layout.md: scope down to a sourced Microsoft
Learn recommendation instead of an unconditional performance
guarantee; cite the three Learn pages.
- Wire the new articles into their review skills' candidate-selection
signals (file-datatype-saas, prefer-email-module,
namespace-must-be-verified-from-source, var-parameters-require-an-
addressable-variable) so they can actually enter a worklist.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Fix dimension-management-wiring.md: ValidateShortcutDimCode and CreateDim
do not exist on the current DimensionManagement codeunit
Verified against microsoft/BCApps: the real master-table validation
procedure is ValidateDimValueCode (or ValidateShortcutDimValues when a
DimSetID is also needed), and the real document-side inheritance
procedure is GetDefaultDimID, not CreateDim. Caught from Jesper
Schulz-Wedde's review thread, which had been partially hidden by
GitHub's comment folding.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Address second round of Jesper Schulz-Wedde's review on PR #156
- dimension-management-wiring.md/.good.al: split into the two distinct
models the article was conflating - master data (Default Dimension
records via ValidateDimValueCode/SaveDefaultDim) vs. transactional/
document data (a single Dimension Set ID assembled via AddDimSource +
GetDefaultDimID, verified against BCApps' ExchRateAdjmtProcess.Codeunit.al).
Added a compiling document-table example alongside the existing master
table one.
- Deleted api-page-flowfields-must-be-calcfields (.md/.good.al/.bad.al):
Microsoft's own FlowFields documentation states a FlowField used as a
control's direct source expression is automatically calculated on any
page - no API-page exception is documented, and none could be
reproduced.
- prefer-email-module.bad.al/.md: Codeunit Mail has no Send/GetErrorDesc
members; fixed to the real current 7-argument CreateMessage signature,
and corrected the claim that the legacy path "still runs" - its base
implementation no longer sends anything, only raises integration events.
- check-post-line-batch-pattern.md/.good.al: reframed from a universal
invariant to the standard shape, naming the real Gen./Item/CA/Res./Job/
Insurance/Mfg. Item/FA Jnl.-Check Line/-Post Line/-Post Batch codeunits
it's based on. Added the missing Check Line companion codeunit so the
good fixture is internally complete.
- test-data-must-be-random-and-complete.good.al: removed leftover
"collision-free" wording contradicting the already-corrected article text.
- fixed-choice-set-must-use-enum-not-integer.md: removed the reintroduced
state-count heuristic ("the line is the state count"), aligned with
binary-choice-must-be-boolean.md's semantics-based distinction.
- namespace-must-be-verified-from-source.md: removed the false claim that
the compiler and AL Language Server use different namespace-resolution
rules.
- intrinsic-al-functions-must-use-modern-casing.md: removed the unverified
claim that PascalCase is the VS Code formatter's default output.
Worklist completeness: added cues for the 8 rules in data-modeling,
testing, performance, and web-services that had none (Jesper's explicit
ask), plus the same gap in all 7 style rules from this PR (not explicitly
named this round, but the identical systemic issue) - 15 cues total across
al-data-modeling-review.md, al-testing-review.md, al-performance-review.md,
al-web-services-review.md, and al-style-review.md.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Fix remaining correctness issues from Jesper's 2026-09-15 re-review
- dimension-management-wiring: SaveDefaultDim's third argument is the
shortcut dimension number (1-8), not the field's AL field ID; the
fixture passed FieldNo(...) = 10. GetDefaultDimID's InheritFromDimSetID
must be 0 when recomputing after the linking record changes, not the
document's existing Dimension Set ID (which would retain the previous
customer's leftover dimensions). Verified against
DimensionManagement.Codeunit.al and BankDepositHeader.Table.al in the
BCApps reference clone.
- check-post-line-batch-pattern: "Post Line writes exactly one line to
the ledger" overclaimed - Gen. Jnl.-Post Line alone calls InsertGLEntry
from a dozen call sites (balancing entry, VAT, currency rounding,
deferrals) and can write several G/L Entries per journal line.
Reworded to "posts exactly one journal line" and softened the
"distinct, non-overlapping responsibilities" absolute.
- namespace-must-be-verified-from-source.bad.al: dropped the "resolves
in a local build, fails in VS Code" comment (taught an inherent
compiler/language-server disagreement that isn't real); reframed as
stale/cached symbols, matching the prose fix already made.
- file-datatype-saas.good.al: replaced the deprecated 5-argument
UploadIntoStream overload with the current 2-argument one, and
actually staged through TempBlob as the article's own Best Practice
instructs (the declared TempBlob variable was previously unused).
- test-data-must-be-random-and-complete: no longer treats a
short-but-valid value as defective merely for being "underfilled" -
AL field lengths are maxima, not minimums. Scoped to missing values
or a scenario with an explicit length/format requirement (e.g. a
truncation test). Updated the al-testing-review.md routing cue to
match.
- stored-derived-fields-must-not-be-exposed-directly: stopped mandating
source-field exposure as part of the core pattern: the good fixture
exposed only one of the derived value's two inputs (Hours Used, not
Budgeted Hours), making the claimed "so the consumer can verify it"
impossible. Reframed as an optional, all-or-nothing addition and
fixed the fixture to expose both inputs.
Rebased onto upstream/main to resolve conflicts in
al-breaking-changes-review.md, al-data-modeling-review.md,
al-performance-review.md, and al-style-review.md against merged PRs
#148 and #153; all sides' worklist tokens/cues retained.
* Fix remaining READ-convention sample links across this PR's 18 articles
The same plain-backtick "See sample: \`x.good.al\`." form fixed on
al-methods-limited-during-write-transactions (PR #161) turned up
repo-wide on 15 more of this PR's articles - Knowledge-Retrieval.ps1
requires the markdown-link form to associate a sample with its
article. All 16 fixed; the four local validators (frontmatter,
knowledge-index, knowledge-retrieval, review-fixtures, skill-index)
pass.
* Fix two merge-critical correctness issues from Jesper's 2026-09-22 review
- api-page-key-fields-must-be-editable-on-insert.good.al and
stored-derived-fields-must-not-be-exposed-directly.good.al: both were
writable API pages missing DelayedInsert = true, contradicting this
repo's own api-page-delayedinsert-true rule - the canonical "good"
samples were teaching code BCQuality itself flags.
- dimension-management-wiring.good.al: UpdateDimensionSetID exited
early when Customer.Get failed, leaving the previous customer's
shortcut dimension and Dimension Set ID in place - the same staleness
bug the InheritFromDimSetID = 0 fix (from the prior review round) was
meant to prevent, just triggered by a failed lookup instead of a
successful one. Now clears the shortcut field and recomputes with an
empty source list on a failed lookup too, so GetDefaultDimID
correctly returns an empty Dimension Set ID instead of never running.
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
- log-writes-must-survive-rollback.good.al: fixed invalid trigger
OnRun(var Rec: ...) declaration; Rec is implicit when TableNo is set.
- exposed-objects-must-be-in-a-permission-set.md: distinguished the three
exposure mechanisms (page/query web service or API, codeunit published
as a web service, [ServiceEnabled] bound action on a page) and their
actual permission targets (page/query "..." = X vs codeunit "..." = X).
- code-must-not-change-workdate.md: scoped from an absolute "never" to
"not as a side effect of unrelated logic" - verified real WorkDate(x)
setter usage in BCApps demo-data generators and test codeunits.
- bcpt-scenarios-must-be-app-specific.md: SingleInstance and
StartScenario/EndScenario reframed as context-dependent patterns, not
mandatory requirements - BCPT Create Customer uses neither.
- test-feature-scenario-tags.good.al/.bad.al: replaced the invented
LibrarySales.CreateCustomerWithPrice/"Item Price Mgt." calls with a real,
verified price-list-line test using Library - Sales/Library - Inventory/
Library - Price Calculation.
- page-design-must-match-bc-page-type-conventions.md: scoped the missing
UsageCategory anti-pattern to pages intended as searchable entry points.
- defensive-vs-offensive-code-must-match-blast-radius.md/.good.al/.bad.al:
replaced the VAT registration number "low blast radius" example with a
genuinely cosmetic field (customer home page URL).
- source-organized-by-feature-not-object-type.md: anti-pattern reframed as
inconsistency with a repo's own convention, not the object-type scheme
itself.
- pictures-must-use-media-not-blob.md: removed leftover "image variants"
wording contradicting the already-corrected MediaSet description.
Proactively fixed while sweeping all fixtures for invented APIs:
- given-blocks-must-cover-full-precondition-chain.bad.al: PostSalesOrder
called with wrong arity and referenced an undeclared variable.
- ui-test-codeunit-naming.good.al/.bad.al: replaced the same fake
"Item Price Mgt."/TestPage "Item Price" with real Library - Sales calls
and the real Customer Card TestPage.
Worklist completeness: added review-skill cues for the 12 of 18 new rules
that had none (al-appsource-review.md, al-data-modeling-review.md,
al-error-handling-review.md, al-security-review.md, al-style-review.md x3,
al-testing-review.md x2, al-ui-review.md, al-upgrade-review.md,
al-web-services-review.md), and fixed test-feature-scenario-tags' cue,
which only matched the compliant (tagged) shape instead of the anti-pattern
(untagged/generic-named test).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Add bounded knowledge retrieval
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0d8b7764-f15a-49ea-8d50-d9147334f8af
* Fix bc-version overflow and pathless-row identity in bounded retrieval
Catalog matching compared an Int32 -BCVersion against a bigint range bound.
PowerShell coerces the right operand to the left operand's type, so a bound
wider than Int32 threw a conversion error and failed the whole domain catalog
rather than the single row. Metadata validation already accepts such bounds,
so compare as bigint on both sides.
The shared pager built its oversized-row message with $row.path, which
throws under Set-StrictMode -Version Latest when a row carries no path,
replacing the explicit bound failure with a property-lookup error. Resolve the
path defensively for dictionary and object rows so the offset-based fallback
is reachable.
Both paths gain regression coverage that fails without these fixes.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0d8b7764-f15a-49ea-8d50-d9147334f8af
* Document standalone review runner contract
Keep model selection and scheduling outside BCQuality while allowing orchestrators to run isolated review leaves concurrently with deterministic rollup semantics.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Support complete app folder reviews
Define folder-path as a current-state review scope and accept it across the standalone adapter, broad coordinator, and every AL review leaf.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Add walk-up app review quick start
Put the complete app-folder installation and prompt flow directly in the README so partners can discover the standalone experience without reading integration details first.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Organize conceptual guides under docs
Move architecture and standalone runner documentation out of the repository root, add a documentation index, and update all inbound links.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Complete AL review knowledge readiness
Fill telemetry and Query coverage, strengthen thin review domains, correct audited content defects, and add deterministic cheap-model evaluation and reference-integrity safeguards.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9825b012-e653-496a-9310-c1f4b6f8ac27
* Generalize review fixture discovery
Derive smoke cases from the leaf, domain, and paired-sample conventions so new leaves require no scoring-contract changes. Keep only exceptional selection/context overrides and fail when retrieval metadata cannot rank the selected article.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9825b012-e653-496a-9310-c1f4b6f8ac27
* Preserve published field IDs in sample
Keep the existing Email and Contact Email field IDs unchanged, clarify that the sample represents an independent baseline, and use a local breaking-change rule for the generic smoke evaluation.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9825b012-e653-496a-9310-c1f4b6f8ac27
* Clarify published field identity rules
State explicitly that a published field keeps its ID, name, and type while a replacement is added as a separate field under an unused ID.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9825b012-e653-496a-9310-c1f4b6f8ac27
* Align field obsoletion sample baselines
Use Email field ID 3 as the shared baseline so the bad example demonstrates a same-ID rename while the good example retains the original field and adds a separate replacement.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9825b012-e653-496a-9310-c1f4b6f8ac27
---------
Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
* Emit human-readable domain label on review findings
Add an optional findings[].domain field to the DO review output contract so
each finding carries its own human-readable review-domain display label. Leaf
review skills set it on every finding they emit; the al-code-review super-skill
copies it verbatim during rollup and sets it to "Agent" for its own
cross-cutting agent findings. This decouples consumers from BCQuality's domain
taxonomy: they render finding.domain verbatim instead of maintaining a
sub-skill-id -> label map.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Define domain display-label constraints
Clarify that review domains may contain internal whitespace, punctuation, case-sensitive text, and non-ASCII characters. Require consumers to preserve and safely encode the complete label instead of relying on lossy slugs, matching the replacement BC-ALAgents consumer.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 77d0a40e-8bf5-40ac-a450-40eb0255db03
---------
Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Seed web-services (API v2) knowledge domain
Add eight web-services API page knowledge articles (each with .good.al/.bad.al samples), a new al-web-services-review leaf skill, and wire it into al-code-review and the README.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Trim web-services domain to 6 non-duplicative articles
Drop API entity-naming/camelCase and DelayedInsert articles (owned by the style domain). Reframe the committed-data and API-versioning articles to stay strictly within the endpoint design/behavior lane, and update the leaf skill's worklist tokens and Output example accordingly.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
