mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-10-07 15:46:55 +01:00
Merge upstream/main; adopt articles[] eval override, add sample-link READ convention
- Resolve conflicts in al-data-modeling-review.md by keeping both sides' additions (folder-path support, InitRecord/Round cues from upstream; the 9 document-distribution/pricing/barcode cues from this branch). - Switch the data-modeling evaluation override from an ad-hoc additionalArticles field to upstream's now-established articles[] convention (used elsewhere for finance/scm/query/reporting/style), removing the redundant parallel code path from Test-ReviewFixtures.ps1. - Fix all 9 new articles' sample references to the markdown-link READ convention required by Knowledge-Retrieval.ps1's Assert-SampleLink (plain backticks satisfy validate_frontmatter.py's regex alone but not this stricter check - both validators must pass). Validators: frontmatter 0/0, review-fixtures 126 cases/20 domains PASSED, knowledge-index 342 articles/575 samples PASSED, skill-index 19 review leaves PASSED. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
commit
e22352b248
504 changed files with 9501 additions and 1969 deletions
|
|
@ -4,7 +4,7 @@ id: al-data-modeling-review
|
|||
version: 1
|
||||
title: AL data-modeling review
|
||||
description: Performs an AL data-modeling review against guidance from BCQuality.
|
||||
inputs: [pr-diff, file-path]
|
||||
inputs: [pr-diff, file-path, folder-path]
|
||||
outputs: [findings-report]
|
||||
bc-version: [all]
|
||||
technologies: [al]
|
||||
|
|
@ -16,11 +16,11 @@ application-area: [all]
|
|||
|
||||
Reviews AL source changes against the `data-modeling` knowledge domain in BCQuality and emits a findings report. This is a leaf action skill: it invokes no sub-skills. It is one of the skills composed by `al-code-review`.
|
||||
|
||||
An orchestrator invokes this skill with either a `pr-diff` (the standard PR-review entry point) or a `file-path` (single-file review). Data-modeling findings are narrow by design — they apply when the diff touches setup or master tables, their card pages, primary keys, number-series assignment, block enforcement, audit fields, document print/email/Post-and-Send actions, `Navigate` page subscribers, Report Selection registration or dispatch, price-calculation/price-source extensibility, `TransferFields`-based posting-cascade field mirroring, or barcode/report-layout font-provider usage. The skill returns `not-applicable` when none of those apply.
|
||||
An orchestrator invokes this skill with a `pr-diff`, `file-path`, or `folder-path`. Data-modeling findings are narrow by design — they apply when the review scope contains setup or master tables, their card pages, primary keys, number-series assignment, block enforcement, audit fields, document print/email/Post-and-Send actions, `Navigate` page subscribers, Report Selection registration or dispatch, price-calculation/price-source extensibility, `TransferFields`-based posting-cascade field mirroring, or barcode/report-layout font-provider usage. The skill returns `not-applicable` when none of those apply.
|
||||
|
||||
## Source
|
||||
|
||||
Read the BCQuality knowledge index once — the `knowledge-index.json` BCQuality builds at the root of the knowledge checkout (Entry's preparation step regenerates it over the live, already-filtered clone — see `skills/entry.md`). It lists every article that survived layer and allow/deny filtering and carries, per article, its `path`, `layer`, `domain`, frontmatter dimensions, `keywords`, `title`, and a one-line `description` hint — exactly the fields Relevance and Worklist consume. Take the index entries whose `domain` is `data-modeling` as this skill's candidate set across every enabled layer; do not open the individual article files at this step. Open an article's full body only once it enters the Worklist below, so a review reads the index plus the handful of worklisted articles instead of every file under `*/knowledge/data-modeling/**`.
|
||||
Use READ's **Bounded retrieval for review skills** workflow with `-Domain data-modeling`. Consume every catalog page across enabled layers before applying this leaf's Relevance and Worklist; preserve each exact catalog path and open complete bodies only for exact paths selected by the Worklist. If the helper or prepared index is unavailable or invalid, use READ's explicit path-discovery and bounded native-read fallback.
|
||||
|
||||
## Relevance
|
||||
|
||||
|
|
@ -39,7 +39,7 @@ Narrow the relevant files to the subset that applies to the changes under review
|
|||
|
||||
- The changed AL object names and types — especially `* Setup` singleton tables and Card pages, custom master tables, tableextensions that add master-data fields, document or journal lines that reference a master, document pages/codeunits exposing print/email/Post-and-Send actions, codeunits subscribing to `Navigate`, enumextensions to `"Report Selection Usage"`/`"Price Calculation Handler"`/`"Price Source Type"`, and report objects that render barcodes.
|
||||
- The changed fields, keys, triggers, and procedures, weighted toward `Primary Key`, `No.`, `No. Series`, `Blocked`, `Last Date Modified`, `OnInsert`, `OnModify`, `OnRename`, reference-field `OnValidate`, posting validation, and posting-cascade `TransferFields` calls.
|
||||
- Tokens extracted from the diff that relate to data modeling (`setup`, `master`, `Primary Key`, `Code[10]`, `Code[20]`, `AutoIncrement`, `SystemId`, `No.`, `No. Series`, `NoSeriesManagement`, `Codeunit "No. Series"`, `GetNextNo`, `IsManual`, `TestManual`, `Blocked`, `TestField`, `Last Date Modified`, `Today`, `WorkDate`, `InsertAllowed`, `DeleteAllowed`, `PageType = Card`, `OnOpenPage`, `GetRecordOnce`, `OnInsert`, `OnModify`, `OnRename`, `TableRelation`, `tableextension`, `enumextension`, `Media`, `MediaSet`, `Item`, `Count`, `TransferFields`, `Navigate`, `OnAfterFindRecords`, `OnBeforeShowRecords`, `Report Selections`, `Report Selection Usage`, `InsertRecord`, `Document Sending Profile`, `PrintForCust`, `PrintWithDialogForCust`, `PrintWithDialogForVend`, `SendEmailToCust`, `SendEmailToVendor`, `Report.RunModal`, `Report.Run`, `Price Calculation Handler`, `Price Calculation`, `OnFindSupportedSetup`, `Price Calculation Setup`, `Price Source Type`, `PriceSourceList`, `OnAfterAddSources`, `UpdateUnitPriceByField`, `Barcode Font Provider`, `Barcode Font Provider 2D`, `EncodeFont`, `ValidateInput`).
|
||||
- Tokens extracted from the diff that relate to data modeling (`setup`, `master`, `Primary Key`, `Code[10]`, `Code[20]`, `AutoIncrement`, `SystemId`, `No.`, `No. Series`, `NoSeriesManagement`, `Codeunit "No. Series"`, `GetNextNo`, `IsManual`, `TestManual`, `Blocked`, `TestField`, `Last Date Modified`, `Today`, `WorkDate`, `InsertAllowed`, `DeleteAllowed`, `PageType = Card`, `OnOpenPage`, `GetRecordOnce`, `OnInsert`, `OnModify`, `OnRename`, `InitRecord`, `Round`, `Precision`, `Direction`, `TableRelation`, `tableextension`, `enumextension`, `Media`, `MediaSet`, `Item`, `Count`, `TransferFields`, `Navigate`, `OnAfterFindRecords`, `OnBeforeShowRecords`, `Report Selections`, `Report Selection Usage`, `InsertRecord`, `Document Sending Profile`, `PrintForCust`, `PrintWithDialogForCust`, `PrintWithDialogForVend`, `SendEmailToCust`, `SendEmailToVendor`, `Report.RunModal`, `Report.Run`, `Price Calculation Handler`, `Price Calculation`, `OnFindSupportedSetup`, `Price Calculation Setup`, `Price Source Type`, `PriceSourceList`, `OnAfterAddSources`, `UpdateUnitPriceByField`, `Barcode Font Provider`, `Barcode Font Provider 2D`, `EncodeFont`, `ValidateInput`).
|
||||
|
||||
A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone. When the diff contains no data-modeling changes by any of the above signals, return `outcome: "not-applicable"` without evaluating files.
|
||||
|
||||
|
|
@ -52,6 +52,8 @@ The following targeted checks cover every current `data-modeling` article. Treat
|
|||
- A master table adds or changes `Last Date Modified`, `OnModify`, or `OnRename`, but the non-editable field is not assigned `Today()` in both triggers — `set-last-date-modified-in-onmodify-and-onrename`.
|
||||
- A `tableextension` appends a conditional `TableRelation` as if it overrides an earlier unconditional relation, or relation branches are otherwise designed without accounting for additive top-down evaluation — `table-relation-extensions-are-additive-and-top-down`.
|
||||
- A `Media` or `MediaSet` field is assigned directly between different table types or different field IDs instead of registering each shared item with `MediaSet.Insert` — `share-mediaset-items-with-insert-not-field-assignment`.
|
||||
- A custom document header assigns defaults outside an `InitRecord` boundary, calls `InitRecord` before assigning its number, or places UI-independent defaults only in a page trigger — `initialize-document-defaults-in-initrecord`.
|
||||
- Directed `Round` calls use `'<'` as mathematical floor or `'>'` as mathematical ceiling, especially where negative amounts are possible — `round-direction-symbols-use-magnitude`.
|
||||
- A codeunit dispatches a document by calling `Report.Run`/`Report.RunModal` with a hardcoded report ID and building its own email directly, with no accompanying `Report Selections` registration for that document — `custom-document-dispatch-must-not-bypass-report-selections`. A call that already goes through `Report Selections`' own Print/Email procedures is not this anti-pattern.
|
||||
- A document's own interactive Print/Email action routes through `Document Sending Profile` (`DocumentSendingProfile.Send`/`SendVendor`) instead of calling `Report Selections` (`PrintForCust`/`PrintWithDialogForCust`/`SendEmailToCust`/`PrintWithDialogForVend`/`SendEmailToVendor`) directly — `document-print-and-email-actions-call-report-selections-directly`. Do not flag `Document Sending Profile` usage that is genuinely part of a combined Post-and-Send action.
|
||||
- An `EventSubscriber` is added for `Navigate::OnAfterFindRecords` (registering a custom table in Find Entries) without a matching `Navigate::OnBeforeShowRecords` subscriber for the same table, or vice versa — `extend-find-entries-navigate-for-new-document-types`. Both subscribers must be added together for the same table.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue