Reviewing #112 surfaced that the authoring docs never state where a new BC
fact (or a false-positive guard) belongs, so an agent iterated do.md -> leaf
skills -> knowledge files across two review rounds before landing knowledge
in a knowledge article. The information to decide existed but was split across
README/do.md/write.md and framed only as positive best practices.
- do.md: add "Skills hold mechanics; knowledge files hold BC facts" — a skill
is a finder/applier; every BC behavioural claim it acts on must be a cited
knowledge file. Names negative knowledge (false-positive guards) as first
class, and forbids both adding a BC fact to a skill and restating an
article's fact inline (the drift/duplication smell).
- write.md: add "Is this a knowledge file?" decision gate at the top, plus a
"Negative knowledge is first-class" section with the Description/Best
Practice/Anti Pattern mapping and a worked example.
- README: note that false-positive-preventing files are first-class knowledge
and add a reviewer heuristic to Contributing.
Prose-only additions; validator passes. Meta-skill contract semantics are
unchanged, so version stays 1 (maintainers may bump if they consider the
explicit boundary rule a contract change).
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 76eba53e-18cd-4618-a205-3607f260f9f4
* 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>
* Add monthly manual versioning: VERSION file + release-version workflow
* Version as major.minor (1.0); bump minor monthly, major for breaking
* Clarify BCQuality minor is monotonic and never resets across major bumps
* Align comment with engine X.Y.Z scheme (minor = Z)
* Document BCQuality versioning in README
* Drop PRReviewAgent reference from README versioning section
* Derive release version from git tags, drop VERSION file
---------
Co-authored-by: wenjiefan <wenjiefan@microsoft.com>
De-enumerate the AL review leaf list so it lives in exactly one place:
the al-code-review super-skill's frontmatter sub-skills list.
- al-code-review.md: drop the enumerated parenthetical from description
and remove the redundant bullet re-list in the Source section.
- README.md: remove the hardcoded count word and the enumerated domain
list from the leaf-skill sentence.
- validate_frontmatter.py: add cross-file rule R26 asserting a
super-skill's declared sub-skills exactly match the sibling
al-*-review.md leaf files on disk (missing, stale, and unregistered
leaves all fail CI).
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>
Seed a new breaking-changes (AL API stability) knowledge domain with six
articles plus good/bad AL samples, a new al-breaking-changes-review leaf
skill, and minimal wiring into al-code-review and the README.
Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Add events knowledge domain and review leaf skill
Add a new `events` knowledge domain covering AL events & subscribers,
wired into the AL review pipeline.
- 3 atomic articles (+ .good.al/.bad.al samples) under
microsoft/knowledge/events/: the IsHandled override pattern, thin
OnBefore/OnAfter integration-event publishers, and static vs manual
subscribers.
- New leaf skill microsoft/skills/review/al-events-review.md sourcing the
events domain.
- Wired into microsoft/skills/review/al-code-review.md (sub-skills + Source
+ description) and README.md (leaf-skill count + domain list).
AL event syntax verified against Microsoft Learn. Samples are
demonstration-only (not compiled by CI). Additive change; no contract change.
Part of #34.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Add 12 general AL event-design articles to events domain
Add 12 atomic knowledge articles under microsoft/knowledge/events covering
general AL event-design best practices: IsHandled initialization and OnAfter
preservation, appending new event parameters, position-based event naming,
reusing/extending events, avoiding per-iteration publishing, Temp-prefixing
temporary record parameters, unabbreviated parameter names, preferring the
this keyword over IncludeSender, avoiding loosely typed parameters, not
mutating existing event contracts, and not bypassing critical operations
with IsHandled. Each article ships a .good.al and .bad.al demonstration
sample (object IDs 50240-50296; not compiled by CI). Extend the
al-events-review leaf Worklist with one targeted check per new rule.
Additive only; no contract or wiring change (events leaf already wired).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Refine events articles after review feedback
Correct wording in five events articles to reflect that AL event
subscribers bind by parameter name, not position:
- add-new-event-parameters-at-the-end: drop the inaccurate claim that
appending a parameter forces subscribers to be updated or causes wrong
values; keep the append-at-end best practice.
- do-not-add-ishandled-to-an-existing-event: reframe from "breaking
change" to the semantic/purpose shift that leaves existing subscribers
pointless; rename the breaking-change keyword to semantic-change.
- name-events-by-publisher-position: extend the good sample with
position-named publishers raised from table and report trigger
contexts.
- initialize-ishandled-to-false-before-publishing: scope the detection
and best practice to events that actually carry a var IsHandled, so an
OnBefore with no IsHandled is not flagged.
- do-not-bypass-critical-operations-with-ishandled: add a litmus-test
definition of a critical operation (code that cannot stand as an
independent, self-contained unit).
Knowledge-only; no contract or wiring change.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Soften Anti Pattern wording in add-new-event-parameters article
Remove the last name-vs-position misconception from the Anti Pattern so it
is consistent with the corrected Description: mid-list insertion is framed
as noisy and harder to review rather than as forcing subscriber re-mapping.
Detection sentence unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Add events domain reviewers to CODEOWNERS
Add @AleksandricMarko and @pchriste-microsoft-com as required reviewers
for the events knowledge domain, matching the existing per-domain expert
ownership convention. Inserted in alphabetical order ahead of the
performance line.
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>
Adds the interfaces knowledge domain covering AL interfaces and enum-with-implementation: three atomic articles with good/bad AL samples, a new al-interfaces-review leaf skill, and additive wiring into al-code-review and the README. Purely additive; no contract change.
Part of #34.
Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
A closed range like [23..28] wrongly implies guidance stops applying after
BC28, so a reviewer targeting BC29+ would not match the file. Introduce an
open-ended shorthand [N..] meaning ''version N and every later version''.
- validate_frontmatter.py: RANGE_SHORTHAND allows an optional upper bound;
expand_bc_version returns the normalized string ''N..'' for open-ended.
- read.md: document the fourth bc-version form and its matching rule
(matches target >= N; not enumerable).
- write.md: prefer [N..] over a closed range for a feature introduced in N
and not expected to be removed.
- Apply [23..] to the actionable-errors article (actionable errors shipped
in BC23 and are not version-bounded above).
- README: mention [N..] in the frontmatter example.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Let super-skills surface findings the agent identifies on its own,
clearly tagged so consumers can render them differently from
knowledge-backed ones.
- skills/do.md: permit references:[] when from-sub-skill='agent';
define the agent-finding encoding (id 'agent:<slug>', confidence
capped at medium, self-contained message); restrict agent findings
to super-skills only.
- microsoft/skills/review/al-code-review.md: add a self-review pass
to Action that validates agent-identified candidates against
BCQuality (cite if matched, suppress if contradicted, surface as
agent finding otherwise). Add example finding.
- agent-consumption.md, README.md: describe the additive model and
the from-sub-skill: 'agent' marker so consumer orchestrators know
to render unbacked findings.
Strictly additive: existing knowledge-backed flow is unchanged and
backward compatible.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The knowledge corpus now covers performance, security, privacy, upgrade,
style, and UI. Previously only two leaf reviewer skills existed
(al-performance-review, al-security-review), so four of the six domains
had knowledge with no skill sourcing from them. A community reader
landing in privacy/, upgrade/, style/, or ui/ would see articles with
no apparent consumer.
Three changes:
1. Move existing review skills into `microsoft/skills/review/`. The
`review/` subfolder groups all review-kind skills together and leaves
room for future non-review action skills at the `microsoft/skills/`
level. Updates references in README.md, agent-consumption.md, and
skills/entry.md to the new paths.
2. Add four new leaf reviewer skills — al-privacy-review,
al-upgrade-review, al-style-review, al-ui-review — each following
the same DO template as al-performance-review/al-security-review but
sourcing from the corresponding knowledge domain. al-upgrade-review
and al-ui-review return `not-applicable` when the diff contains no
upgrade surface or no page files, respectively.
3. Update al-code-review to compose all six leaf skills and retarget
the dangling references in every populated JSON example
(`use-setloadfields.md`, `no-plaintext-secrets-in-telemetry.md`,
`avoid-implicit-commit.md` — none of which exist in the corpus) to
real knowledge files: `call-setloadfields-before-filters.md`,
`use-secrettext-for-credentials.md`, `never-hardcode-secrets-in-al.md`.
Validator passes with 0 errors / 0 warnings.
Most of the corpus — FindSet/SetLoadFields/CalcFields patterns, permission
sets, SingleInstance codeunits, DataClassification, IsolatedStorage,
transaction scope, SecretText — describes BC platform behaviour that is
identical across supported versions. The seed [26..28] range on every
file implied a version-specificity the content does not actually have,
and there was no way to express "applies to every version" in the
schema the way [w1] and [all] already do for countries and
application-area.
Extend the v1 schema with a universal sentinel for bc-version, parallel
to the sentinels already defined for the other dimensions:
bc-version: [all] # applies to every BC version
[all] is mutually exclusive with explicit versions. Range shorthand
([26..28]) and explicit lists ([26, 27, 28]) continue to work for files
genuinely tied to a version-gated API or deprecation.
Update read.md (field definition, matching semantics, partial-context
rule), write.md (default to [all], use ranges only with a concrete
reason), README.md (frontmatter example), and the CI validator. All
forty existing knowledge files and the three action skills convert to
[all]; none of the current content is version-gated. Validator passes.
Remove seven knowledge files whose content is generic software-engineering
guidance that a capable LLM already applies without BCQuality present
(HTTPS-only, secret-leakage-in-errors, no-credentials-in-URLs, silent
security-error swallowing, short transaction scope, HTTP timeouts,
StrSubstNo-vs-concatenation). These fail the remedial-knowledge premise
and dilute the signal of the preview corpus.
Strip the "Seed article — domain stewards should expand" banner from ten
files that are ready to showcase (AA0232/AA0233 rules, FindSet read-only
semantics, SetLoadFields ordering and usage, CalcFields-in-loops,
SecretText end-to-end, DataClassification). The banner remains on files
that still need domain-steward refinement.
Add a "What belongs here" section to the README stating the admission
test: a file exists only if a modern LLM would get something wrong or
miss something without it. Gives contributors a concrete yes/no filter
before they open a PR.
- Introduced unit tests for the bc-domain-context implementation, covering various scenarios including filtering by application area, technology mismatches, layer precedence, and conditional applicability.
- Added knowledge files related to finance, including topics such as Chart of Accounts, Codeunit 12, Dimension Management, and VAT on prepayment chains, among others.
- Each knowledge file includes structured metadata and best practices to enhance the domain knowledge available for Business Central tasks.
Add a new skill kind, 'entry-point', and its sole instance at
skills/entry.md. When an orchestrator points an agent at BCQuality,
the agent's first call is Entry: it receives a task context and
returns a dispatch record naming the action skill(s) to invoke.
Routing logic lives in Entry, not in the orchestrator.
Entry structurally follows DO's Source -> Relevance -> Worklist ->
Action pattern but the units it selects are action skills (not
knowledge files) and its output is a dispatch record (not a
findings-report).
Contract highlights:
- Inputs semantics in DO clarified as any-of: orchestrator supplies
whichever listed input types it has; skill must return
'not-applicable' if the subset is insufficient. This matches
the existing al-* canonical skills which declare
[pr-diff, file-path] as alternatives.
- Relevance admits candidates whose inputs intersect
inputs-available, not whose inputs are a subset.
- Dispatched inputs are the intersection, not the full
inputs-available set, to avoid leakage between skills.
- Super-skill precedence in Worklist supersedes a sub-skill only
when the goal is a broader match for the super than the sub.
When the goal specifically names a concern the sub handles
(e.g., 'performance review'), the sub wins and the super is
dropped with reason 'narrower-sub-skill-selected'.
- Skill layer precedence is defined here as custom > community >
microsoft, matching READ's rule for knowledge files.
- skipped[] carries 'superseded-by' for layer-precedence,
sub-skill, and super-skill drops, for traceability.
Propagate the concept through:
- skills/README.md: distinguish the runtime entry-point skill from
the three meta-skill contracts.
- skills/do.md: acknowledge entry-point alongside meta-skills as
the only kinds that live outside a layer.
- README.md: rewrite the Skills and Agent bootstrapping sections
so the bootstrap instruction is 'invoke /skills/entry.md first'.
- agent-consumption.md: update the Mermaid flow and step narrative
to show Entry dispatch, with READ and DO read on demand.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The /samples/ top-level tree is replaced with sibling files in each
knowledge-layer folder. An article and its demonstrations now live
side-by-side:
microsoft/knowledge/<domain>/<slug>.md
microsoft/knowledge/<domain>/<slug>.good.al
microsoft/knowledge/<domain>/<slug>.bad.al
Rationale:
- Proximity. An article and its paired samples are one unit; the
filesystem now reflects that.
- Layer ownership. Samples inherit layer precedence for free -- a
/custom/ fork can override an article and its samples atomically,
which the shared /samples/ tree previously made awkward.
- Trivial migration path. Action-skill source globs
(*/knowledge/<domain>/**/*.md) are unchanged; sample discovery is a
sibling-filename lookup.
Changes:
- git mv of all 65 sample files from samples/<domain>/<slug>/{bad,good}.al
to microsoft/knowledge/<domain>/<slug>.{bad,good}.al (history preserved).
- Update See-sample references in all 37 articles that ship samples.
- skills/read.md: replace the no-code-blocks bullet with a pointer to a
new Sample files section that fully specifies the sibling convention,
the kinds (good/bad + forward-compatible), multi-technology rules,
demonstration-only status, and layer-precedence behaviour.
- skills/write.md: update the samples pointer to match.
- README.md: annotate the knowledge tree with the sample sibling shape.
- samples/README.md deleted; content lifted into skills/read.md.
- Both generators (C:\temp\gen_performance_knowledge.py,
C:\temp\gen_security_knowledge.py) updated to emit at the new paths
and to stop writing samples/README.md. Re-running them is idempotent
against the committed layout.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
DO contract (skills/do.md)
- New 'sub-skills' optional frontmatter field on action skills: when
present and non-empty, the skill is a super-skill that composes
other action skills.
- New 'Composition (super-skills)' section covering section
interpretation, outcome rollup, summary aggregation, and suppression
scope.
- Output schema gains three optional fields: 'from-sub-skill' on each
finding, top-level 'sub-results[]' carrying nested findings-reports,
and top-level 'skipped-sub-skills[]'.
- Super-skills MUST NOT filter sub-skills by task content; leaves own
task-level applicability and signal via outcome.
- Findings from a failed sub-skill MUST NOT flow into the parent's
findings[] or counts, consistent with DO's rule that consumers
ignore a failed skill's findings. Reports are still preserved in
sub-results[] for traceability.
- Rolled-up non-citation finding ids MUST be prefixed with the sub-
skill id to prevent collisions across sub-skills. Citation-based
ids are already unique via repo path and are not rewritten.
- Outcome rollup rules updated: 'partial' covers S = {partial},
{partial, partial}, and {partial, failed}. Empty worklist rolls up
to 'not-applicable' with outcome-reason.
- Nested super-skills are not permitted in v1.
Reference skills (microsoft/skills/)
- al-code-review.md rewritten as the canonical super-skill: lists
al-performance-review and al-security-review as sub-skills, orch-
estrates invocation, aggregates output, and includes a worked
rolled-up JSON example plus the empty-corpus rollup.
- al-performance-review.md added as a leaf reference skill for the
performance knowledge domain.
- al-security-review.md added as a leaf reference skill for the
security knowledge domain.
- Both leaves retain the leaf-level rules validated in the prior
pass: partial-context message requirement, worklist-scoped
suppression, application-area semantics, and the platform-guarantee
threshold for blocker severity.
README updated to describe leaf vs super-skill and link all three
reference skills.
Two rubber-duck passes tightened the contract and caught schema
violations in the worked examples before commit.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- microsoft/skills/al-code-review.md: full action skill reviewing AL
source changes. Exercises every part of the DO contract:
- frontmatter with all required action-skill fields and the four
optional knowledge filters
- all five required sections (Source, Relevance, Worklist, Action,
Output) in order
- applicability-based filtering with READ's partial-context rule,
including the requirement to name unknown dimensions in the
finding message
- conflict resolution and suppression scoped to worklist candidates
- full outcome taxonomy handled (completed, no-knowledge,
not-applicable, partial, failed), including the current empty-
corpus state
- worked JSON output example showing structured references,
per-finding confidence, and a suppression entry
- README.md: link the skill as the canonical reference so skill
authors have a starting point.
Two rubber-duck passes caught and fixed contract precision issues
before commit: location.line/range consistency, partial-context
message requirement, over-broad suppression scope, application-area
matching semantics, and the blocker severity threshold.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- skills/read.md (READ, Schema + Use) — knowledge-file contract:
frontmatter schema, required/optional sections (normative vs
non-normative), layer precedence with applicability-based conflict
detection, explicit frontmatter matching semantics (including
partial-context handling).
- skills/do.md (DO, Action Skill) — action-skill template:
frontmatter schema, required sections, four-step pattern
(Source -> Relevance -> Worklist -> Action), and the output contract
as a JSON schema with outcome, findings, structured references,
confidence, and mandatory suppression recording. Includes a worked
example.
- skills/write.md (WRITE, New Knowledge) — authoring guide:
atomicity, size, section guidance, field-by-field choices, file
naming, layer choice, pre-PR checklist. Defers to READ for the
format spec.
- README.md — link the three files from the meta-skills section and
update the output-contract paragraph to include outcome and
suppressed.
- agent-consumption.md — update step 5 (Agent emits structured output)
to match the richer DO contract.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
