mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-10-07 07:36:54 +01:00
Merge current main into development guidance
Resolve the README scope conflict, align guidance validation with the 333-article corpus, and reject failed reports that carry unreliable knowledge constraints. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 638b66d2-9f06-4f60-8781-808709e1485c
This commit is contained in:
commit
5793996e55
91 changed files with 2696 additions and 24 deletions
|
|
@ -28,6 +28,8 @@ sub-skills:
|
|||
- microsoft/skills/review/al-reporting-review.md
|
||||
- microsoft/skills/review/al-appsource-review.md
|
||||
- microsoft/skills/review/al-telemetry-review.md
|
||||
- microsoft/skills/review/al-scm-review.md
|
||||
- microsoft/skills/review/al-finance-review.md
|
||||
---
|
||||
|
||||
# AL code review
|
||||
|
|
|
|||
|
|
@ -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, and document or journal lines that reference a master.
|
||||
- The changed fields, keys, triggers, and procedures, weighted toward `Primary Key`, `No.`, `No. Series`, `Blocked`, `Last Date Modified`, `OnInsert`, `OnModify`, `OnRename`, reference-field `OnValidate`, and posting validation.
|
||||
- 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`).
|
||||
- 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`).
|
||||
|
||||
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`.
|
||||
|
||||
Once the candidate worklist is known, resolve layer-precedence conflicts per READ. Drop lower-precedence files whose normative guidance (`## Best Practice` or `## Anti Pattern`) directly contradicts a higher-precedence candidate, and record each dropped file in `suppressed` with `reason: "layer-precedence"`. Files that would have been candidates but are hidden because their layer is disabled in consumer configuration are recorded with `reason: "configuration"`. Files that never became candidates are NOT recorded in `suppressed`.
|
||||
|
||||
|
|
|
|||
146
microsoft/skills/review/al-finance-review.md
Normal file
146
microsoft/skills/review/al-finance-review.md
Normal file
|
|
@ -0,0 +1,146 @@
|
|||
---
|
||||
kind: action-skill
|
||||
id: al-finance-review
|
||||
version: 1
|
||||
title: AL Finance review
|
||||
description: Reviews financial journal posting, ledger corrections, applications, VAT handling, and posting-linked dimensions against BCQuality Finance guidance.
|
||||
inputs: [pr-diff, file-path, folder-path]
|
||||
outputs: [findings-report]
|
||||
bc-version: [all]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# AL Finance review
|
||||
|
||||
Reviews the `finance` knowledge domain. This is a leaf action skill composed by
|
||||
`al-code-review`; it invokes no other skills. Application-area metadata does
|
||||
not gate Finance coverage; resolved source records and operations do.
|
||||
|
||||
## Source
|
||||
|
||||
Apply the source-surface gate in Relevance before retrieving knowledge.
|
||||
If it passes, use READ's **Bounded retrieval for review skills** workflow with
|
||||
`-Domain finance`. Consume every catalog page across enabled layers, preserving
|
||||
each exact path and applicability metadata. Do not select a top-k catalog or
|
||||
deduplicate by basename. Open complete bodies only for exact Worklist paths,
|
||||
in stable chunks of at most eight, consuming every continuation. If the helper
|
||||
or prepared index is unavailable or invalid, use READ's explicit path-discovery
|
||||
and bounded native-read fallback; a retrieval error is not an empty corpus.
|
||||
|
||||
## Relevance
|
||||
|
||||
Apply READ's frontmatter matching rules using only known task dimensions.
|
||||
Use the target application version from `app.json` when available; do not invent
|
||||
a country or application area from a filename or UI `ApplicationArea` token.
|
||||
Retain conditional articles only when configured, cap resulting confidence at
|
||||
`medium`, and name every unknown dimension in the finding.
|
||||
|
||||
Inspect the supplied AL scope and its enclosing declarations. Admit only
|
||||
changed executable behavior involving at least one of these surfaces:
|
||||
|
||||
- General-journal construction or posting, journal-batch processing, or an
|
||||
event subscriber whose resolved publisher is in the financial posting path.
|
||||
- Writes or correction/application/reversal calls involving `G/L Entry`,
|
||||
`Cust. Ledger Entry`, `Vendor Ledger Entry`, `Detailed Cust. Ledg. Entry`,
|
||||
`Detailed Vendor Ledg. Entry`, `VAT Entry`, or financial-posting/G/L-register
|
||||
records.
|
||||
- Dimension transfer or dimension-set mutation connected by visible data flow
|
||||
to an existing general journal, financial posting document, or Finance-owned
|
||||
ledger record.
|
||||
|
||||
Exclude `Item Ledger Entry`, `Value Entry`, Capacity/Warehouse entries,
|
||||
`Item Application Entry`, and other inventory-posting records owned by SCM.
|
||||
Do not adopt their findings when the SCM skill is absent or disabled. For one
|
||||
inventory-originated posting bypass, equivalent findings have one SCM primary
|
||||
owner; distinct independent financial defects remain Finance. Classify the
|
||||
operation and actual record, not an inventory/finance word in a module name.
|
||||
|
||||
Return `not-applicable` when none is present. Imports, object names, comments,
|
||||
read-only ledger displays, generic `Amount`/`Date`/`Open` fields, and calls to
|
||||
`DimensionManagement` without posting-linked context do not establish relevance.
|
||||
For a diff, retain surrounding variable types, field provenance, event
|
||||
attributes, and reachable helpers; do not review isolated added lines without
|
||||
the context needed to classify their record or call.
|
||||
|
||||
## Worklist
|
||||
|
||||
Match the complete relevant catalog's keywords, titles, and descriptions to
|
||||
the admitted source surfaces. Add an exact catalog path only when its concern
|
||||
maps to the changed behavior; generic financial vocabulary is not enough.
|
||||
The following deterministic cues must select their named articles even if
|
||||
keyword ranking would otherwise omit them:
|
||||
|
||||
- Persistent Finance-owned ledger inserts reached from extension posting
|
||||
code — `post-ledger-entries-through-posting-codeunits`.
|
||||
- Persisted general-journal lines posted through a line-codeunit loop or custom
|
||||
aggregate check, with visible template/document/date balancing context —
|
||||
`preserve-journal-batch-document-balance`.
|
||||
- Imported net/tax/gross values mapped into `Gen. Journal Line.Amount`, with
|
||||
evidence of the VAT posting mode and posting-setup combination —
|
||||
`normal-vat-journal-amount-includes-vat`.
|
||||
- Persisted original Finance accounting-value changes, deletion of Finance rows, or
|
||||
fabricated reversal flags/links — `do-not-modify-or-delete-posted-ledger-entries`.
|
||||
- Customer/vendor settlement/reopening code writing `Open`, closure fields,
|
||||
detailed customer/vendor application amounts, or unapplication flags —
|
||||
`apply-ledger-entries-through-application-codeunits`.
|
||||
- A due-date change persisted on an existing customer/vendor ledger entry —
|
||||
`change-ledger-due-dates-through-entry-edit`.
|
||||
- A `Reversal Entry.ReverseTransaction` or `ReverseRegister` argument with
|
||||
visible ledger-entry, transaction, or register provenance —
|
||||
`reverse-transactions-by-transaction-number`.
|
||||
- A complete Finance posting-dimension transfer represented by shortcut/global
|
||||
fields or a `Dimension Set ID` assignment —
|
||||
`write-dimensions-as-dimension-set-entries`.
|
||||
- A dimension/value membership change on `Dimension Set Entry`, reached from
|
||||
a general-journal/financial-document/Finance-ledger set ID —
|
||||
`do-not-edit-shared-dimension-sets`.
|
||||
|
||||
These are retrieval cues, not findings. Use the selected articles' normative
|
||||
exceptions and ownership boundaries to classify standard workflows, temporary
|
||||
records, operational edits, and extension fields. Do not select a Finance
|
||||
ledger rule from `*Ledger Entry` or `Insert`/`Modify` alone. Finance does not
|
||||
own SCM records, generic custom-table/master dimension wiring, number-series
|
||||
API migration, or general AL validation, locking, transaction, and event-style
|
||||
advice. Do not add those concerns as Finance agent findings.
|
||||
|
||||
Resolve actual normative conflicts across layers per READ and record suppressed
|
||||
candidates per DO. Keep every remaining exact path in a stable worklist.
|
||||
Return `no-knowledge` if no applicable Finance knowledge survives filtering or
|
||||
configuration; return `completed` with no findings when applicable knowledge
|
||||
exists but no article matches the admitted changes.
|
||||
|
||||
## Action
|
||||
|
||||
Evaluate every worklist article in full against the changed behavior and its
|
||||
surrounding control flow. Establish record type, existing versus newly prepared
|
||||
state, temporariness, fields actually persisted, argument provenance, and the
|
||||
posting/edit API boundary before emitting a finding. Do not infer a financial
|
||||
defect from a method name, missing external setup, or unsupported speculation
|
||||
about callers.
|
||||
|
||||
Use the most specific article for the correction: application-state, due-date,
|
||||
and shared-dimension findings must not also become generic posted-row findings
|
||||
for the same change. Do not emit an equivalent Finance finding for the
|
||||
financial-row leg of one SCM-owned inventory posting bypass; evaluate a
|
||||
distinct financial defect only when its corrective action is independent.
|
||||
Emit `major` for a demonstrated financial-correctness
|
||||
violation and reserve `blocker` for directly evidenced destructive corruption
|
||||
under DO's severity rules. Applicability alone produces no finding.
|
||||
|
||||
Set `high` confidence only for established source evidence and known matching
|
||||
context. Domain-scoped agent findings follow DO's precision bar and remain
|
||||
capped at `minor`/`medium`; do not broaden this pass into other AL domains.
|
||||
Provide literal `suggested-code` for complete, local, unambiguous fixes.
|
||||
Otherwise give `suggested-code-omission-reason`, particularly when selecting
|
||||
the correct posting workflow requires business context.
|
||||
|
||||
Follow DO's acceptance gate and outcome rules. Report `partial` rather than
|
||||
silently dropping worklist items when a budget is reached, and `failed` for an
|
||||
unrecoverable retrieval or evaluation error.
|
||||
|
||||
## Output
|
||||
|
||||
Output conforms to the DO findings-report contract. Every finding this skill
|
||||
emits MUST set `findings[].domain` to `"Finance"`.
|
||||
|
|
@ -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 `interface` objects, codeunits and enums declared with the `implements` keyword, and consumers that declare or assign an `Interface` variable.
|
||||
- The changed procedures and triggers, weighted toward factory or dispatch routines that resolve a variant to behaviour, setter-injection procedures that take an `Interface` parameter, and `case`-over-enum blocks that select between strategies.
|
||||
- Tokens extracted from the diff that relate to interfaces and enum-backed implementation (`interface`, `extends`, `implements`, `Implementation`, `DefaultImplementation`, `UnknownValueImplementation`, `enum`, `Extensible`, `Interface`, `case`, and the `case <enum> of` anti-pattern signal — a `case` over an enum value whose branches choose between variant computations).
|
||||
- Tokens extracted from the diff that relate to interfaces and enum-backed implementation (`interface`, `extends`, `implements`, `Implementation`, `DefaultImplementation`, `UnknownValueImplementation`, `enum`, `Extensible`, `Interface`, `Variant`, `is`, `as`, `case`, and the `case <enum> of` anti-pattern signal — a `case` over an enum value whose branches choose between variant computations).
|
||||
|
||||
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.
|
||||
|
||||
|
|
@ -54,6 +54,7 @@ The following targeted checks map diff signals to specific `interfaces` articles
|
|||
- `DefaultImplementation` used as the only fallback where a persisted ordinal may no longer match any declared enum value, or a persisted enum lacks `UnknownValueImplementation` on BC18 or later — `handle-unknown-enum-ordinals-with-unknownvalueimplementation`.
|
||||
- A method added directly to an interface that exists in the baseline, instead of adding a BC25+ interface that `extends` it or a versioned sibling for older targets — `extend-published-interfaces-dont-edit-them`.
|
||||
- A declared enum value with no `Implementation` and no enum-level `DefaultImplementation` — `set-defaultimplementation-on-enum`.
|
||||
- An `Interface` or `Variant` is cast with `as` to an optional extended interface without first establishing support with `is` — `guard-interface-casts-with-is`.
|
||||
|
||||
For `set-defaultimplementation-on-enum`, inspect the complete containing enum before emitting. An enum-level `DefaultImplementation = <Interface> = <Codeunit>;` conclusively covers every declared value that omits its own `Implementation`; do not flag such a value and do not replace the intentional fallback with a per-value mapping.
|
||||
|
||||
|
|
|
|||
121
microsoft/skills/review/al-scm-review.md
Normal file
121
microsoft/skills/review/al-scm-review.md
Normal file
|
|
@ -0,0 +1,121 @@
|
|||
---
|
||||
kind: action-skill
|
||||
id: al-scm-review
|
||||
version: 1
|
||||
title: AL Supply Chain Management review
|
||||
description: Reviews SCM inventory costing, item application, reservations, order tracking, item tracking, warehouse, transfer, and planning workflows in AL.
|
||||
inputs: [pr-diff, file-path, folder-path]
|
||||
outputs: [findings-report]
|
||||
bc-version: [all]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# AL Supply Chain Management review
|
||||
|
||||
Reviews AL source against the `scm` knowledge domain. This leaf invokes no
|
||||
sub-skills and is composed by `al-code-review`. For a folder, inspect every
|
||||
relevant AL file; a folder supplies no historical baseline.
|
||||
|
||||
## Source
|
||||
|
||||
Apply Relevance's source gate before retrieval. For a relevant scope, use READ's
|
||||
**Bounded retrieval for review skills** with `-Domain scm` and
|
||||
`-Technologies @('al')`. Consume every catalog page across enabled layers,
|
||||
preserving exact paths and applicability. Select from metadata, then read only
|
||||
worklisted complete articles. Entry owns index preparation; the leaf does not
|
||||
rebuild. Unavailable/invalid helpers or indexes use READ's bounded native-read
|
||||
fallback, never a success-shaped empty result.
|
||||
|
||||
## Relevance
|
||||
|
||||
Resolve changed record/codeunit types, source tables, publishers and calls.
|
||||
Gate on code that mutates or posts inventory, application, reservation,
|
||||
tracking, warehouse, transfer or planning state, or makes a supply/demand
|
||||
availability decision. Stock displays and other read-only queries without
|
||||
that decision do not pass the gate. Names, comments, captions, an `Item`
|
||||
reference or a broad `ApplicationArea` alone are not signals.
|
||||
If no SCM surface remains, return `not-applicable` with zero coverage
|
||||
and no article-body retrieval; in mixed diffs, retain only relevant procedures
|
||||
and their visible supporting context.
|
||||
|
||||
SCM owns `"Item Ledger Entry"`, `"Value Entry"`, `"Capacity Ledger Entry"`,
|
||||
`"Warehouse Entry"` and inventory posting/application records. Pure `"G/L Entry"`,
|
||||
`"Cust. Ledger Entry"`, `"Vendor Ledger Entry"`, `"Detailed Cust. Ledg. Entry"`,
|
||||
`"Detailed Vendor Ledg. Entry"`, `"VAT Entry"` and financial-only posting
|
||||
mutations belong to Finance. They remain outside SCM even if Finance is absent
|
||||
or disabled; do not reclaim them as SCM agent findings. Ownership is not a
|
||||
claim that every owned surface already has a dedicated article.
|
||||
|
||||
Apply READ's frontmatter filters using the target BC major version from
|
||||
application dependency/host context (not the extension version), AL, known
|
||||
localization and actual task/object application areas. Omitted context stays
|
||||
unknown, not `[all]`; unknown areas alone do not exclude codeunits/subscribers.
|
||||
Retain conditional articles only when configured, cap their findings at
|
||||
`medium`, and name every unknown dimension.
|
||||
|
||||
## Worklist
|
||||
|
||||
Extract resolved object/type names, quoted fields, methods, enum members and
|
||||
publishers. Normalize these and catalog keywords by lowercasing invariantly,
|
||||
replacing punctuation/whitespace runs with one hyphen and trimming hyphens:
|
||||
`"Item Ledger Entry"` becomes `item-ledger-entry`; `RunWithCheck` becomes
|
||||
`runwithcheck`. Match whole tokens/phrases, not identifier substrings.
|
||||
|
||||
Select matching keywords or catalog topics only for the same source surface
|
||||
**and operation**. The following cues resolve slugs to actual enabled catalog
|
||||
paths; they select articles, not findings. Facts and exceptions stay in articles.
|
||||
|
||||
| Changed source surface and operation | Article slug |
|
||||
| --- | --- |
|
||||
| `"Item Ledger Entry"`/`"Value Entry"` transaction writes, or a standalone item-journal quantity/value posting entry point | `post-item-ledger-changes-through-item-journals` |
|
||||
| Revaluation `"Item Journal Line"` with `"Inventory Value Per"` or `"Partial Revaluation"`, and its line/batch posting calls | `post-revaluation-through-the-item-journal-batch` |
|
||||
| `"Item Application Entry"` relationship/quantity mutation, or `UnApply`, `ReApply`, `RedoApplications`, `CostAdjust` in an application-correction flow | `change-item-applications-through-posting-routines` |
|
||||
| Binding-reservation cancellation: `"Reservation Entry"` status, delete/quantity/source edits, `CancelReservation`, or source reservation-lifecycle calls | `cancel-reservations-through-reservation-management` |
|
||||
| Tracking source conversion/partial movement: `"Sales Line-Reserve"`, `TransferSaleLineToSalesLine`, `TransferReservEntry`, `CopyItemTracking`, or `"Reservation Entry"`/`"Tracking Specification"` source/quantity writes | `transfer-item-tracking-through-source-reservation-codeunits` |
|
||||
| Registered warehouse quantity/physical-adjustment synchronization, `"Directed Put-away and Pick"`, `"Adjustment Bin Code"`, `"Warehouse Adjustment"`, or `"Calculate Whse. Adjustment"` and the resulting item-journal posting | `reconcile-warehouse-adjustments-with-the-item-ledger` |
|
||||
| `"Transfer Header"`/`"Transfer Line"` shipment/receipt completion, transfer posting publishers, in-transit/document-link changes, or item-journal posting presented as transfer-order completion | `post-transfers-through-shipment-and-receipt-codeunits` |
|
||||
| `Inventory`, `CalcQtyAvailableToPromise`, or stock sums used in a dated supply/demand promise, including changed location/variant/date filters and source-demand context | `use-date-aware-availability-for-promising` |
|
||||
| `"Requisition Line"` action-message execution, accepted planning suggestions, `"Req. Wksh.-Make Order"`, `CarryOutBatchAction`, or linked supply creation/change plus requisition-line deletion | `carry-out-requisition-actions-through-the-standard-workflow` |
|
||||
|
||||
Route clean supported calls through the same cues, not just suspicious writes.
|
||||
Resolve actual normative conflicts per READ, preserving additive layers and
|
||||
recording `layer-precedence`/`configuration` suppressions, not noncandidates.
|
||||
Retrieve exact paths in ordinal chunks of at most eight, consume every
|
||||
continuation, and never impose a top-eight cutoff. Samples use exact READ links.
|
||||
|
||||
## Action
|
||||
|
||||
Evaluate every opened article's normative facts, scope and exclusions against
|
||||
visible persistence, caller contract, document state and operation. Emit only
|
||||
concrete violations with business consequences and supported remediation; a
|
||||
declaration, valid alternative or unseen caller is not evidence of a defect.
|
||||
|
||||
- Use `major` for material SCM defects, `minor` for narrower best-practice
|
||||
conflicts, and `blocker` only for an article-established platform guarantee.
|
||||
Applicability alone produces no finding. High confidence requires unambiguous
|
||||
evidence and known applicability; inference/conditional applicability caps it
|
||||
at `medium`.
|
||||
- Apply DO's single-owner deduplication. Equivalent findings for the same
|
||||
inventory-originated posting bypass and correction have one SCM primary
|
||||
owner, even when financial records are downstream. Prefer the most specific
|
||||
SCM article and retain other applicable references as supporting evidence.
|
||||
Distinct independent financial defects remain Finance; do not duplicate them.
|
||||
- Agent findings stay strictly SCM-scoped under DO's precision bar, with
|
||||
`references: []`, an `agent:` id and `minor`/`medium` ceilings. Generic AL and
|
||||
other domains' concerns remain outside this leaf.
|
||||
- Supply literal `suggested-code` only for a complete, local, unambiguous fix,
|
||||
not a sample call that omits workflow setup/source identity. Explain omitted
|
||||
mechanical-looking fixes with `suggested-code-omission-reason`.
|
||||
|
||||
Outcome selection follows DO, including accurate coverage and reasons for
|
||||
`partial`/`failed`. No surviving applicable corpus is `no-knowledge`; an existing
|
||||
corpus with no matching operation is `completed` with an empty worklist.
|
||||
|
||||
## Output
|
||||
|
||||
Output conforms to the DO findings-report contract and shared schema. Every
|
||||
finding MUST set `domain` to `"Supply Chain Management"`. Knowledge-backed ids
|
||||
equal the primary opened article's exact catalog path. The coordinator, not
|
||||
this leaf, sets `from-sub-skill`.
|
||||
|
|
@ -3,7 +3,7 @@ kind: action-skill
|
|||
id: al-style-review
|
||||
version: 1
|
||||
title: AL style review
|
||||
description: Reviews AL source changes against naming, labelling, and code-convention guidance from BCQuality.
|
||||
description: Reviews AL source changes against naming, labelling, localization, and code-convention guidance from BCQuality.
|
||||
inputs: [pr-diff, file-path, folder-path]
|
||||
outputs: [findings-report]
|
||||
bc-version: [all]
|
||||
|
|
@ -16,7 +16,7 @@ application-area: [all]
|
|||
|
||||
Reviews AL source changes against the `style` 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`.
|
||||
|
||||
Style findings cover AL conventions that require contextual judgment — API page naming, temporary-variable prefixes, label semantics, named invocations, `FieldCaption`/`TableCaption` in user messages, error-parameter handling, and file naming. Mechanical compiler and analyzer rules are intentionally outside this skill; run the consuming app's configured analyzers separately.
|
||||
Style findings cover AL conventions that require contextual judgment — API page naming, temporary-variable prefixes, label semantics, date-formula localization, named invocations, `FieldCaption`/`TableCaption` in user messages, error-parameter handling, and file naming. Mechanical compiler and analyzer rules are intentionally outside this skill; run the consuming app's configured analyzers separately.
|
||||
|
||||
An orchestrator invokes this skill with a `pr-diff`, `file-path`, or `folder-path`. The skill produces a single JSON document conforming to the DO output contract.
|
||||
|
||||
|
|
@ -40,8 +40,8 @@ Discard files that are not applicable. Retain conditionally applicable files onl
|
|||
Narrow the relevant files to the subset that applies to the changes under review. For each relevant file, compute overlap against:
|
||||
|
||||
- Changed AL objects — especially API pages (`PageType = API`), tables and pages declaring Labels/TextConsts, codeunits issuing `Error`/`Message`/`Confirm`, and any file whose name violates the `<ObjectName>.<ObjectType>.al` convention.
|
||||
- Changed declarations, weighted toward `: Label '...'`, `: TextConst '...'`, temporary record variables, error-handling call sites, and API declarations.
|
||||
- Tokens extracted from the diff (`Label`, `TextConst`, `Locked`, `Comment`, `MaxLength`, `temporary`, `APIPublisher`, `APIGroup`, `APIVersion`, `EntityName`, `EntitySetName`, `DelayedInsert`, `FieldCaption`, `TableCaption`, `FieldName`, `TableName`, `Page.RunModal`, `Report.Run`, `StrSubstNo`).
|
||||
- Changed declarations, weighted toward `: Label '...'`, `: TextConst '...'`, temporary record variables, `DateFormula` declarations and their `Evaluate` call sites, error-handling call sites, and API declarations.
|
||||
- Tokens extracted from the diff (`Label`, `TextConst`, `Locked`, `Comment`, `MaxLength`, `temporary`, `DateFormula`, `Evaluate`, `CalcDate`, `APIPublisher`, `APIGroup`, `APIVersion`, `EntityName`, `EntitySetName`, `DelayedInsert`, `FieldCaption`, `TableCaption`, `FieldName`, `TableName`, `Page.RunModal`, `Report.Run`, `StrSubstNo`).
|
||||
|
||||
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 or declaration. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone.
|
||||
|
||||
|
|
@ -50,6 +50,8 @@ Do not worklist `temporary-variable-temp-prefix.md` for an event publisher param
|
|||
Apply these high-signal mappings before fuzzy topic ranking:
|
||||
|
||||
- A `Label` or `TextConst` contains multiple or ambiguous placeholders but has no `Comment`, or its Comment does not explain every placeholder — `label-comment-explains-placeholders.md`. A single placeholder whose meaning is explicit in the text, such as `Customer %1`, is allowed without a Comment and must not be flagged.
|
||||
- A normal two-argument `Evaluate` has a resolved `DateFormula` destination and a hard-coded non-angle-bracket date-formula literal, directly or through a visible constant — `dateformula-evaluate-needs-language-independent-literals.md`. Do not use this cue for dynamic/localized external input, already invariant `<...>` input, or direct `CalcDate(Text, ...)` calls.
|
||||
|
||||
Once the candidate worklist is known, resolve layer-precedence conflicts per READ and record suppressions.
|
||||
|
||||
When the post-conflict worklist is empty because no applicable style knowledge exists, or because configuration suppressed every candidate, emit `outcome: "no-knowledge"`. When the worklist is empty because no applicable style knowledge matched the changes, emit `outcome: "completed"` with an empty `findings` array.
|
||||
|
|
|
|||
|
|
@ -41,10 +41,15 @@ Narrow the relevant files to the subset that applies to the changes under review
|
|||
|
||||
- **UI-file filter.** UI review applies to files declaring `page`, `pageextension`, or `pagecustomization`, and to JavaScript/CSS/HTML that implements a control add-in's rendering or Business Central communication. When the diff contains no such files, return `outcome: "not-applicable"` without evaluating knowledge files.
|
||||
- For each relevant knowledge file, compute overlap against changed page declarations and control add-in files, weighted toward `Caption`, `ToolTip`, `AboutTitle`, `AboutText`, `OptionCaption`, `ShowCaption`, `InstructionalText`, `GridLayout`, `Style`, `StyleExpr`, promoted action definitions, field importance, page background tasks, DOM creation, ARIA attributes, keyboard/focus handlers, packaged-resource AJAX, and calls from JavaScript into AL.
|
||||
- Tokens extracted from the diff (`Caption`, `ToolTip`, `AboutTitle`, `AboutText`, `PageType`, `ShowCaption`, `InstructionalText`, `grid`, `fixed`, `GridLayout`, `Style`, `StyleExpr`, `Importance`, `Promoted`, `Additional`, `area(Promoted)`, `actionref`, `PromotedCategory`, `PromotedOnly`, `PromotedIsBig`, `ShowAs`, `SplitButton`, `EnqueueBackgroundTask`, `OnAfterGetCurrRecord`, `OnAfterGetRecord`, `OnPageBackgroundTaskCompleted`, `OnPageBackgroundTaskError`, `RunPageBackgroundTask`, `Favorable`, `Unfavorable`, `Ambiguous`, `cuegroup`, `controladdin`, `control-add-in`, `usercontrol`, `aria-`, `tabindex`, `keydown`, `focus`, `innerHTML`, `createElement`, `packaged-resource`, `ajax`, `$.get`, `$.ajax`, `XMLHttpRequest`, `xhrFields`, `withCredentials`, `withcredentials`, `InvokeExtensibilityMethod`, `invokeextensibilitymethod`, `skipIfBusy`, `successCallback`, `success-callback`, `errorCallback`, `setInterval`, `JSON.stringify`, `payload`, `throttling`, `reduced-functionality`, `ClientServicesMaxUploadSize`, `&`, `Specifies`, `Message(`, `Confirm(`, `Error(` in a page context, `Disabled`, `Invalid`, `Whitelist`, `Blacklist`, trailing punctuation patterns on captions).
|
||||
- Tokens extracted from the diff (`Caption`, `ToolTip`, `AboutTitle`, `AboutText`, `PageType`, `ShowCaption`, `InstructionalText`, `grid`, `fixed`, `GridLayout`, `Style`, `StyleExpr`, `Importance`, `Promoted`, `Additional`, `area(Promoted)`, `actionref`, `PromotedCategory`, `PromotedOnly`, `PromotedIsBig`, `ShowAs`, `SplitButton`, `fieldgroups`, `DropDown`, `UpdatePropagation`, `EnqueueBackgroundTask`, `OnAfterGetCurrRecord`, `OnAfterGetRecord`, `OnPageBackgroundTaskCompleted`, `OnPageBackgroundTaskError`, `RunPageBackgroundTask`, `Favorable`, `Unfavorable`, `Ambiguous`, `cuegroup`, `controladdin`, `control-add-in`, `usercontrol`, `aria-`, `tabindex`, `keydown`, `focus`, `innerHTML`, `createElement`, `packaged-resource`, `ajax`, `$.get`, `$.ajax`, `XMLHttpRequest`, `xhrFields`, `withCredentials`, `withcredentials`, `InvokeExtensibilityMethod`, `invokeextensibilitymethod`, `skipIfBusy`, `successCallback`, `success-callback`, `errorCallback`, `setInterval`, `JSON.stringify`, `payload`, `throttling`, `reduced-functionality`, `ClientServicesMaxUploadSize`, `&`, `Specifies`, `Message(`, `Confirm(`, `Error(` in a page context, `Disabled`, `Invalid`, `Whitelist`, `Blacklist`, trailing punctuation patterns on captions).
|
||||
|
||||
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 page element. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone.
|
||||
|
||||
Apply these high-signal mappings before fuzzy topic ranking:
|
||||
|
||||
- A tableextension adds a field to `DropDown` while the corresponding lookup-page control remains `Visible = false` — `dropdown-fieldgroup-respects-lookup-page-visibility`.
|
||||
- An editable page part affects a total, FlowField, or FactBox on the parent but does not set `UpdatePropagation = Both` — `updatepropagation-both-refreshes-main-page`.
|
||||
|
||||
Once the candidate worklist is known, resolve layer-precedence conflicts per READ and record suppressions.
|
||||
|
||||
When the post-conflict worklist is empty because no applicable UI knowledge exists, or because configuration suppressed every candidate, emit `outcome: "no-knowledge"`. When the worklist is empty because no applicable UI knowledge matched the page changes, emit `outcome: "completed"` with an empty `findings` array.
|
||||
|
|
|
|||
|
|
@ -39,10 +39,12 @@ Narrow the relevant files to the subset that applies to the changes under review
|
|||
|
||||
- The changed AL object names and types — especially codeunits with `Subtype = Upgrade` or `Subtype = Install`, tables and tableextensions adding or changing fields, enums and enumextensions, and objects under `Hybrid*`/`Migration`/`Upgrade` namespaces.
|
||||
- The changed triggers and procedures, weighted toward `OnCheckPreconditionsPerCompany`/`PerDatabase`, `OnUpgradePerCompany`/`PerDatabase`, `OnValidateUpgradePerCompany`/`PerDatabase`, `OnInstallAppPerCompany`/`PerDatabase`, the `OnGetPerCompanyUpgradeTags`/`OnGetPerDatabaseUpgradeTags` subscribers, and helper procedures transitively reachable from those entry points.
|
||||
- Tokens extracted from the diff that relate to upgrade concerns (`Subtype = Upgrade`, `Subtype = Install`, `Upgrade Tag`, `HasUpgradeTag`, `SetUpgradeTag`, `OnCheckPreconditions`, `OnUpgrade`, `OnValidateUpgrade`, `OnInstallApp`, `DataTransfer`, `CopyFields`, `Insert`, `Modify`, `Delete`, `Rename`, `InitValue`, `ObsoleteState`, `ObsoleteReason`, `ObsoleteTag`, `DataVersion`, `ExecutionContext`, `PrimaryKey`, `key(`, `field(`, `value(`, `enum`, `enumextension`, `HybridSL`, `HybridGP`, `HybridBC`, `HybridBaseDeployment`).
|
||||
- Tokens extracted from the diff that relate to upgrade concerns (`Subtype = Upgrade`, `Subtype = Install`, `Upgrade Tag`, `HasUpgradeTag`, `SetUpgradeTag`, `OnCheckPreconditions`, `OnUpgrade`, `OnValidateUpgrade`, `OnInstallApp`, `DataTransfer`, `CopyFields`, `Insert`, `Modify`, `Delete`, `Rename`, `InitValue`, `ObsoleteState`, `ObsoleteReason`, `ObsoleteTag`, `ModuleInfo`, `AppVersion`, `DataVersion`, `NavApp.GetCurrentModuleInfo`, `ExecutionContext`, `PrimaryKey`, `key(`, `field(`, `value(`, `enum`, `enumextension`, `HybridSL`, `HybridGP`, `HybridBC`, `HybridBaseDeployment`).
|
||||
- For each `OnCheckPreconditions...` and `OnValidateUpgrade...` trigger, build the best available call graph from surrounding unchanged source as well as changed hunks, tracing resolved calls through reachable local or internal helpers. Worklist the check-only rule when a database write occurs either directly in the trigger or in any helper procedure reachable from it. Writes include `Insert`, `Modify`, `ModifyAll`, `Delete`, `DeleteAll`, `Rename`, and `DataTransfer`. Also perform the reverse check when a PR changes a writing helper body: worklist the rule when that helper is invoked directly or transitively by an unchanged check or validation trigger.
|
||||
- Treat a direct write or a fully resolved call chain as high-confidence evidence. When cross-object dispatch, unavailable declarations, or an incomplete call graph prevents proving the complete chain, cap confidence at `medium`, name the unresolved edge in the finding, and do not claim a violation without a resolved path from a check or validation trigger to a write.
|
||||
- Worklist the install-versus-upgrade rule when migration helpers are reachable only from an install codeunit.
|
||||
- Worklist `install-and-upgrade-codeunits-have-no-order.md` when a change adds multiple install or upgrade codeunits whose same-phase triggers share state or depend on one another.
|
||||
- Worklist `appversion-meaning-depends-on-execution-context.md` when install or upgrade code branches on `ModuleInfo.AppVersion()` or confuses it with `DataVersion()`.
|
||||
|
||||
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 upgrade-related changes by any of the above signals, return `outcome: "not-applicable"` without evaluating files.
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue