diff --git a/README.md b/README.md index 43032d4..3ef86f5 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,7 @@ Skills define how agents consume knowledge. They come in three flavors: ### Agent bootstrapping -An orchestrator (such as AL-Go) points the agent at BCQuality's URL and provides a task context. The agent's first call is `/skills/entry.md`, which returns a dispatch record naming the action skill(s) to invoke. The agent then invokes each dispatched skill in turn, reading READ and DO on demand. No prior knowledge of BCQuality's structure is baked into the orchestrator — only the convention *"invoke `/skills/entry.md` first."* +An orchestrator (such as AL-Go) points the agent at BCQuality's URL and provides a task context. The agent's first call is `/skills/entry.md`, which returns a dispatch record naming the action skill(s) to invoke. The agent then invokes the dispatched skills, reading READ and DO on demand. No prior knowledge of BCQuality's structure is baked into the orchestrator — only the convention *"invoke `/skills/entry.md` first."* ### Standalone plugin installation @@ -108,6 +108,11 @@ formats. Their paths make the boundary explicit. The adapter lives under `skills/al-code-review/SKILL.md`; the internal Microsoft-layer coordinator lives at `microsoft/skills/review/al-code-review.md`. +Partners that want model selection, parallel leaf execution, retries, or usage +telemetry can add a thin runner outside BCQuality. See +[Build a lightweight standalone review runner](standalone-runner.md) for the +integration contract and a minimal implementation checklist. + ## Knowledge file format Every knowledge file is a markdown file with mandatory YAML frontmatter. Files target under 100 lines (ideal under 50). If two ideas would share a file, split them. diff --git a/microsoft/skills/review/al-code-review.md b/microsoft/skills/review/al-code-review.md index 9b3f934..1ab77b6 100644 --- a/microsoft/skills/review/al-code-review.md +++ b/microsoft/skills/review/al-code-review.md @@ -63,18 +63,18 @@ The worklist is the list of sub-skills judged relevant by the previous step. Eve ### Execution discipline (mandatory) -The Action step is a sequence of **discrete iterations**, not one combined generation. The contract requires the super-skill to invoke each sub-skill in turn and then perform a self-review pass. Concretely this means: +The Action step consists of **discrete leaf invocations**, not one combined generation. Invocation scheduling belongs to the orchestrator: independent leaves may run serially or concurrently, but their evaluation contexts and findings-reports remain isolated. Concretely this means: - **Isolate leaf invocations when the host supports it.** For fast/small models, each sub-skill SHOULD run in a fresh model call or child context containing only the task input, READ/DO contracts, the leaf instructions, a domain-filtered slice of the current knowledge index, and articles that leaf worklists. Preserve each index row's exact `path`; the leaf must copy references from that slice. The coordinator then collects the resulting JSON. This is the preferred fast-model profile: it bounds context, prevents later leaves from being skipped as attention is exhausted, and removes any reason to synthesize article paths. -- Treat each sub-skill in the worklist as its own pass: read the sub-skill's instructions, apply its Source → Relevance → Worklist → Action steps to the orchestrator-supplied inputs, and produce that sub-skill's complete findings-report before moving on. +- Treat each sub-skill in the worklist as its own pass: read the sub-skill's instructions, apply its Source → Relevance → Worklist → Action steps to the orchestrator-supplied inputs, and produce that sub-skill's complete findings-report independently. - Do not collapse multiple sub-skills into one shared reasoning step. Each sub-skill has a distinct knowledge subset and a distinct evaluation procedure; sharing one rolled-up scan dilutes per-skill attention and causes leaves to silently underreport (this has been observed in production: leaf skills returned empty `findings[]` while their standalone runs against the same diff produced multiple matches). - The agent self-review pass is its own final iteration. Begin it only after every sub-skill in the worklist has completed and its sub-result is recorded. -- Sub-skills are independent: re-walking the diff once per sub-skill is correct and expected. The output schema accommodates this — `sub-results` carries one entry per sub-skill, each a complete findings-report. +- Sub-skills are independent: re-walking the diff once per sub-skill is correct and expected. The output schema accommodates this — `sub-results` carries one entry per sub-skill, each a complete findings-report, in the frontmatter `sub-skills` order regardless of completion order. - When isolated calls are unavailable and the current model cannot finish every leaf within its budget, return `partial` with completed `sub-results` and name the first unevaluated sub-skill in `outcome-reason`. Never silently mark the remaining leaves clean. ### Roll up sub-skill findings -For each sub-skill in the worklist, executed one at a time per the discipline above: +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`. 2. Capture the sub-skill's complete findings-report verbatim and append it to `sub-results`. @@ -116,7 +116,7 @@ Sub-skills MAY also emit `suggested-code` when their knowledge file unambiguousl ### 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). +Calculate `summary.counts` from the final top-level `findings[]`, after failed sub-results have been excluded and duplicates have been merged. Aggregate `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`. diff --git a/skills/do.md b/skills/do.md index a79c5fc..1cd69ec 100644 --- a/skills/do.md +++ b/skills/do.md @@ -231,7 +231,7 @@ Omit `suggested-code` only when the appropriate fix depends on context the skill - `reference` — the suppressed file (same object shape as `findings[].references`). - `reason` — `layer-precedence` when another layer won under READ's precedence rules; `configuration` when the consumer disabled the file's layer. -**`sub-results`** — super-skills only. Array of complete findings-reports, one per sub-skill that was invoked (i.e., every sub-skill not listed in `skipped-sub-skills`). Each entry MUST itself conform to this output contract. Leaf skills MUST NOT emit `sub-results`. +**`sub-results`** — super-skills only. Array of complete findings-reports, one per sub-skill that was invoked (i.e., every sub-skill not listed in `skipped-sub-skills`). Each entry MUST itself conform to this output contract. Entries MUST appear in the worklist's declared order, regardless of invocation or completion order. Leaf skills MUST NOT emit `sub-results`. **`skipped-sub-skills`** — super-skills only. Array of sub-skills that were declared in frontmatter but not invoked. `reason` is `configuration` when the orchestrator disabled the sub-skill, or `not-applicable` when the super-skill's Relevance step ruled it out. @@ -248,6 +248,20 @@ A **super-skill** is an action skill whose frontmatter declares a non-empty `sub Composition is flat: a super-skill MAY list only leaf skills (skills without their own `sub-skills`). Nested super-skills are not permitted in v1. +### Scheduling boundary + +The super-skill defines which leaves must run, the input and output contracts, +and how their results are composed. It does not prescribe a model, concurrency +limit, retry policy, or telemetry system. Those choices belong to the +orchestrator. + +Each leaf invocation MUST remain a discrete evaluation with its own complete +findings-report. An orchestrator MAY execute independent leaves serially or +concurrently, but MUST invoke every worklisted leaf, preserve `sub-results` in +the declared worklist order, and wait for every invocation to finish before +performing any super-skill self-review or final rollup. Scheduling MUST NOT +change relevance, coverage, failure, reference-integrity, or output semantics. + ### Section interpretation for super-skills The five required sections still apply. Their meaning shifts from knowledge files to sub-skills: @@ -274,7 +288,13 @@ When the worklist is empty (every sub-skill was skipped), `outcome` is `not-appl ### Rolled-up summary -`summary.counts` is the sum of sub-skill counts. `summary.coverage.worklist-size` and `items-evaluated` are the sums across invoked sub-skills. +`summary.counts` counts the findings in the super-skill's final top-level +`findings[]`, after failed sub-results have been excluded and duplicates have +been merged. It MUST NOT be calculated by summing sub-skill counts, because the +same concern may appear in more than one sub-result. + +`summary.coverage.worklist-size` and `items-evaluated` are the sums across +invoked sub-skills whose outcomes are not `failed`. ### Suppression scope diff --git a/standalone-runner.md b/standalone-runner.md new file mode 100644 index 0000000..cc5b7c4 --- /dev/null +++ b/standalone-runner.md @@ -0,0 +1,85 @@ +# Build a lightweight standalone review runner + +BCQuality contains review knowledge, routing, execution instructions, and +structured output contracts. It intentionally does not choose models, schedule +agents, retry failures, or collect usage telemetry. A standalone runner can add +those host-specific capabilities without copying Business Central rules out of +BCQuality. + +Use the built-in standalone plugin when the host's default execution is +sufficient. Build a runner when you need explicit control over cost, latency, +concurrency, or integration with another review surface. + +## Keep BCQuality current + +Install or update the plugin with GitHub Copilot CLI: + +```shell +copilot plugin install microsoft/BCQuality +copilot plugin update bcquality +``` + +A runner that reads BCQuality from a checkout should pin a commit or release +and upgrade it deliberately. Do not copy knowledge files or action-skill prose +into the runner; doing so creates a second, drifting quality policy. + +## Minimal runner flow + +1. Give the agent the review input and a task context containing the user's + actual goal, available input types, and any known BC applicability + dimensions. +2. Invoke `skills/entry.md`. Entry prepares the knowledge index and returns the + 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. +5. Collect each complete 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. +6. Apply the DO composition, failure, deduplication, reference-integrity, and + outcome rules. Return strict JSON before rendering it for people or another + system. + +The runner must never inspect the diff to skip a review domain. A leaf decides +its own task-level applicability and reports `not-applicable` or +`no-knowledge`. + +## Runner-owned choices + +Keep these settings and behaviors outside BCQuality: + +- coordinator and leaf models; +- serial or concurrent scheduling and maximum concurrency; +- retries, timeouts, and rate-limit handling; +- token, cost, duration, and actual-concurrency telemetry; +- conversion of the findings report into Markdown, annotations, or PR + comments. + +Model selection and requested concurrency are deployment choices, not review +rules. Evaluate them against representative applications before making them a +default. Report actual usage and concurrency only when the host exposes native +evidence; do not infer them from the requested profile. + +## Failure and output checklist + +A compatible runner: + +- invokes every worklisted leaf exactly once unless a documented retry replaces + a failed attempt; +- keeps leaf contexts isolated and passes only the inputs they declare; +- preserves every leaf report, including failed reports, in `sub-results`; +- excludes unreliable findings from failed leaves and returns `partial` when + only part of the review is reliable; +- orders `sub-results` by the declared worklist and orders rendered findings + deterministically; +- calculates top-level severity counts from deduplicated top-level findings, + not by summing leaf counts; +- preserves knowledge paths verbatim and verifies references before publishing; +- records the BCQuality commit or release used for the run. + +BC-ALAgents, AL-Go, a Copilot custom agent, or a small host-native plugin can +all implement this runner contract. They remain optional consumers: +BCQuality's knowledge and skills stay independent of their orchestration +choices.