Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 77 additions & 2 deletions .github/scripts/Test-SkillIndex.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@ function Assert-ThrowsLike {
}

$generator = Join-Path $Root 'tools/Build-SkillIndex.ps1'
$resolver = Join-Path $Root 'tools/Resolve-SkillWorklist.ps1'
$indexSchema = Join-Path $Root 'schemas/skill-index.schema.json'
$reportSchema = Join-Path $Root 'schemas/findings-report.schema.json'
foreach ($path in $generator, $indexSchema, $reportSchema) {
foreach ($path in $generator, $resolver, $indexSchema, $reportSchema) {
if (-not (Test-Path -LiteralPath $path -PathType Leaf)) {
throw "Required contract file not found: $path"
}
Expand Down Expand Up @@ -235,9 +236,83 @@ Output.
Assert-ThrowsLike -Pattern '*Nested super-skills are not supported*' -Action {
& $generator -BCQualityRoot $fixtureRoot -IndexPath (Join-Path $tmp 'nested.json')
}

Remove-Item -LiteralPath (Join-Path $fixtureSkills 'al-nested-review.md') -Force
$validSuper = @'
---
kind: action-skill
id: al-code-review
version: 1
title: Review
description: Test super-skill.
inputs: [file-path]
outputs: [findings-report]
sub-skills:
- microsoft/skills/review/al-leaf-review.md
---

# Review

## Source
Source.
## Relevance
Relevance.
## Worklist
Worklist.
## Action
Action.
## Output
Output.
'@
Set-Content -LiteralPath $superPath -Value $validSuper -Encoding utf8NoBOM

$customSkills = Join-Path -Path $fixtureRoot -ChildPath 'custom/skills/review'
New-Item -ItemType Directory -Path $customSkills -Force | Out-Null
$customLeaf = $leaf.Replace('title: Leaf', 'title: Custom Leaf')
Set-Content -LiteralPath (Join-Path $customSkills 'custom-leaf-review.md') -Value $customLeaf -Encoding utf8NoBOM

$layeredPath = Join-Path $tmp 'layered.json'
& $generator -BCQualityRoot $fixtureRoot -IndexPath $layeredPath | Out-Null
$layeredIndex = Get-Content -LiteralPath $layeredPath -Raw | ConvertFrom-Json
$layeredLeaves = @($layeredIndex.skills | Where-Object id -eq 'al-leaf-review')
if ($layeredLeaves.Count -ne 2) {
throw "Expected both layered al-leaf-review implementations, found $($layeredLeaves.Count)."
}
if ((@($layeredLeaves.layer | Sort-Object) -join ',') -cne 'custom,microsoft') {
throw 'Layered al-leaf-review implementations did not preserve custom and microsoft records.'
}

$resolved = & $resolver -BCQualityRoot $fixtureRoot -IndexPath $layeredPath -SuperSkillPath (
'microsoft/skills/review/al-code-review.md'
)
if ($resolved.subSkills.Count -ne 1 -or
$resolved.subSkills[0].path -cne 'custom/skills/review/custom-leaf-review.md') {
throw 'The custom implementation did not win the layered leaf slot.'
}

$microsoftOnly = & $resolver -BCQualityRoot $fixtureRoot -IndexPath $layeredPath -SuperSkillPath (
'microsoft/skills/review/al-code-review.md'
) -EnabledLayers microsoft
if ($microsoftOnly.subSkills.Count -ne 1 -or
$microsoftOnly.subSkills[0].path -cne 'microsoft/skills/review/al-leaf-review.md') {
throw 'Disabling the custom layer did not fall back to the Microsoft implementation.'
}

$customDisabled = & $resolver -BCQualityRoot $fixtureRoot -IndexPath $layeredPath -SuperSkillPath (
'microsoft/skills/review/al-code-review.md'
) -DisabledSkills 'custom/skills/review/custom-leaf-review.md'
if ($customDisabled.subSkills.Count -ne 1 -or
$customDisabled.subSkills[0].path -cne 'microsoft/skills/review/al-leaf-review.md') {
throw 'Disabling the custom implementation did not fall back to Microsoft.'
}

Set-Content -LiteralPath (Join-Path $customSkills 'duplicate-leaf-review.md') -Value $customLeaf -Encoding utf8NoBOM
Assert-ThrowsLike -Pattern '*Duplicate action-skill IDs within a layer: custom:al-leaf-review*' -Action {
& $generator -BCQualityRoot $fixtureRoot -IndexPath (Join-Path $tmp 'duplicate-layer.json')
}
}
finally {
Remove-Item -LiteralPath $tmp -Recurse -Force -ErrorAction SilentlyContinue
}

Write-Output "Skill-index check PASSED: deterministic, schema-valid, and all $($expectedLeaves.Count) review leaves preserved in order."
Write-Output "Skill-index check PASSED: deterministic, schema-valid, layered overrides resolved, and all $($expectedLeaves.Count) review leaves preserved in order."
8 changes: 6 additions & 2 deletions .github/scripts/validate_frontmatter.py
Original file line number Diff line number Diff line change
Expand Up @@ -688,12 +688,16 @@ def run(root: Path) -> Report:
if domain_dir.is_dir():
validate_samples_in_domain(domain_dir, root, report)

# Third pass: R24 unique ids within kind
# Third pass: R24 unique ids within kind. Layered action-skill overrides
# may share an id, but two definitions in one layer are ambiguous.
by_kind: dict[str, dict[str, list[Path]]] = {}
for rec in skill_records:
if rec.skill_id is None:
continue
by_kind.setdefault(rec.kind, {}).setdefault(rec.skill_id, []).append(rec.path)
scope = rec.kind
if rec.kind == "action-skill":
scope = f"{rec.kind}:{rec.path.relative_to(root).parts[0]}"
by_kind.setdefault(scope, {}).setdefault(rec.skill_id, []).append(rec.path)
for kind, by_id in by_kind.items():
for sid, paths in by_id.items():
if len(paths) > 1:
Expand Down
21 changes: 20 additions & 1 deletion .github/workflows/review-fixtures.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,26 @@ jobs:
steps:
- name: Check out repository
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
fetch-depth: 0

- name: Collect changed paths
shell: pwsh
env:
BASE_SHA: ${{ github.event.pull_request.base.sha || github.event.before }}
run: |
$paths = if (-not $env:BASE_SHA -or $env:BASE_SHA -match '^0+$') {
@(git diff-tree --no-commit-id --name-only -r $env:GITHUB_SHA)
} else {
@(git diff --name-only $env:BASE_SHA $env:GITHUB_SHA)
}
$paths | Set-Content -LiteralPath "$env:RUNNER_TEMP/changed-paths.txt" -Encoding utf8NoBOM

- name: Validate review evaluation corpus
shell: pwsh
run: ./tools/Test-ReviewFixtures.ps1 -Root . -PrepareDirectory "$env:RUNNER_TEMP/bcquality-review-fixtures"
run: |
./tools/Test-ReviewFixtures.ps1 -Root . `
-PrepareDirectory "$env:RUNNER_TEMP/bcquality-review-fixtures" `
-ChangedPathsFile "$env:RUNNER_TEMP/changed-paths.txt" `
-CoverageReportPath "$env:RUNNER_TEMP/review-coverage.json"
Get-Content -LiteralPath "$env:RUNNER_TEMP/review-coverage.json"
9 changes: 9 additions & 0 deletions docs/customizing-bcquality.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,15 @@ 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`.

Action skills use the same layer order but override by frontmatter `id`. A
custom leaf with the same `id` as a Community or Microsoft leaf replaces that
leaf in every super-skill slot while its layer is enabled. The files may have
different names. IDs must remain unique within each layer. Disabling the custom
layer or the custom skill path makes composition fall back to the next enabled
implementation. Hosts should build the skill index and use
`tools/Resolve-SkillWorklist.ps1`; they must not implement this selection from
filenames.

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
Expand Down
16 changes: 11 additions & 5 deletions docs/standalone-runner.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,20 @@ only result.
action skills to run. Do not reproduce its routing logic.
3. Execute every dispatched action skill with the exact input subset in its
dispatch record. Read `skills/read.md` and `skills/do.md` on demand.
4. When an action skill declares `sub-skills`, execute every relevant leaf as a
discrete invocation. Leaves are independent and may be scheduled serially
or concurrently.
4. When an action skill declares `sub-skills`, resolve its ordered leaf slots
with `tools/Resolve-SkillWorklist.ps1`, passing the enabled layers and
disabled skill paths from the task context. Execute every resolved leaf as
a discrete invocation. Leaves are independent and may be scheduled serially
or concurrently.
5. Capture the exact Task return as the immutable raw audit payload and primary
transport. Preserve it unchanged in private artifacts or host logs. Before
the full DO acceptance gate, create a normalized candidate only for DO's
bounded optional-range case, record that normalization separately in private
telemetry, and accept the candidate only if the entire copy passes the
unchanged strict gate. The accepted report contains no undeclared telemetry
fields.
unchanged strict gate. Use `tools/Validate-FindingsReport.ps1`, passing the
exact source paths and fully retrieved article paths; pass `-SkillKind super`
for the final rolled-up report. The accepted report contains no undeclared
telemetry fields.
6. Collect each accepted findings-report into `sub-results` in the declared
`sub-skills` order, not completion order. Run the super-skill self-review
only after all leaves have finished.
Expand Down Expand Up @@ -92,6 +96,8 @@ A compatible runner:

- invokes every worklisted leaf exactly once unless a documented retry replaces
a failed attempt;
- resolves same-ID leaf implementations by `custom > community > microsoft`,
preserves declared slot order, and falls back when a higher layer is disabled;
- keeps leaf contexts isolated and passes only the inputs they declare;
- preserves each raw Task return unchanged for audit and distinguishes it from
any normalized accepted copy;
Expand Down
16 changes: 16 additions & 0 deletions evaluation/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,15 @@ The evaluation is convention-driven. The harness discovers every `<layer>/skills

`review-fixtures.json` contains only global thresholds and optional exceptional overrides. An override may select a different `article`, add context when the generic convention cannot express a scenario, or use an `articles` array when one domain needs explicit regression coverage for several paired articles. Specify either `article` or `articles`, not both. The first selected article retains the stable `<domain>-bad` and `<domain>-good` manifest IDs; additional articles use slug-qualified IDs. Overrides should remain empty in the normal case.

CI also measures selected paired articles against every effective article that
has both AL companions. A changed paired article must be selected by the
domain convention or an override. When adding it would not provide a useful
deterministic regression, add a narrow `coverageWaivers` entry with its exact
article path and a non-empty reason. Waivers are reviewable exceptions, not a
substitute for domain coverage. The generated coverage report includes totals
and per-domain ratios; the ratio is informational, while changed-file coverage
is mandatory.

Model-facing preparation hashes case IDs, neutralizes `Good`/`Bad` object-name tokens, and removes full-line sample comments so neither the article slug, domain, nor expected outcome reveals the answer.

The SCM `articles` override deliberately selects every rule in the initial
Expand Down Expand Up @@ -36,6 +45,13 @@ pwsh ./tools/Test-ReviewFixtures.ps1 -Root .

This credential-free check proves every selected leaf maps to a same-named knowledge domain with at least one complete AL sample pair and that all configured overrides are valid.

To reproduce the changed-file gate and emit the same measurable report as CI:

```powershell
git diff --name-only origin/main...HEAD | Set-Content .changed-paths.txt
pwsh ./tools/Test-ReviewFixtures.ps1 -Root . -ChangedPathsFile .changed-paths.txt -CoverageReportPath .coverage.json
```

## Run a fast-model evaluation

1. Prepare neutral inputs:
Expand Down
23 changes: 20 additions & 3 deletions evaluation/review-fixtures.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"selection": "first-paired-al-article",
"minimumExpectedRecall": 1.0,
"minimumCleanRate": 1.0,
"coverageWaivers": [],
"overrides": {
"agents": {
"article": "wire-all-three-agent-interfaces"
Expand Down Expand Up @@ -75,7 +76,14 @@
"reconcile-warehouse-adjustments-with-the-item-ledger",
"post-transfers-through-shipment-and-receipt-codeunits",
"use-date-aware-availability-for-promising",
"carry-out-requisition-actions-through-the-standard-workflow"
"carry-out-requisition-actions-through-the-standard-workflow",
"derive-base-quantities-through-the-line-unit-of-measure"
]
},
"security": {
"articles": [
"al-has-no-built-in-htmlencode",
"do-not-concatenate-external-text-into-setfilter"
]
},
"style": {
Expand All @@ -88,14 +96,23 @@
"article": "telemetry-event-id-stable-unique"
},
"testing": {
"article": "ui-handlers-in-tests"
"articles": [
"ui-handlers-in-tests",
"reset-per-test-state-before-the-isinitialized-guard"
]
},
"upgrade": {
"article": "initvalue-does-not-update-existing-rows",
"context": "The extended table existed in the previous app version and already contains rows."
},
"web-services": {
"article": "expose-systemid-as-the-api-key"
"articles": [
"expose-systemid-as-the-api-key",
"handle-httpclient-platform-failure-before-response-access",
"check-http-status-before-consuming-response-body",
"check-json-null-before-converting-values",
"format-exchanged-values-with-standard-format-9"
]
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
codeunit 50181 "Scanner Receipt Import Bad"
{
procedure PostScannedReceipt(ItemNo: Code[20]; LocationCode: Code[10]; UnitOfMeasureCode: Code[10]; ScannedQuantity: Decimal; DocumentNo: Code[20])
var
ItemJournalLine: Record "Item Journal Line";
ItemJnlPostLine: Codeunit "Item Jnl.-Post Line";
begin
if ScannedQuantity <= 0 then
Error(PositiveQuantityErr);

ItemJournalLine.Init();
ItemJournalLine.Validate("Posting Date", WorkDate());
ItemJournalLine.Validate("Entry Type", ItemJournalLine."Entry Type"::"Positive Adjmt.");
ItemJournalLine.Validate("Document No.", DocumentNo);
ItemJournalLine.Validate("Item No.", ItemNo);
ItemJournalLine.Validate("Location Code", LocationCode);
ItemJournalLine."Unit of Measure Code" := UnitOfMeasureCode;
ItemJournalLine.Quantity := ScannedQuantity;
ItemJournalLine."Quantity (Base)" := ScannedQuantity;

ItemJnlPostLine.RunWithCheck(ItemJournalLine);
end;

var
PositiveQuantityErr: Label 'The scanned quantity must be greater than zero.';
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
codeunit 50180 "Scanner Receipt Import Good"
{
procedure PostScannedReceipt(ItemNo: Code[20]; LocationCode: Code[10]; UnitOfMeasureCode: Code[10]; ScannedQuantity: Decimal; DocumentNo: Code[20])
var
ItemJournalLine: Record "Item Journal Line";
ItemJnlPostLine: Codeunit "Item Jnl.-Post Line";
begin
if ScannedQuantity <= 0 then
Error(PositiveQuantityErr);

ItemJournalLine.Init();
ItemJournalLine.Validate("Posting Date", WorkDate());
ItemJournalLine.Validate("Entry Type", ItemJournalLine."Entry Type"::"Positive Adjmt.");
ItemJournalLine.Validate("Document No.", DocumentNo);
ItemJournalLine.Validate("Item No.", ItemNo);
ItemJournalLine.Validate("Location Code", LocationCode);
ItemJournalLine.Validate("Unit of Measure Code", UnitOfMeasureCode);
ItemJournalLine.Validate(Quantity, ScannedQuantity);

ItemJnlPostLine.RunWithCheck(ItemJournalLine);
end;

var
PositiveQuantityErr: Label 'The scanned quantity must be greater than zero.';
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
---
bc-version: [all]
domain: scm
keywords: [quantity-base, qty-per-unit-of-measure, unit-of-measure-code, unit-of-measure-management, calcbaseqty, getqtyperunitofmeasure, qty-rounding-precision, item-journal-line]
technologies: [al]
countries: [w1]
application-area: [all]
---

# Derive base quantities through the line's unit of measure

## Description

Inventory, item ledger entries, reservations, item tracking, and warehouse quantities are measured in the item's base unit of measure. Document and journal lines hold `Quantity` in the line's `"Unit of Measure Code"`, together with `"Qty. per Unit of Measure"` and base-unit fields such as `"Quantity (Base)"`. Ten boxes of twelve pieces are 120 base units, not 10. When a line's `Quantity` is validated, the table derives the base quantity through its `CalcBaseQty` procedure. That procedure calls `"Unit of Measure Management".CalcBaseQty` with the line's quantity rounding precision and raises an error when rounding would turn a non-zero quantity into a zero base quantity. Code that bypasses this conversion creates a line whose quantity and base quantity disagree, or makes a stock decision in the wrong unit.

## Best Practice

On a document or journal line, validate `"Unit of Measure Code"` before `Quantity`, and validate both. Validating the unit of measure sets `"Qty. per Unit of Measure"` from the item unit of measure; validating the quantity then fills the base fields with the correct rounding. Compare line quantities with inventory or availability in base units, for example `"Quantity (Base)"` or `"Outstanding Qty. (Base)"`.

Outside a line, get the factor with `"Unit of Measure Management".GetQtyPerUnitOfMeasure(Item, UnitOfMeasureCode)` and convert with its `CalcBaseQty` or `CalcQtyFromBase` procedures instead of multiplying by hand. Pass the item unit's quantity rounding precision where the available overload accepts it.

Reading these fields for display, reporting, or a temporary buffer that is never posted is not a conversion defect. Code that proves the line uses the base unit of measure (`"Qty. per Unit of Measure"` equal to 1) is also correct, but don't assume this from the item alone, because a line can use another unit.

See sample: [`derive-base-quantities-through-the-line-unit-of-measure.good.al`](derive-base-quantities-through-the-line-unit-of-measure.good.al).

## Anti Pattern

Assigning `Quantity` directly on an item journal, sales, purchase, or transfer line and then inserting, modifying, or posting it. Also assigning a base field such as `"Quantity (Base)" := Quantity`, multiplying by a hard-coded or separately looked-up factor without the line's rounding, or comparing a line's `Quantity` with `Item.Inventory` or another base-unit value. Detection signal: a direct `:=` to `Quantity`, `"Qty. per Unit of Measure"`, or a `(Base)` quantity field on a persisted or posted line, or a comparison between a non-base line quantity and an inventory quantity.

See sample: [`derive-base-quantities-through-the-line-unit-of-measure.bad.al`](derive-base-quantities-through-the-line-unit-of-measure.bad.al).

## References

- [Set up units of measure, including quantity rounding precision](https://learn.microsoft.com/en-us/dynamics365/business-central/inventory-how-setup-units-of-measure)
- [BCApps: Unit of Measure Management conversions](https://github.com/microsoft/BCApps/blob/4abbb8ff848cdcb4e1187fc7a3e2da0612dd0d2b/src/Layers/W1/BaseApp/Foundation/UOM/UnitofMeasureManagement.Codeunit.al)
- [BCApps: Item Journal Line quantity validation and CalcBaseQty](https://github.com/microsoft/BCApps/blob/4abbb8ff848cdcb4e1187fc7a3e2da0612dd0d2b/src/Layers/W1/BaseApp/Inventory/Journal/ItemJournalLine.Table.al)
- [BCApps: Sales Line quantity validation and CalcBaseQty](https://github.com/microsoft/BCApps/blob/4abbb8ff848cdcb4e1187fc7a3e2da0612dd0d2b/src/Layers/W1/BaseApp/Sales/Document/SalesLine.Table.al)
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
codeunit 50161 "Cancel External Quotes Bad"
{
procedure CancelQuote(ExternalDocumentNo: Text)
var
SalesHeader: Record "Sales Header";
begin
SalesHeader.SetRange("Document Type", SalesHeader."Document Type"::Quote);
SalesHeader.SetFilter("External Document No.", ExternalDocumentNo);
SalesHeader.DeleteAll(true);
end;
}
Loading
Loading