mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-08-06 09:26:52 +01:00
Promote validated community knowledge (#105)
Move eight net-new rules into the Microsoft layer, remove six overlapping articles, and update review skill discovery and references. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0b130227-d418-4bc0-9e7d-ec6a37adf039 Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
This commit is contained in:
Jesper Schulz-Wedde
GitHub
parent
be1b92b624
commit
3d29c172a9
35 changed files with 136 additions and 478 deletions
|
|
@ -46,7 +46,7 @@ A file enters the candidate worklist when its `keywords` intersect the extracted
|
|||
The following targeted checks cover every current `appsource` article across the Microsoft and community layers. Treat each as a candidate-selection cue: when the signal appears in changed code, add the named article to the worklist and evaluate it in Action.
|
||||
|
||||
- A new or renamed object lacks the reserved prefix/suffix, or a tableextension/pageextension/reportextension adds an unaffixed field, key, control, or action to a base object despite `mandatoryAffixes`/`mandatoryPrefix` and AS0011 — `object-affixes-prevent-collisions`.
|
||||
- For BC v24 or later, `app.json` adds or changes the `help` URL to a path deeper than two levels, or a changed Copilot/context-sensitive help arrangement would ground the app under an overly broad truncated parent — `keep-copilot-help-url-to-two-path-levels`.
|
||||
- For BC v27 or later, `app.json` adds or changes the `help` URL to a path deeper than two levels, or a changed Copilot/context-sensitive help arrangement would ground the app under an overly broad truncated parent — `keep-copilot-help-url-to-two-path-levels`.
|
||||
|
||||
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,15 +146,15 @@ Output conforms to the DO output contract, extended with `sub-results` and `skip
|
|||
"from-sub-skill": "al-performance-review"
|
||||
},
|
||||
{
|
||||
"id": "community/knowledge/performance/setloadfields-unlisted-field-triggers-jit-load.md",
|
||||
"id": "microsoft/knowledge/performance/use-setloadfields-for-partial-records.md",
|
||||
"severity": "minor",
|
||||
"message": "The loop reads an unlisted field after SetLoadFields, triggering a hidden JIT load for each record passed by value.",
|
||||
"message": "The loop reads only a small subset of fields from a wide table without SetLoadFields, transferring every column for each row.",
|
||||
"location": {
|
||||
"file": "src/Sales/PostingRoutines.Codeunit.al",
|
||||
"line": 152
|
||||
},
|
||||
"references": [
|
||||
{ "path": "community/knowledge/performance/setloadfields-unlisted-field-triggers-jit-load.md" }
|
||||
{ "path": "microsoft/knowledge/performance/use-setloadfields-for-partial-records.md" }
|
||||
],
|
||||
"confidence": "high",
|
||||
"from-sub-skill": "al-performance-review"
|
||||
|
|
@ -175,7 +175,7 @@ Output conforms to the DO output contract, extended with `sub-results` and `skip
|
|||
"from-sub-skill": "al-security-review"
|
||||
},
|
||||
{
|
||||
"id": "community/knowledge/security/secrets-isolated-storage.md",
|
||||
"id": "microsoft/knowledge/security/secrets-isolated-storage.md",
|
||||
"severity": "minor",
|
||||
"message": "A setup table stores an API key in an ordinary Text field, exposing it through table reads and exports. Persist it in IsolatedStorage instead.",
|
||||
"location": {
|
||||
|
|
@ -183,7 +183,7 @@ Output conforms to the DO output contract, extended with `sub-results` and `skip
|
|||
"line": 12
|
||||
},
|
||||
"references": [
|
||||
{ "path": "community/knowledge/security/secrets-isolated-storage.md" }
|
||||
{ "path": "microsoft/knowledge/security/secrets-isolated-storage.md" }
|
||||
],
|
||||
"confidence": "medium",
|
||||
"from-sub-skill": "al-security-review"
|
||||
|
|
@ -227,15 +227,15 @@ Output conforms to the DO output contract, extended with `sub-results` and `skip
|
|||
"confidence": "high"
|
||||
},
|
||||
{
|
||||
"id": "community/knowledge/performance/setloadfields-unlisted-field-triggers-jit-load.md",
|
||||
"id": "microsoft/knowledge/performance/use-setloadfields-for-partial-records.md",
|
||||
"severity": "minor",
|
||||
"message": "The loop reads an unlisted field after SetLoadFields, triggering a hidden JIT load for each record passed by value.",
|
||||
"message": "The loop reads only a small subset of fields from a wide table without SetLoadFields, transferring every column for each row.",
|
||||
"location": {
|
||||
"file": "src/Sales/PostingRoutines.Codeunit.al",
|
||||
"line": 152
|
||||
},
|
||||
"references": [
|
||||
{ "path": "community/knowledge/performance/setloadfields-unlisted-field-triggers-jit-load.md" }
|
||||
{ "path": "microsoft/knowledge/performance/use-setloadfields-for-partial-records.md" }
|
||||
],
|
||||
"confidence": "high"
|
||||
}
|
||||
|
|
@ -265,7 +265,7 @@ Output conforms to the DO output contract, extended with `sub-results` and `skip
|
|||
"confidence": "high"
|
||||
},
|
||||
{
|
||||
"id": "community/knowledge/security/secrets-isolated-storage.md",
|
||||
"id": "microsoft/knowledge/security/secrets-isolated-storage.md",
|
||||
"severity": "minor",
|
||||
"message": "A setup table stores an API key in an ordinary Text field, exposing it through table reads and exports. Persist it in IsolatedStorage instead.",
|
||||
"location": {
|
||||
|
|
@ -273,7 +273,7 @@ Output conforms to the DO output contract, extended with `sub-results` and `skip
|
|||
"line": 12
|
||||
},
|
||||
"references": [
|
||||
{ "path": "community/knowledge/security/secrets-isolated-storage.md" }
|
||||
{ "path": "microsoft/knowledge/security/secrets-isolated-storage.md" }
|
||||
],
|
||||
"confidence": "medium"
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 codeunits that post or validate, tables and table extensions with `OnValidate` triggers, and any procedure that raises errors or orchestrates a batch over records.
|
||||
- The changed procedures and triggers, weighted toward `OnValidate`/`OnInsert`/`OnModify` triggers, posting and validation routines, and procedures attributed with `[ErrorBehavior(...)]` or `[TryFunction]`.
|
||||
- Tokens extracted from the diff that relate to error surfacing and diagnostics (`Error`, `ErrorInfo`, `Title`, `Message`, `DetailedMessage`, `AddAction`, `AddNavigationAction`, `RecordId`, `PageNo`, `ErrorBehavior`, `Collect`, `HasCollectedErrors`, `GetCollectedErrors`, `ClearCollectedErrors`, `ErrorType`, `Internal`, `Client`, `TryFunction`, `GetLastErrorText`, Boolean assignment).
|
||||
- Tokens extracted from the diff that relate to error surfacing and diagnostics (`Error`, `ErrorInfo`, `FieldError`, `TestField`, `Title`, `Message`, `DetailedMessage`, `AddAction`, `AddNavigationAction`, `RecordId`, `PageNo`, `ErrorBehavior`, `Collect`, `HasCollectedErrors`, `GetCollectedErrors`, `ClearCollectedErrors`, `ErrorType`, `Internal`, `Client`, `TryFunction`, `GetLastErrorText`, Boolean assignment).
|
||||
- Resolve changed standalone call targets; when the target declaration has `[TryFunction]`, worklist the ignored-return rule even if the declaration itself is unchanged. Only assignment and conditional use activate try semantics.
|
||||
|
||||
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.
|
||||
|
|
|
|||
|
|
@ -111,15 +111,15 @@ Output conforms to the DO output contract. A populated example:
|
|||
"confidence": "high"
|
||||
},
|
||||
{
|
||||
"id": "community/knowledge/performance/setloadfields-unlisted-field-triggers-jit-load.md",
|
||||
"id": "microsoft/knowledge/performance/use-setloadfields-for-partial-records.md",
|
||||
"severity": "minor",
|
||||
"message": "The loop reads an unlisted field after SetLoadFields, triggering a hidden JIT load for each record passed by value.",
|
||||
"message": "The loop reads only a small subset of fields from a wide table without SetLoadFields, transferring every column for each row.",
|
||||
"location": {
|
||||
"file": "src/Sales/PostingRoutines.Codeunit.al",
|
||||
"line": 152
|
||||
},
|
||||
"references": [
|
||||
{ "path": "community/knowledge/performance/setloadfields-unlisted-field-triggers-jit-load.md" }
|
||||
{ "path": "microsoft/knowledge/performance/use-setloadfields-for-partial-records.md" }
|
||||
],
|
||||
"confidence": "high"
|
||||
}
|
||||
|
|
|
|||
|
|
@ -103,7 +103,7 @@ Output conforms to the DO output contract. A populated example:
|
|||
"confidence": "high"
|
||||
},
|
||||
{
|
||||
"id": "community/knowledge/security/secrets-isolated-storage.md",
|
||||
"id": "microsoft/knowledge/security/secrets-isolated-storage.md",
|
||||
"severity": "minor",
|
||||
"message": "A setup table stores an API key in an ordinary Text field, exposing it through table reads and exports. Persist it in IsolatedStorage instead.",
|
||||
"location": {
|
||||
|
|
@ -111,7 +111,7 @@ Output conforms to the DO output contract. A populated example:
|
|||
"line": 12
|
||||
},
|
||||
"references": [
|
||||
{ "path": "community/knowledge/security/secrets-isolated-storage.md" }
|
||||
{ "path": "microsoft/knowledge/security/secrets-isolated-storage.md" }
|
||||
],
|
||||
"confidence": "medium"
|
||||
}
|
||||
|
|
|
|||
|
|
@ -43,10 +43,6 @@ Narrow the relevant files to the subset that applies to the changes under review
|
|||
|
||||
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 telemetry-related changes by any of the above signals, return `outcome: "not-applicable"` without evaluating files.
|
||||
|
||||
The following targeted check covers every current `telemetry` article across the Microsoft and community layers. Treat it as a candidate-selection cue: when the signal appears in changed code, add the named article to the worklist and evaluate it in Action.
|
||||
|
||||
- `Session.LogMessage` or `Session.LogError` uses `TelemetryScope::All` for publisher-only diagnostics, a telemetry wrapper defaults its scope to `All`, or a `FeatureTelemetry`/custom logging change routes signals to customer environment telemetry without a customer-actionable reason — `default-telemetryscope-to-extensionpublisher`.
|
||||
|
||||
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`.
|
||||
|
||||
When the post-conflict worklist is empty because no applicable telemetry knowledge exists, or because configuration suppressed every candidate, emit `outcome: "no-knowledge"`. When the worklist is empty because no applicable telemetry knowledge matched the changes, emit `outcome: "completed"` with an empty `findings` array.
|
||||
|
|
@ -81,37 +77,7 @@ Outcome selection:
|
|||
|
||||
## Output
|
||||
|
||||
Output conforms to the DO output contract. A populated example:
|
||||
|
||||
```json
|
||||
{
|
||||
"skill": { "id": "al-telemetry-review", "version": 1 },
|
||||
"outcome": "completed",
|
||||
"summary": {
|
||||
"counts": { "blocker": 0, "major": 1, "minor": 0, "info": 0 },
|
||||
"coverage": { "worklist-size": 1, "items-evaluated": 1 }
|
||||
},
|
||||
"findings": [
|
||||
{
|
||||
"id": "community/knowledge/telemetry/default-telemetryscope-to-extensionpublisher.md",
|
||||
"severity": "major",
|
||||
"message": "This publisher-only diagnostic uses TelemetryScope::All, which also sends it to each customer's environment telemetry and adds avoidable ingestion cost.",
|
||||
"location": {
|
||||
"file": "src/Telemetry/Diagnostics.Codeunit.al",
|
||||
"line": 31
|
||||
},
|
||||
"references": [
|
||||
{ "path": "community/knowledge/telemetry/default-telemetryscope-to-extensionpublisher.md" }
|
||||
],
|
||||
"confidence": "high",
|
||||
"suggested-code": "TelemetryScope::ExtensionPublisher"
|
||||
}
|
||||
],
|
||||
"suppressed": []
|
||||
}
|
||||
```
|
||||
|
||||
The empty-corpus case produces:
|
||||
Output conforms to the DO output contract. The empty-corpus case produces:
|
||||
|
||||
```json
|
||||
{
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
||||
- **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`, action definitions, field-level properties, 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`, `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).
|
||||
- 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).
|
||||
|
||||
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.
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue