From 822cae1b2771ac25f665f73369f69093bd4fd630 Mon Sep 17 00:00:00 2001 From: Jesper Schulz-Wedde Date: Thu, 4 Jun 2026 15:02:12 +0200 Subject: [PATCH] Own the knowledge-index generator + index-aware review skills (#25) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Make domain-skill knowledge discovery index-aware The 6 AL domain review skills and read.md now enumerate candidate articles from the BCQuality knowledge index (knowledge-index.json) instead of opening every file under the domain folder to read its frontmatter. The worklist selection predicate is unchanged (keywords intersect diff tokens, or topic matches a changed object type) - only the discovery source changes, so the same articles are selected. Full article bodies are read only for worklisted entries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Reconcile §Source wording with the lean knowledge index The BCQuality filter now emits a lean index whose per-article description is a one-line hint rather than the full verbatim Description. Update the six domain skills' §Source to say the index carries a one-line description hint (keywords, title, and a one-line description) instead of the full description. The worklist selection predicate is unchanged: keywords drive selection and the agent opens worklisted articles in full for their rule bodies. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Own the knowledge-index generator in BCQuality The knowledge index is an acceleration of the skills' Source step, and its schema is part of that contract — so BCQuality should own the generator rather than each consumer re-implementing it. Add tools/Build-KnowledgeIndex.ps1 (the parser + lean-description shaping + emit, lifted verbatim from the BCAppsBCQuality filter prototype) and document the index in agent-consumption.md. Consumers prune their clone to policy, then call this script; the index stays in lockstep with the Source contract and every orchestrator gets the same faithful index for free. The worklist selection predicate is unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Own knowledge-index generation in BCQuality (runtime + CI), not the consumer The index is now produced by BCQuality itself: Entry's preparation step rebuilds knowledge-index.json over the live, already-pruned clone at the start of every run, and a new CI workflow validates the generator's health (determinism, full coverage, selection-input integrity). Consumers no longer invoke or know about the index. Rebuilding over the pruned clone (vs shipping a committed full-corpus index) keeps the index exact for any consumer policy: it can never list a denied article, so policy-excluded rules cannot leak into discovery. READ now states the index is discovery-only -- a finding must cite an article opened in full, and rows whose file is absent are discarded before ranking. - skills/entry.md: new 'Preparation -- knowledge index' precondition - skills/read.md: index ownership + discovery-only invariant - microsoft/skills/review/*.md (6): 'BCQuality builds' (not 'the filter emits') - agent-consumption.md 5a: runtime+CI ownership rationale - .github/workflows/knowledge-index.yml + scripts/Test-KnowledgeIndex.ps1: generator guard - .gitignore: never commit the runtime index Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Make runtime index build non-interactive and self-contained entry.md now gives the exact build command (pwsh ./tools/Build-KnowledgeIndex.ps1) so the agent's preparation step is unambiguous, and the generator's -BCQualityRoot parameter is optional (defaults to the clone root) so it runs in non-interactive -p mode without prompting. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Resolve knowledge-index root to absolute path (cross-platform fix) Get-ChildItem.FullName is always absolute, so deriving the relative article path via Substring(\.Length) requires an absolute root. A relative root such as '.' (used by the CI guard's 'Test-KnowledgeIndex.ps1 -Root .') left the full path almost intact on Linux, producing bogus 'home/runner/.../knowledge' paths and failing the coverage check. Normalise both the generator's -BCQualityRoot and the test's -Root with Resolve-Path before use. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Jesper Schulz-Wedde --- .github/scripts/Test-KnowledgeIndex.ps1 | 93 +++++++ .github/workflows/knowledge-index.yml | 24 ++ .gitignore | 4 + agent-consumption.md | 8 + .../skills/review/al-performance-review.md | 4 +- microsoft/skills/review/al-privacy-review.md | 4 +- microsoft/skills/review/al-security-review.md | 4 +- microsoft/skills/review/al-style-review.md | 4 +- microsoft/skills/review/al-ui-review.md | 4 +- microsoft/skills/review/al-upgrade-review.md | 4 +- skills/entry.md | 15 ++ skills/read.md | 2 +- tools/Build-KnowledgeIndex.ps1 | 245 ++++++++++++++++++ 13 files changed, 402 insertions(+), 13 deletions(-) create mode 100644 .github/scripts/Test-KnowledgeIndex.ps1 create mode 100644 .github/workflows/knowledge-index.yml create mode 100644 tools/Build-KnowledgeIndex.ps1 diff --git a/.github/scripts/Test-KnowledgeIndex.ps1 b/.github/scripts/Test-KnowledgeIndex.ps1 new file mode 100644 index 0000000..76896af --- /dev/null +++ b/.github/scripts/Test-KnowledgeIndex.ps1 @@ -0,0 +1,93 @@ +<# +.SYNOPSIS + CI guard for the knowledge-index generator (tools/Build-KnowledgeIndex.ps1). + +.DESCRIPTION + BCQuality owns the knowledge index, so BCQuality CI — not each consumer — + proves the generator is healthy. This script does NOT ship a committed + index that consumers trust at runtime (the index is rebuilt over each + consumer's already-pruned clone by Entry's preparation step, which keeps it + exact for any policy). Instead it asserts the generator itself is sound: + + 1. Determinism — building twice yields byte-identical output once the + volatile `generatedAt` header is normalized. + 2. Coverage — every `*/knowledge/**/*.md` article appears exactly once; + every indexed path exists; no duplicates; no article is dropped. + 3. Selection-input integrity — every parsed article row carries the + non-empty `domain` + `keywords` the worklist predicate selects on, and + every article parses (an unparseable article is an invalid file). + + Exit code 0 = healthy; non-zero = a problem CI must block on. +#> +[CmdletBinding()] +param( + [string] $Root = (Resolve-Path (Join-Path $PSScriptRoot '..' '..')) +) + +Set-StrictMode -Version Latest +$ErrorActionPreference = 'Stop' + +# Normalise to an absolute path so Substring-based relative-path derivation +# below matches the absolute FullName the generator emits (CI passes -Root .). +$Root = (Resolve-Path -LiteralPath $Root).Path + +$generator = Join-Path $Root 'tools/Build-KnowledgeIndex.ps1' +if (-not (Test-Path $generator)) { throw "Generator not found: $generator" } + +$tmp = Join-Path ([System.IO.Path]::GetTempPath()) ("kbindex_" + [guid]::NewGuid().ToString('N')) +New-Item -ItemType Directory -Force -Path $tmp | Out-Null +$idxA = Join-Path $tmp 'a.json' +$idxB = Join-Path $tmp 'b.json' + +$problems = [System.Collections.Generic.List[string]]::new() + +& $generator -BCQualityRoot $Root -IndexPath $idxA | Out-Null +& $generator -BCQualityRoot $Root -IndexPath $idxB | Out-Null + +# 1. Determinism (ignoring the volatile generatedAt timestamp). +$norm = { param($p) ((Get-Content -LiteralPath $p -Raw) -replace '"generatedAt":"[^"]*"', '"generatedAt":""') } +if ((& $norm $idxA) -ne (& $norm $idxB)) { + $problems.Add('Non-deterministic: two builds differ beyond generatedAt.') | Out-Null +} + +$index = Get-Content -LiteralPath $idxA -Raw | ConvertFrom-Json +$rows = @($index.articles) + +# 2. Coverage: one row per knowledge .md, every path real, no duplicates. +$onDisk = @( + foreach ($layer in 'microsoft', 'community', 'custom') { + $kb = Join-Path $Root (Join-Path $layer 'knowledge') + if (Test-Path $kb) { + Get-ChildItem -LiteralPath $kb -Recurse -File -Filter '*.md' | + ForEach-Object { ($_.FullName.Substring($Root.Length).TrimStart([char]'/', [char]'\') -replace '\\', '/') } + } + } +) +if ($rows.Count -ne $onDisk.Count) { + $problems.Add("Coverage mismatch: index has $($rows.Count) rows, disk has $($onDisk.Count) knowledge .md files.") | Out-Null +} +$rowPaths = @($rows | ForEach-Object { $_.path }) +$dupes = @($rowPaths | Group-Object | Where-Object Count -gt 1 | ForEach-Object { $_.Name }) +if ($dupes.Count) { $problems.Add("Duplicate index rows: $($dupes -join ', ')") | Out-Null } +$missingOnDisk = @($rowPaths | Where-Object { -not (Test-Path (Join-Path $Root $_)) }) +if ($missingOnDisk.Count) { $problems.Add("Indexed paths absent on disk: $($missingOnDisk -join ', ')") | Out-Null } +$missingInIndex = @($onDisk | Where-Object { $rowPaths -notcontains $_ }) +if ($missingInIndex.Count) { $problems.Add("Articles missing from index: $($missingInIndex -join ', ')") | Out-Null } + +# 3. Selection-input integrity: parsed rows must carry domain + keywords. +$unparsed = @($rows | Where-Object { -not $_.parsed } | ForEach-Object { $_.path }) +if ($unparsed.Count) { $problems.Add("Unparseable (invalid) articles: $($unparsed -join ', ')") | Out-Null } +foreach ($r in $rows | Where-Object { $_.parsed }) { + if ([string]::IsNullOrWhiteSpace([string]$r.domain)) { $problems.Add("Empty domain: $($r.path)") | Out-Null } + if (-not @($r.keywords).Where({ "$_".Trim() }).Count) { $problems.Add("Empty keywords: $($r.path)") | Out-Null } +} + +Remove-Item -Recurse -Force $tmp -ErrorAction SilentlyContinue + +if ($problems.Count) { + Write-Host "Knowledge-index check FAILED ($($problems.Count) problem(s)):" -ForegroundColor Red + $problems | ForEach-Object { Write-Host " - $_" -ForegroundColor Red } + exit 1 +} +Write-Host "Knowledge-index check PASSED: $($rows.Count) articles, deterministic, full coverage, selection inputs intact." -ForegroundColor Green +exit 0 diff --git a/.github/workflows/knowledge-index.yml b/.github/workflows/knowledge-index.yml new file mode 100644 index 0000000..71acfcb --- /dev/null +++ b/.github/workflows/knowledge-index.yml @@ -0,0 +1,24 @@ +name: Validate knowledge index + +# BCQuality owns the knowledge-index generator (tools/Build-KnowledgeIndex.ps1), +# which the AL review skills' Source step consumes. This guards the generator's +# health so consumers never have to: the index a consumer uses at runtime is +# rebuilt over its own pruned clone, but the generator that builds it lives and +# is validated here. + +on: + pull_request: + branches: [main] + push: + branches: [main] + +jobs: + validate-index: + runs-on: ubuntu-latest + steps: + - name: Check out repository + uses: actions/checkout@v4 + + - name: Validate knowledge-index generator + shell: pwsh + run: ./.github/scripts/Test-KnowledgeIndex.ps1 -Root . diff --git a/.gitignore b/.gitignore index f5279e6..1209309 100644 --- a/.gitignore +++ b/.gitignore @@ -23,3 +23,7 @@ node_modules/ # Build artifacts *.log + +# Runtime artifact: the knowledge index is rebuilt over each consumer's +# pruned clone by Entry's preparation step; it is never committed. +/knowledge-index.json diff --git a/agent-consumption.md b/agent-consumption.md index cbeb07f..0d2e5b4 100644 --- a/agent-consumption.md +++ b/agent-consumption.md @@ -52,6 +52,14 @@ Example: a performance review skill sources from `/microsoft/knowledge/performan At this point the agent reads READ and DO on demand — it needs READ to interpret each knowledge file's frontmatter and sections, and DO to shape its output. Those contracts are fetched when first needed, not as part of bootstrap. +### 5a. The knowledge index (Source acceleration) + +Discovering candidates at the Source step naively means opening every file under a domain folder just to read its frontmatter `keywords` — on a large corpus that is hundreds of file reads per review. To avoid this, BCQuality maintains a **knowledge index**: a single artifact (`knowledge-index.json`) that lists every article surviving the consumer's layer/allow-deny filtering and carries, per article, the exact inputs the Source/Worklist steps consume — `path`, `layer`, `domain`, frontmatter dimensions, `keywords`, `title`, and a one-line `description` hint. + +The index is **owned and produced by BCQuality**, not by each consumer: its generator (`tools/Build-KnowledgeIndex.ps1`) ships here, next to the skills and knowledge it derives from, so the index schema stays in lockstep with the Source contract and every consumer gets the same faithful index for free instead of re-implementing the parser. The consuming orchestrator does **not** build or invoke the index — it only prunes its clone to policy as it already does. The index is then (re)generated by BCQuality itself: **Entry's preparation step runs `Build-KnowledgeIndex.ps1` over the live, already-pruned clone** at the start of every run (see `skills/entry.md`), and BCQuality CI (`.github/workflows/knowledge-index.yml`) validates that the generator is healthy and deterministic. Building over the *pruned* clone — rather than shipping a committed full-corpus index that consumers trust — keeps the index exact for any consumer policy: it can never list an article the consumer denied, so policy-excluded rules cannot leak into discovery. + +The index changes only *how candidates are discovered*, never *which are selected*. The Worklist predicate is unchanged — `keywords` still drive selection — and the agent still opens each worklisted article **in full** to read its `## Best Practice` / `## Anti Pattern` rule bodies; the index is discovery metadata only and never substitutes for the article body. When no index is present, skills fall back to path-based discovery (collect by domain folder), so review still works. + ### 6. Agent emits structured output The output contract is defined in the DO meta-skill so that every action skill — today's and next year's — produces the same shape: diff --git a/microsoft/skills/review/al-performance-review.md b/microsoft/skills/review/al-performance-review.md index 33ed759..09bef27 100644 --- a/microsoft/skills/review/al-performance-review.md +++ b/microsoft/skills/review/al-performance-review.md @@ -20,7 +20,7 @@ An orchestrator invokes this skill with either a `pr-diff` (the standard PR-revi ## Source -Collect all knowledge files under `*/knowledge/performance/**/*.md`, across every enabled layer (`/microsoft/`, `/community/`, `/custom/`). Relevance trims the result to the subset that applies. +Read the BCQuality knowledge index once — the `knowledge-index.json` BCQuality builds at the root of the knowledge checkout (Entry's preparation step regenerates it over the live, already-filtered clone — see `skills/entry.md`). It lists every article that survived layer and allow/deny filtering and carries, per article, its `path`, `layer`, `domain`, frontmatter dimensions, `keywords`, `title`, and a one-line `description` hint — exactly the fields Relevance and Worklist consume. Take the index entries whose `domain` is `performance` as this skill's candidate set across every enabled layer; do not open the individual article files at this step. Open an article's full body only once it enters the Worklist below, so a review reads the index plus the handful of worklisted articles instead of every file under `*/knowledge/performance/**`. ## Relevance @@ -41,7 +41,7 @@ Narrow the relevant files to the subset that applies to the changes under review - The changed procedures and triggers, weighted toward those that perform loops, Find/FindSet/FindFirst calls, CalcFields, CalcSums, FlowField access, or cross-table navigation. - Tokens extracted from the diff that relate to data access and hot-path costs (`SetRange`, `SetFilter`, `SetLoadFields`, `SetCurrentKey`, `FindSet`, `ReadIsolation`, `LockTable`, `ModifyAll`, `DeleteAll`, `TextBuilder`, `Dictionary`, `temporary`, `repeat`, `until`, `CalcFields`, `CalcSums`). -A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from filename and Description) matches a changed object type. +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. 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`. diff --git a/microsoft/skills/review/al-privacy-review.md b/microsoft/skills/review/al-privacy-review.md index c0eed13..4ef87e3 100644 --- a/microsoft/skills/review/al-privacy-review.md +++ b/microsoft/skills/review/al-privacy-review.md @@ -20,7 +20,7 @@ An orchestrator invokes this skill with either a `pr-diff` (the standard PR-revi ## Source -Collect all knowledge files under `*/knowledge/privacy/**/*.md`, across every enabled layer (`/microsoft/`, `/community/`, `/custom/`). Relevance trims the result to the subset that applies. +Read the BCQuality knowledge index once — the `knowledge-index.json` BCQuality builds at the root of the knowledge checkout (Entry's preparation step regenerates it over the live, already-filtered clone — see `skills/entry.md`). It lists every article that survived layer and allow/deny filtering and carries, per article, its `path`, `layer`, `domain`, frontmatter dimensions, `keywords`, `title`, and a one-line `description` hint — exactly the fields Relevance and Worklist consume. Take the index entries whose `domain` is `privacy` as this skill's candidate set across every enabled layer; do not open the individual article files at this step. Open an article's full body only once it enters the Worklist below, so a review reads the index plus the handful of worklisted articles instead of every file under `*/knowledge/privacy/**`. ## Relevance @@ -41,7 +41,7 @@ Narrow the relevant files to the subset that applies to the changes under review - The changed procedures and triggers, weighted toward those that call `Error`, `Session.LogMessage`, `StrSubstNo`, `GetLastErrorText`, `FeatureTelemetry.LogUsage`/`LogUptake`/`LogError`, `HttpClient.Post`/`Get`, `IsolatedStorage.Set`/`SetEncrypted`/`Get`, or `PrivacyNotice.GetPrivacyNoticeApprovalState`. - Tokens extracted from the diff that relate to privacy (`DataClassification`, `CustomerContent`, `EndUserIdentifiableInformation`, `EndUserPseudonymousIdentifiers`, `SystemMetadata`, `ToBeClassified`, `PrivacyNotice`, `GetLastErrorText`, `TelemetryScope`, `FeatureTelemetry`, `CustomDimensions`, `LogUsage`, `LogUptake`, `LogError`, `HybridSL`, `HybridGP`, `HybridBC`). -A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from filename and Description) matches a changed object type. +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. 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`. diff --git a/microsoft/skills/review/al-security-review.md b/microsoft/skills/review/al-security-review.md index a8ced83..1e72447 100644 --- a/microsoft/skills/review/al-security-review.md +++ b/microsoft/skills/review/al-security-review.md @@ -20,7 +20,7 @@ An orchestrator invokes this skill with either a `pr-diff` (the standard PR-revi ## Source -Collect all knowledge files under `*/knowledge/security/**/*.md`, across every enabled layer (`/microsoft/`, `/community/`, `/custom/`). Relevance trims the result to the subset that applies. +Read the BCQuality knowledge index once — the `knowledge-index.json` BCQuality builds at the root of the knowledge checkout (Entry's preparation step regenerates it over the live, already-filtered clone — see `skills/entry.md`). It lists every article that survived layer and allow/deny filtering and carries, per article, its `path`, `layer`, `domain`, frontmatter dimensions, `keywords`, `title`, and a one-line `description` hint — exactly the fields Relevance and Worklist consume. Take the index entries whose `domain` is `security` as this skill's candidate set across every enabled layer; do not open the individual article files at this step. Open an article's full body only once it enters the Worklist below, so a review reads the index plus the handful of worklisted articles instead of every file under `*/knowledge/security/**`. ## Relevance @@ -41,7 +41,7 @@ Narrow the relevant files to the subset that applies to the changes under review - The changed procedures and triggers, weighted toward those that call `HttpClient`, validate or compose URLs, write to telemetry, read or write secrets, unwrap SecretText, manipulate record-level security, expose var Boolean guard parameters, or bypass the permission model (for example, `RecordRef.Open`, `Record.WritePermission`, direct table access from a non-owning app). - Tokens extracted from the diff that relate to security concerns (`IsolatedStorage`, `SetEncrypted`, `OAuth2`, `SecretText`, `Unwrap`, `NonDebuggable`, `Password`, `Token`, `HttpClient`, `Uri`, `AreURIsHaveSameHost`, `IsValidURIPattern`, `RecordRef`, `RecordId`, `Open`, `IntegrationEvent`, `SkipValidation`, `HasAccess`, `Permission`, `UserSecurityId`, `Commit`). -A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from filename and Description) matches a changed object type. +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. 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`. diff --git a/microsoft/skills/review/al-style-review.md b/microsoft/skills/review/al-style-review.md index 752dc14..d926df3 100644 --- a/microsoft/skills/review/al-style-review.md +++ b/microsoft/skills/review/al-style-review.md @@ -22,7 +22,7 @@ An orchestrator invokes this skill with either a `pr-diff` or a `file-path`. The ## Source -Collect all knowledge files under `*/knowledge/style/**/*.md`, across every enabled layer. +Read the BCQuality knowledge index once — the `knowledge-index.json` BCQuality builds at the root of the knowledge checkout (Entry's preparation step regenerates it over the live, already-filtered clone — see `skills/entry.md`). It lists every article that survived layer and allow/deny filtering and carries, per article, its `path`, `layer`, `domain`, frontmatter dimensions, `keywords`, `title`, and a one-line `description` hint — exactly the fields Relevance and Worklist consume. Take the index entries whose `domain` is `style` as this skill's candidate set across every enabled layer; do not open the individual article files at this step. Open an article's full body only once it enters the Worklist below, so a review reads the index plus the handful of worklisted articles instead of every file under `*/knowledge/style/**`. ## Relevance @@ -43,7 +43,7 @@ Narrow the relevant files to the subset that applies to the changes under review - Changed declarations, weighted toward `: Label '...'`, `: TextConst '...'`, temporary record variables, option fields, error-handling call sites, and codeunit-internal method calls. - Tokens extracted from the diff (`Label`, `TextConst`, `Locked`, `Comment`, `MaxLength`, `temporary`, `OptionMembers`, `OptionCaption`, `APIPublisher`, `APIGroup`, `APIVersion`, `EntityName`, `EntitySetName`, `DelayedInsert`, `FieldCaption`, `TableCaption`, `FieldName`, `TableName`, `Page.RunModal`, `Report.Run`, `this.`, `StrSubstNo`). -A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic matches a changed object or declaration. +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 or declaration. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone. Once the candidate worklist is known, resolve layer-precedence conflicts per READ and record suppressions. diff --git a/microsoft/skills/review/al-ui-review.md b/microsoft/skills/review/al-ui-review.md index 1511fd0..ef2e94d 100644 --- a/microsoft/skills/review/al-ui-review.md +++ b/microsoft/skills/review/al-ui-review.md @@ -22,7 +22,7 @@ An orchestrator invokes this skill with either a `pr-diff` or a `file-path`. The ## Source -Collect all knowledge files under `*/knowledge/ui/**/*.md`, across every enabled layer. +Read the BCQuality knowledge index once — the `knowledge-index.json` BCQuality builds at the root of the knowledge checkout (Entry's preparation step regenerates it over the live, already-filtered clone — see `skills/entry.md`). It lists every article that survived layer and allow/deny filtering and carries, per article, its `path`, `layer`, `domain`, frontmatter dimensions, `keywords`, `title`, and a one-line `description` hint — exactly the fields Relevance and Worklist consume. Take the index entries whose `domain` is `ui` as this skill's candidate set across every enabled layer; do not open the individual article files at this step. Open an article's full body only once it enters the Worklist below, so a review reads the index plus the handful of worklisted articles instead of every file under `*/knowledge/ui/**`. ## Relevance @@ -43,7 +43,7 @@ Narrow the relevant files to the subset that applies to the changes under review - For each relevant knowledge file, compute overlap against changed page declarations and control add-in UI files, weighted toward `Caption`, `ToolTip`, `AboutTitle`, `AboutText`, `OptionCaption`, `ShowCaption`, `InstructionalText`, `GridLayout`, `Style`, `StyleExpr`, action definitions, field-level properties, DOM creation, ARIA attributes, and keyboard/focus handlers. - Tokens extracted from the diff (`Caption`, `ToolTip`, `AboutTitle`, `AboutText`, `PageType`, `ShowCaption`, `InstructionalText`, `grid`, `fixed`, `GridLayout`, `Style`, `StyleExpr`, `Favorable`, `Unfavorable`, `Ambiguous`, `cuegroup`, `controladdin`, `usercontrol`, `aria-`, `tabindex`, `keydown`, `focus`, `innerHTML`, `createElement`, `&`, `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 matches a changed page element. +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. Once the candidate worklist is known, resolve layer-precedence conflicts per READ and record suppressions. diff --git a/microsoft/skills/review/al-upgrade-review.md b/microsoft/skills/review/al-upgrade-review.md index 76083b6..7ccfa4a 100644 --- a/microsoft/skills/review/al-upgrade-review.md +++ b/microsoft/skills/review/al-upgrade-review.md @@ -20,7 +20,7 @@ An orchestrator invokes this skill with either a `pr-diff` (the standard PR-revi ## Source -Collect all knowledge files under `*/knowledge/upgrade/**/*.md`, across every enabled layer (`/microsoft/`, `/community/`, `/custom/`). Relevance trims the result to the subset that applies. +Read the BCQuality knowledge index once — the `knowledge-index.json` BCQuality builds at the root of the knowledge checkout (Entry's preparation step regenerates it over the live, already-filtered clone — see `skills/entry.md`). It lists every article that survived layer and allow/deny filtering and carries, per article, its `path`, `layer`, `domain`, frontmatter dimensions, `keywords`, `title`, and a one-line `description` hint — exactly the fields Relevance and Worklist consume. Take the index entries whose `domain` is `upgrade` as this skill's candidate set across every enabled layer; do not open the individual article files at this step. Open an article's full body only once it enters the Worklist below, so a review reads the index plus the handful of worklisted articles instead of every file under `*/knowledge/upgrade/**`. ## Relevance @@ -41,7 +41,7 @@ Narrow the relevant files to the subset that applies to the changes under review - The changed triggers and procedures, weighted toward `OnUpgradePerCompany`, `OnUpgradePerDatabase`, `OnValidateUpgradePerCompany`, `OnValidateUpgradePerDatabase`, `OnInstallAppPerCompany`, and the `OnGetPerCompanyUpgradeTags`/`OnGetPerDatabaseUpgradeTags` subscribers. - Tokens extracted from the diff that relate to upgrade concerns (`Subtype = Upgrade`, `Upgrade Tag`, `HasUpgradeTag`, `SetUpgradeTag`, `OnValidateUpgrade`, `DataTransfer`, `CopyFields`, `InitValue`, `ObsoleteState`, `ObsoleteReason`, `ObsoleteTag`, `DataVersion`, `ExecutionContext`, `PrimaryKey`, `key(`, `field(`, `value(`, `enum`, `enumextension`, `HybridSL`, `HybridGP`, `HybridBC`, `HybridBaseDeployment`). -A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic matches a changed object type. When the diff contains no upgrade-related changes by any of the above signals, return `outcome: "not-applicable"` without evaluating files. +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 upgrade-related changes by any of the above signals, return `outcome: "not-applicable"` without evaluating files. Once the candidate worklist is known, resolve layer-precedence conflicts per READ. Drop lower-precedence files whose normative guidance directly contradicts a higher-precedence candidate, and record each dropped file in `suppressed` with `reason: "layer-precedence"`. Files suppressed by configuration are recorded with `reason: "configuration"`. diff --git a/skills/entry.md b/skills/entry.md index d63ca2e..f094aa6 100644 --- a/skills/entry.md +++ b/skills/entry.md @@ -33,6 +33,21 @@ task-context: `goal` and `inputs-available` are required. Filter dimensions (`technologies`, `bc-version`, `countries`, `application-area`) are optional; omitting a dimension is equivalent to "unconstrained" — see Relevance for the exact matching rule. `enabled-layers` defaults to all three. `disabled-skills` defaults to empty. +## Preparation — knowledge index + +Before routing, ensure the knowledge index is current for the **live** clone. The dispatched review skills read `knowledge-index.json` (at the clone root) at their Source step instead of opening every knowledge file — see READ's [Retrieval workflow](read.md). Because a consumer prunes its clone to policy *before* the agent runs, the index MUST be built over the clone as it exists now, so it lists exactly the articles that survived pruning and never an article the consumer denied: + +- If `knowledge-index.json` is absent — or you cannot confirm it reflects the current knowledge tree — regenerate it by running, from the checkout root: + + ``` + pwsh ./tools/Build-KnowledgeIndex.ps1 + ``` + + It defaults to indexing this checkout and writes `knowledge-index.json` at the root in well under a second. When in doubt, rebuild: a sub-second rebuild is always cheaper than a stale or over-listing index, which is a correctness risk. +- This is a side step. It MUST NOT change Entry's output — the dispatch record below is the only thing Entry emits, and build logs are never part of the dispatch JSON. + +Generation is **owned by BCQuality**: the generator ships here next to the skills and knowledge it derives from, and the consuming orchestrator neither builds nor knows about the index. + ## Source All action skills under `*/skills/**/*.md` across the layers named in `enabled-layers`. Meta-skills in `/skills/` (including this file) are not candidates and MUST be excluded. Entry never dispatches Entry. diff --git a/skills/read.md b/skills/read.md index e071d1d..dbeaa63 100644 --- a/skills/read.md +++ b/skills/read.md @@ -140,7 +140,7 @@ Consumers that surface sample code to an end user or agent SHOULD cite the sampl The standard workflow for finding applicable files: -1. Collect candidates by path (typically by `domain` subfolder, across enabled layers). +1. Collect candidates from the knowledge index (`knowledge-index.json`). BCQuality maintains it: Entry's preparation step (see [entry.md](entry.md)) regenerates it over the live, already-filtered clone, so it lists exactly the articles that survived the consumer's layer/allow-deny pruning, each with the frontmatter, `keywords`, `title`, and one-line `description` that steps 2-3 need — candidates are enumerated without opening each file. The index is **discovery metadata only**: it tells you *which* files to open, it does not substitute for them. A finding MUST cite only an article that was opened and read in full; an index row whose file is absent from the clone MUST be discarded *before* ranking or worklisting, and its metadata MUST NOT seed a finding. Absent an index, collect candidates by path (typically by `domain` subfolder, across enabled layers). 2. Filter by frontmatter using the matching rules above. Files that are not applicable are discarded. 3. Rank or narrow by `keywords` relevance to the task. 4. Resolve conflicts via layer precedence. diff --git a/tools/Build-KnowledgeIndex.ps1 b/tools/Build-KnowledgeIndex.ps1 new file mode 100644 index 0000000..5835e5d --- /dev/null +++ b/tools/Build-KnowledgeIndex.ps1 @@ -0,0 +1,245 @@ +<# +.SYNOPSIS + Builds the BCQuality knowledge index — the discovery artifact the review + skills' §Source step consumes. + +.DESCRIPTION + BCQuality owns the knowledge-index contract: the index schema is part of + the Source step that the action skills (e.g. al-*-review.md) declare, so + the generator lives here, next to the skills and knowledge it derives from. + Consumers (orchestrators such as the BCAppsBCQuality PR-review filter) call + this script instead of re-implementing the parser, so every consumer gets + the same faithful index for free and the index stays in lockstep with the + skill contract. + + The index lets the agent enumerate candidate articles and compute the + keyword/topic worklist overlap by reading ONE file, instead of opening + every file under `*/knowledge//**` just to read its frontmatter. + The worklist SELECTION predicate is unchanged: the index carries the same + inputs the predicate already reads (keywords + frontmatter dimensions + + domain + path + title), and the agent still opens each worklisted article + in full for its `## Best Practice` / `## Anti Pattern` rule bodies. + + The index is LEAN by default: the verbatim Description is trimmed to a + one-line hint and the JSON is emitted compact, so the index prefix the + agent replays across passes stays small. The selection inputs remain + lossless. Pass -FullIndex for the verbatim, pretty-printed variant. + + This script does NOT apply layer/allow-deny policy by reading config — it + indexes whatever knowledge files are present on disk (a consumer is + expected to prune its clone to policy first). For provenance and to + reproduce a consumer's exact view, pass -EnabledLayers to restrict the walk + to those layers and to record the policy in the index header. + +.PARAMETER BCQualityRoot + Path to the BCQuality content root to index (typically a filtered clone). + Defaults to the clone root (the parent of this script's `tools/` folder), so + the agent can run `pwsh ./tools/Build-KnowledgeIndex.ps1` from the clone root + with no arguments. + +.PARAMETER IndexPath + Where to write the index JSON. Defaults to `/knowledge-index.json`. + +.PARAMETER EnabledLayers + Optional layer allowlist (e.g. microsoft, community, custom). When provided, + only those layers are walked and the value is recorded in the index header. + Omit to index every layer present on disk. + +.PARAMETER KnowledgeAllow + Optional allow globs to record in the index header (provenance only). + +.PARAMETER KnowledgeDeny + Optional deny globs to record in the index header (provenance only). + +.PARAMETER FullIndex + Emit the verbatim-Description, pretty-printed index instead of the lean one. + +.OUTPUTS + Returns the number of articles written to the index. +#> +[CmdletBinding()] +param( + [string] $BCQualityRoot, + [string] $IndexPath, + [string[]] $EnabledLayers, + [string[]] $KnowledgeAllow = @(), + [string[]] $KnowledgeDeny = @(), + [switch] $FullIndex +) + +Set-StrictMode -Version Latest +$ErrorActionPreference = 'Stop' + +# Default to the clone root (parent of this script's tools/ folder) so the +# agent's Entry preparation step can invoke this with no arguments from the +# checkout root. A consumer/orchestrator may still pass -BCQualityRoot. +if (-not $BCQualityRoot) { + $BCQualityRoot = (Resolve-Path (Join-Path $PSScriptRoot '..')).Path +} +if (-not (Test-Path $BCQualityRoot)) { + throw "BCQuality root not found: $BCQualityRoot" +} +# Normalise to an absolute path: Get-ChildItem.FullName below is always +# absolute, so Get-RelativePath's Substring needs an absolute root to strip. +# A relative root (e.g. '.') would otherwise leave the full path intact. +$BCQualityRoot = (Resolve-Path -LiteralPath $BCQualityRoot).Path +if (-not $IndexPath) { + $IndexPath = Join-Path $BCQualityRoot 'knowledge-index.json' +} + +function Get-RelativePath { + param([string] $Root, [string] $Full) + $rel = $Full.Substring($Root.Length).TrimStart([char]'/', [char]'\') + return ($rel -replace '\\', '/') +} + +# Trims a Description to a single short line (<= $Max chars) for the lean +# index. Takes the first sentence; truncates on a word boundary if still long. +function Get-LeanDescription { + param([string] $Text, [int] $Max = 120) + if ([string]::IsNullOrWhiteSpace($Text)) { return '' } + $t = ($Text -replace '\s+', ' ').Trim() + $m = [regex]::Match($t, '^(.*?[\.!?])(\s|$)') + if ($m.Success) { $t = $m.Groups[1].Value.Trim() } + if ($t.Length -le $Max) { return $t } + $cut = $t.Substring(0, $Max) + $sp = $cut.LastIndexOf(' ') + if ($sp -gt 40) { $cut = $cut.Substring(0, $sp) } + return ($cut.TrimEnd() + '…') +} + +function ConvertFrom-ArticleFrontmatter { + # Parses a knowledge file into the fields the knowledge index needs. + # Captures the verbatim frontmatter dimensions/keywords, the H1 title, + # and the full Description section (the article's primary retrieval + # target per READ). No rule-body content (## Best Practice / ## Anti + # Pattern) is included; the index is a lossless substitute for the + # frontmatter + Description the worklist predicate reads, not a + # substitute for the article's normative guidance. + param([string] $Path) + + $lines = Get-Content -LiteralPath $Path -ErrorAction Stop + + # Frontmatter is the first '---'-delimited block. + if ($lines.Count -lt 1 -or $lines[0].Trim() -ne '---') { return $null } + $fmEnd = -1 + for ($i = 1; $i -lt $lines.Count; $i++) { + if ($lines[$i].Trim() -eq '---') { $fmEnd = $i; break } + } + if ($fmEnd -lt 0) { return $null } + + $fm = @{} + for ($i = 1; $i -lt $fmEnd; $i++) { + $line = $lines[$i] + if ($line -match '^\s*([a-zA-Z][\w-]*)\s*:\s*(.*)$') { + $key = $Matches[1] + $val = $Matches[2].Trim() + if ($val -match '^\[(.*)\]$') { + $inner = $Matches[1].Trim() + if ($inner -eq '') { $fm[$key] = @() } + else { $fm[$key] = @($inner -split '\s*,\s*' | ForEach-Object { $_.Trim() }) } + } + elseif ($val -ne '') { $fm[$key] = $val } + } + } + + # Body parsing: H1 title and the full Description section. The Description + # is the article's primary retrieval target per READ and is captured + # verbatim (it carries no rule-body guidance and no fenced code per the + # schema), so the index is a lossless substitute for the frontmatter + + # Description that the worklist predicate reads. Normative rule bodies + # (## Best Practice / ## Anti Pattern) are deliberately NOT included. + $title = '' + $description = '' + $inDescription = $false + $descBuffer = [System.Collections.Generic.List[string]]::new() + for ($i = $fmEnd + 1; $i -lt $lines.Count; $i++) { + $line = $lines[$i] + if (-not $title -and $line -match '^\#\s+(.+?)\s*$') { $title = $Matches[1].Trim(); continue } + if ($line -match '^\#\#\s+Description\s*$') { $inDescription = $true; continue } + if ($inDescription) { + if ($line -match '^\#\#\s') { break } # next section ends Description + if ($line.Trim() -ne '') { $descBuffer.Add($line.Trim()) | Out-Null } + } + } + if ($descBuffer.Count -gt 0) { $description = ($descBuffer -join ' ').Trim() } + + return [pscustomobject]@{ + domain = if ($fm.ContainsKey('domain')) { [string]$fm['domain'] } else { '' } + 'bc-version' = @($fm['bc-version']) + technologies = @($fm['technologies']) + countries = @($fm['countries']) + 'application-area'= @($fm['application-area']) + keywords = @($fm['keywords']) + title = $title + description = $description + } +} + +# Walk the knowledge files and emit a single compact discovery artifact so +# consumers can enumerate candidate articles and compute keyword/topic worklist +# overlap without opening every file. When -EnabledLayers is supplied the walk +# is restricted to those layers (a consumer reproduces its filtered view); the +# article set otherwise reflects whatever is present on disk. +$indexArticles = [System.Collections.Generic.List[object]]::new() +foreach ($layerDir in @('microsoft', 'community', 'custom')) { + $kbRoot = Join-Path $BCQualityRoot (Join-Path $layerDir 'knowledge') + if (-not (Test-Path $kbRoot)) { continue } + if ($EnabledLayers -and ($EnabledLayers -notcontains $layerDir)) { continue } + + Get-ChildItem -LiteralPath $kbRoot -Recurse -File -Filter '*.md' -ErrorAction SilentlyContinue | + Sort-Object FullName | + ForEach-Object { + $rel = Get-RelativePath -Root $BCQualityRoot -Full $_.FullName + $parsed = $null + try { $parsed = ConvertFrom-ArticleFrontmatter -Path $_.FullName } catch { $parsed = $null } + if (-not $parsed) { + # Invalid/unparseable file: list path + domain-from-path so it + # is never silently dropped from discovery. Consumers fall back + # to reading it in full. + $domainFromPath = if ($rel -match '/knowledge/([^/]+)/') { $Matches[1] } else { '' } + $indexArticles.Add([pscustomobject]@{ + path = $rel; layer = $layerDir; domain = $domainFromPath + 'bc-version' = @(); technologies = @(); countries = @(); 'application-area' = @() + keywords = @(); title = ''; description = ''; parsed = $false + }) | Out-Null + return + } + $indexArticles.Add([pscustomobject]@{ + path = $rel + layer = $layerDir + domain = $parsed.domain + 'bc-version' = @($parsed.'bc-version') + technologies = @($parsed.technologies) + countries = @($parsed.countries) + 'application-area' = @($parsed.'application-area') + keywords = @($parsed.keywords) + title = $parsed.title + description = if ($FullIndex) { $parsed.description } else { Get-LeanDescription -Text $parsed.description } + parsed = $true + }) | Out-Null + } +} + +$index = [pscustomobject]@{ + version = 1 + generatedAt = (Get-Date).ToUniversalTime().ToString('o') + enabledLayers = @($EnabledLayers) + knowledgeAllow= @($KnowledgeAllow) + knowledgeDeny = @($KnowledgeDeny) + articleCount = $indexArticles.Count + articles = @($indexArticles) +} + +$indexDir = Split-Path -Parent $IndexPath +if ($indexDir -and -not (Test-Path $indexDir)) { + New-Item -ItemType Directory -Force -Path $indexDir | Out-Null +} +if ($FullIndex) { + $index | ConvertTo-Json -Depth 8 | Set-Content -LiteralPath $IndexPath -Encoding UTF8 +} else { + $index | ConvertTo-Json -Depth 8 -Compress | Set-Content -LiteralPath $IndexPath -Encoding UTF8 +} + +Write-Host "BCQuality index: $($indexArticles.Count) article(s). Index: $IndexPath" +return $indexArticles.Count