mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-08-06 17:36:53 +01:00
Make BCQuality an additive knowledge layer with agent findings
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>
This commit is contained in:
parent
613c4b4019
commit
637e7ac602
4 changed files with 66 additions and 5 deletions
|
|
@ -118,6 +118,8 @@ Action skills follow a four-step pattern:
|
||||||
|
|
||||||
Every action skill produces output in a common format that orchestrators can consume without skill-specific parsing. The format is JSON and includes an `outcome` (so a clean run, a not-applicable skill, and a partial failure are all distinguishable), `findings` (what the skill observed), structured `references` back to the knowledge files that informed each finding, per-finding `confidence`, and a `suppressed` list recording any knowledge files overridden by layer precedence. This contract is defined in the Action Skill meta-skill so that orchestrators and action skills remain independently evolvable.
|
Every action skill produces output in a common format that orchestrators can consume without skill-specific parsing. The format is JSON and includes an `outcome` (so a clean run, a not-applicable skill, and a partial failure are all distinguishable), `findings` (what the skill observed), structured `references` back to the knowledge files that informed each finding, per-finding `confidence`, and a `suppressed` list recording any knowledge files overridden by layer precedence. This contract is defined in the Action Skill meta-skill so that orchestrators and action skills remain independently evolvable.
|
||||||
|
|
||||||
|
BCQuality is an **additive** knowledge layer: it augments the agent's review judgement, it does not replace it. Super-skills (such as `al-code-review`) run a self-review pass alongside their sub-skills and surface concerns the agent identified on its own, marked with `from-sub-skill: "agent"` and an empty `references: []` so consumers can render them distinctly from knowledge-backed findings. See [agent-consumption.md](agent-consumption.md) and [`skills/do.md`](skills/do.md) for the full contract.
|
||||||
|
|
||||||
The meta-skills in `/skills/` define this pattern. Every concrete action skill follows it.
|
The meta-skills in `/skills/` define this pattern. Every concrete action skill follows it.
|
||||||
|
|
||||||
For the end-to-end flow — from orchestrator trigger through to how output reaches developers — see [agent-consumption.md](agent-consumption.md).
|
For the end-to-end flow — from orchestrator trigger through to how output reaches developers — see [agent-consumption.md](agent-consumption.md).
|
||||||
|
|
|
||||||
|
|
@ -66,6 +66,17 @@ The orchestrator parses this **without skill-specific logic**. This is the point
|
||||||
### 7. Orchestrator integrates
|
### 7. Orchestrator integrates
|
||||||
The orchestrator turns findings into PR comments, build gates, or IDE diagnostics, and links the references back to the knowledge files so the PR author — human or agent — can read the guidance.
|
The orchestrator turns findings into PR comments, build gates, or IDE diagnostics, and links the references back to the knowledge files so the PR author — human or agent — can read the guidance.
|
||||||
|
|
||||||
|
## Knowledge-backed and agent findings
|
||||||
|
|
||||||
|
BCQuality is an **additive** knowledge layer. The agent surfaces two kinds of findings, both shaped to the same DO output contract:
|
||||||
|
|
||||||
|
- **Knowledge-backed findings** carry one or more entries in `references[]` pointing at BCQuality knowledge files. Their `id` is the primary file's repo-relative path. These are produced by leaf sub-skills and rolled up by super-skills.
|
||||||
|
- **Agent findings** are surfaced by a super-skill from its own self-review pass when no BCQuality knowledge file backs the concern. They are tagged with `from-sub-skill: "agent"`, carry an empty `references: []`, use a slug `id` prefixed `agent:`, and have `confidence` capped at `medium`. Their `message` is self-contained because there is no knowledge-file footer to fall back on.
|
||||||
|
|
||||||
|
Before a super-skill emits an agent finding, it validates the candidate against the BCQuality knowledge already loaded for the task: a matching file upgrades the candidate to a knowledge-backed finding (and merges or deduplicates against the relevant sub-skill output); a contradicting file suppresses the candidate. Only candidates with no BCQuality coverage become agent findings.
|
||||||
|
|
||||||
|
Orchestrators MAY render the two kinds differently — for example, by labelling agent findings or routing them to a separate review domain — and MAY apply independent severity floors. The `from-sub-skill: "agent"` marker is the contract.
|
||||||
|
|
||||||
## Why this architecture
|
## Why this architecture
|
||||||
|
|
||||||
- **Entry is the only hardcoded thing.** Orchestrators ship with one convention — *"invoke `/skills/entry.md` first"* — and nothing else. New action skills and new knowledge files are picked up automatically because Entry discovers them at dispatch time.
|
- **Entry is the only hardcoded thing.** Orchestrators ship with one convention — *"invoke `/skills/entry.md` first"* — and nothing else. New action skills and new knowledge files are picked up automatically because Entry discovers them at dispatch time.
|
||||||
|
|
|
||||||
|
|
@ -23,7 +23,7 @@ sub-skills:
|
||||||
|
|
||||||
Reviews AL source changes by composing the leaf AL review skills. This is the canonical reference implementation of a **super-skill** — skill authors writing composed reviews should copy its structure.
|
Reviews AL source changes by composing the leaf AL review skills. This is the canonical reference implementation of a **super-skill** — skill authors writing composed reviews should copy its structure.
|
||||||
|
|
||||||
`al-code-review` does not evaluate knowledge files directly. It invokes each of its sub-skills against the same task input, collects their findings-reports, and returns a rolled-up findings-report.
|
`al-code-review` does not evaluate knowledge files directly. It invokes each of its sub-skills against the same task input, collects their findings-reports, and then performs its own **self-review pass** over the diff using the agent's built-in BC and AL knowledge. BCQuality knowledge is an additive layer: anything the sub-skills found is cited from BCQuality, and anything the agent finds on its own is validated against BCQuality (cited if matched, suppressed if contradicted, surfaced as an **agent finding** otherwise). The result is a single rolled-up findings-report that mixes knowledge-backed and agent findings, each clearly tagged via `from-sub-skill`.
|
||||||
|
|
||||||
An orchestrator invokes this skill with either a `pr-diff` (the standard PR-review entry point) or a `file-path` (single-file review). The skill produces a single JSON document conforming to the DO output contract, extended with `sub-results` and — when applicable — `skipped-sub-skills`.
|
An orchestrator invokes this skill with either a `pr-diff` (the standard PR-review entry point) or a `file-path` (single-file review). The skill produces a single JSON document conforming to the DO output contract, extended with `sub-results` and — when applicable — `skipped-sub-skills`.
|
||||||
|
|
||||||
|
|
@ -60,6 +60,8 @@ The worklist is the list of sub-skills judged relevant by the previous step. Eve
|
||||||
|
|
||||||
## Action
|
## Action
|
||||||
|
|
||||||
|
### Roll up sub-skill findings
|
||||||
|
|
||||||
For each sub-skill in the worklist:
|
For each sub-skill in the worklist:
|
||||||
|
|
||||||
1. Invoke the sub-skill with the orchestrator's inputs, passing only the subset each sub-skill declares in its `inputs`.
|
1. Invoke the sub-skill with the orchestrator's inputs, passing only the subset each sub-skill declares in its `inputs`.
|
||||||
|
|
@ -67,7 +69,28 @@ For each sub-skill in the worklist:
|
||||||
3. If the sub-skill's `outcome` is `failed`, stop here for this sub-skill: its findings are not reliable per the DO contract and MUST NOT be copied into the super-skill's top-level `findings[]` or counted in `summary.counts`.
|
3. If the sub-skill's `outcome` is `failed`, stop here for this sub-skill: its findings are not reliable per the DO contract and MUST NOT be copied into the super-skill's top-level `findings[]` or counted in `summary.counts`.
|
||||||
4. Otherwise, append each entry from the sub-skill's `findings[]` to the super-skill's top-level `findings[]`, setting `from-sub-skill` to the sub-skill's `skill.id`. For non-citation findings (those whose `id` is a skill-defined slug rather than a reference path), prefix `id` with `<from-sub-skill>:` to prevent collisions across sub-skills. Other finding fields are preserved.
|
4. Otherwise, append each entry from the sub-skill's `findings[]` to the super-skill's top-level `findings[]`, setting `from-sub-skill` to the sub-skill's `skill.id`. For non-citation findings (those whose `id` is a skill-defined slug rather than a reference path), prefix `id` with `<from-sub-skill>:` to prevent collisions across sub-skills. Other finding fields are preserved.
|
||||||
|
|
||||||
Aggregate `summary.counts` and `summary.coverage` as the sums across invoked sub-skills whose `outcome` is not `failed`.
|
### Agent self-review pass
|
||||||
|
|
||||||
|
After the sub-skill rollup, perform a self-review pass against the same task input using the agent's built-in BC and AL knowledge. BCQuality is an **additive** knowledge layer: it augments the agent's review judgement, it does not replace it. The goal of this pass is to surface defects the agent recognises on its own — bugs, anti-patterns, error-handling gaps, AL idioms — that the leaf sub-skills did not catch because no BCQuality knowledge file covers them yet.
|
||||||
|
|
||||||
|
For every candidate the agent identifies in this pass:
|
||||||
|
|
||||||
|
1. **Validate against BCQuality knowledge.** Check the candidate against the knowledge files the sub-skills have already loaded for this task (visible via their `references` and `suppressed` lists in `sub-results`).
|
||||||
|
- If a BCQuality knowledge file matches the candidate, upgrade it to a knowledge-backed finding: cite the file in `references`, set `id` to the file's path, set `from-sub-skill` to the sub-skill that owns that knowledge domain, and merge with or deduplicate against any sub-skill finding that already covers the same concern at the same location.
|
||||||
|
- If a BCQuality knowledge file **explicitly contradicts** the candidate (its `## Best Practice` or `## Anti Pattern` says the opposite of what the agent flagged), suppress the candidate and do not surface it.
|
||||||
|
- Otherwise the candidate has no BCQuality coverage; emit it as an agent finding.
|
||||||
|
2. **Emit agent finding.** Per DO's *Agent findings* rules:
|
||||||
|
- `from-sub-skill: "agent"`
|
||||||
|
- `references: []`
|
||||||
|
- `id` is a skill-defined slug prefixed with `agent:` (for example, `agent:missing-error-handling-on-http-call`).
|
||||||
|
- `confidence` capped at `medium`.
|
||||||
|
- `message` is non-empty and self-contained, describing both the issue and a concrete recommendation. A consumer rendering the finding has no knowledge-file footer to fall back on.
|
||||||
|
|
||||||
|
Leaf sub-skills MUST NOT emit agent findings: their scope is bounded by the knowledge subset they evaluate. The self-review pass is a super-skill responsibility.
|
||||||
|
|
||||||
|
### Summary and rollup
|
||||||
|
|
||||||
|
Aggregate `summary.counts` and `summary.coverage` as the sums across invoked sub-skills whose `outcome` is not `failed`. Agent findings emitted by the super-skill itself contribute to `summary.counts` but not to `summary.coverage` (coverage is a sub-skill worklist metric and is undefined for self-review).
|
||||||
|
|
||||||
`suppressed[]` at the super-skill level remains empty. Knowledge-file-level suppression is reported by each sub-skill within its own entry in `sub-results`.
|
`suppressed[]` at the super-skill level remains empty. Knowledge-file-level suppression is reported by each sub-skill within its own entry in `sub-results`.
|
||||||
|
|
||||||
|
|
@ -82,7 +105,7 @@ Output conforms to the DO output contract, extended with `sub-results` and `skip
|
||||||
"skill": { "id": "al-code-review", "version": 1 },
|
"skill": { "id": "al-code-review", "version": 1 },
|
||||||
"outcome": "completed",
|
"outcome": "completed",
|
||||||
"summary": {
|
"summary": {
|
||||||
"counts": { "blocker": 1, "major": 1, "minor": 2, "info": 0 },
|
"counts": { "blocker": 1, "major": 1, "minor": 3, "info": 0 },
|
||||||
"coverage": { "worklist-size": 4, "items-evaluated": 4 }
|
"coverage": { "worklist-size": 4, "items-evaluated": 4 }
|
||||||
},
|
},
|
||||||
"findings": [
|
"findings": [
|
||||||
|
|
@ -143,6 +166,19 @@ Output conforms to the DO output contract, extended with `sub-results` and `skip
|
||||||
],
|
],
|
||||||
"confidence": "medium",
|
"confidence": "medium",
|
||||||
"from-sub-skill": "al-security-review"
|
"from-sub-skill": "al-security-review"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"id": "agent:missing-error-handling-on-http-client",
|
||||||
|
"severity": "minor",
|
||||||
|
"message": "HttpClient.Send is called without inspecting the response status or wrapping the call in a TryFunction. Network or remote-server failures will surface as runtime errors to the user. Recommendation: branch on the HttpResponseMessage.IsSuccessStatusCode and either retry, surface a controlled error, or fall back, depending on the integration's contract.",
|
||||||
|
"location": {
|
||||||
|
"file": "src/Integration/ApiClient.Codeunit.al",
|
||||||
|
"line": 60,
|
||||||
|
"range": { "start-line": 60, "end-line": 64 }
|
||||||
|
},
|
||||||
|
"references": [],
|
||||||
|
"confidence": "medium",
|
||||||
|
"from-sub-skill": "agent"
|
||||||
}
|
}
|
||||||
],
|
],
|
||||||
"suppressed": [],
|
"suppressed": [],
|
||||||
|
|
|
||||||
16
skills/do.md
16
skills/do.md
|
|
@ -133,6 +133,18 @@ An empty `findings` array with `outcome: completed` means the skill ran and foun
|
||||||
|
|
||||||
When a super-skill rolls up a non-citation finding from a sub-skill (an `id` that is a slug, not a path), the super-skill MUST prefix the `id` with `<from-sub-skill>:` to avoid collisions across sub-skills (for example, a slug `missing-test` from `al-security-review` becomes `al-security-review:missing-test`). Citation-based findings are already globally unique through their repo-relative path and MUST NOT be rewritten.
|
When a super-skill rolls up a non-citation finding from a sub-skill (an `id` that is a slug, not a path), the super-skill MUST prefix the `id` with `<from-sub-skill>:` to avoid collisions across sub-skills (for example, a slug `missing-test` from `al-security-review` becomes `al-security-review:missing-test`). Citation-based findings are already globally unique through their repo-relative path and MUST NOT be rewritten.
|
||||||
|
|
||||||
|
**Agent findings.** A super-skill MAY emit findings that the agent identified through its own reasoning rather than from a BCQuality knowledge file. BCQuality is an **additive** knowledge layer: it augments the agent's pre-existing review judgement, it does not replace it. An agent finding is encoded by:
|
||||||
|
|
||||||
|
- `from-sub-skill: "agent"` — the canonical marker. Use this exact value; do not invent equivalents.
|
||||||
|
- `references: []` — required. An agent finding has no knowledge-file citation by definition; if a citation existed, the finding would be a knowledge-backed finding instead.
|
||||||
|
- `id` — a skill-defined slug, prefixed with `agent:` (mirroring the `<from-sub-skill>:` rule). For example, `agent:obsolete-find-signature`.
|
||||||
|
- `confidence` — capped at `medium`. Without a knowledge-file citation there is no authoritative basis for `high` confidence.
|
||||||
|
- `message` — non-empty and self-contained. It MUST describe the issue and a concrete recommendation, since a consumer rendering the finding has no knowledge-file footer to fall back on.
|
||||||
|
|
||||||
|
Agent findings are emitted **only by super-skills** (the `al-code-review` super-skill is the canonical example). Leaf sub-skills MUST NOT emit agent findings: a leaf's job is to evaluate one knowledge subset, and a finding it cannot cite from that subset is out of scope for it. Before emitting an agent finding, a super-skill MUST validate the candidate against the BCQuality knowledge it has already loaded for the task — if a knowledge file matches, the candidate is upgraded to a knowledge-backed finding (and merged or deduplicated against any sub-skill output that already covers the same concern); if a knowledge file explicitly contradicts the candidate, it is suppressed.
|
||||||
|
|
||||||
|
Consumers that render output MAY treat agent findings differently from knowledge-backed findings (for example, by labelling them and routing them to a separate review domain). The `from-sub-skill: "agent"` marker is the contract they rely on.
|
||||||
|
|
||||||
**`findings[].severity`** — see the taxonomy below.
|
**`findings[].severity`** — see the taxonomy below.
|
||||||
|
|
||||||
**`findings[].message`** — human-readable explanation of the finding. Single short paragraph. No markdown formatting assumptions.
|
**`findings[].message`** — human-readable explanation of the finding. Single short paragraph. No markdown formatting assumptions.
|
||||||
|
|
@ -150,11 +162,11 @@ Findings without a `location` are permitted (for example, repository-wide observ
|
||||||
- `path` (required) — repo-relative path to the knowledge file, forward slashes.
|
- `path` (required) — repo-relative path to the knowledge file, forward slashes.
|
||||||
- `sha` (optional) — commit SHA the skill read when producing the finding. Consumers SHOULD include `sha` when the skill was invoked with a specific repo state.
|
- `sha` (optional) — commit SHA the skill read when producing the finding. Consumers SHOULD include `sha` when the skill was invoked with a specific repo state.
|
||||||
|
|
||||||
The first reference is the **primary** reference: the knowledge file the finding most directly cites. Additional references provide supporting context and are not ranked. `references` MAY be empty for findings the skill generates without a knowledge-file citation.
|
The first reference is the **primary** reference: the knowledge file the finding most directly cites. Additional references provide supporting context and are not ranked. `references` MAY be empty only for **agent findings** (see the `findings[].id` section above for the full encoding); any other finding MUST have at least one reference.
|
||||||
|
|
||||||
**`findings[].confidence`** — the skill's confidence that the finding is a true positive, given the evidence it evaluated. Not applicability confidence, not severity confidence. Values: `high`, `medium`, `low`.
|
**`findings[].confidence`** — the skill's confidence that the finding is a true positive, given the evidence it evaluated. Not applicability confidence, not severity confidence. Values: `high`, `medium`, `low`.
|
||||||
|
|
||||||
**`findings[].from-sub-skill`** — optional. Set only by super-skills. The `skill.id` of the sub-skill that produced the finding. Absent on findings produced directly by the emitting skill.
|
**`findings[].from-sub-skill`** — optional. Set only by super-skills. The `skill.id` of the sub-skill that produced the finding, or the literal string `"agent"` for an agent finding the super-skill produced from its own reasoning. Absent on findings produced directly by a leaf skill.
|
||||||
|
|
||||||
**`suppressed`** — MUST list every knowledge file that was discarded due to layer precedence or consumer configuration, whenever that file would otherwise have contributed to the worklist. Each entry contains:
|
**`suppressed`** — MUST list every knowledge file that was discarded due to layer precedence or consumer configuration, whenever that file would otherwise have contributed to the worklist. Each entry contains:
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue