TCOG

custom/knowledge/performance/pure-reads-set-readisolation.md

@@ -0,0 +1,22 @@
+---
+bc-version: [all]
+domain: performance
+keywords: [readisolation, readuncommitted, pure-read, get, findset, findfirst, findlast]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Set ReadIsolation explicitly on pure reads
+
+## Description
+
+In tCOG apps, pure reads should set an explicit `ReadIsolation` on the record instance before the read operation. This makes the read semantics intentional instead of inheriting whatever isolation level happens to be active in the surrounding transaction or caller. For display, lookup, and calculation helpers that only read data and do not make decisions that require committed consistency, the default convention is `IsolationLevel::ReadUncommitted`.
+
+## Best Practice
+
+Before a pure read such as `Get`, `FindSet`, `FindFirst`, `FindLast`, or a read-only existence or aggregation call, set `Rec.ReadIsolation(IsolationLevel::ReadUncommitted);` on the specific record instance, then perform the read. Use a stronger isolation level only when the procedure's functional contract requires committed or repeatable data.
+
+## Anti Pattern
+
+A read-only helper performs `Get`, `FindSet`, or another pure read without setting `ReadIsolation` on the record variable first. The code works, but the locking and consistency behavior is left implicit and can drift with surrounding transaction context. On hot paths such as page calculations, lookups, and repeated helper calls, this creates inconsistent conventions and makes concurrency behavior harder to reason about.

custom/skills/review/al-code-review.md

@@ -0,0 +1,73 @@
+---
+kind: action-skill
+id: al-code-review
+version: 1
+title: AL code review
+description: Reviews AL source changes by composing the AL review leaf skills, using tCOG-specific performance guidance where available.
+inputs: [pr-diff, file-path]
+outputs: [findings-report]
+bc-version: [all]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+sub-skills:
+  - custom/skills/review/al-performance-review.md
+  - microsoft/skills/review/al-security-review.md
+  - microsoft/skills/review/al-privacy-review.md
+  - microsoft/skills/review/al-upgrade-review.md
+  - microsoft/skills/review/al-style-review.md
+  - microsoft/skills/review/al-ui-review.md
+  - microsoft/skills/review/al-error-handling-review.md
+  - microsoft/skills/review/al-events-review.md
+  - microsoft/skills/review/al-interfaces-review.md
+  - microsoft/skills/review/al-breaking-changes-review.md
+  - microsoft/skills/review/al-web-services-review.md
+---
+
+# AL code review
+
+Reviews AL source changes by composing the AL review leaf skills. This custom-layer variant preserves the standard BCQuality multi-domain review flow, but replaces the performance leaf with the tCOG custom performance review so local conventions participate in the same code-review route.
+
+## Source
+
+The sub-skills invoked by this skill are exactly those listed in frontmatter `sub-skills`. The skill does not discover sub-skills implicitly.
+
+## Relevance
+
+A sub-skill is relevant when both of the following hold:
+
+- The orchestrator supplied inputs that satisfy the sub-skill's declared `inputs`.
+- The orchestrator did not disable the sub-skill via configuration.
+
+Per the DO contract, this super-skill must not filter sub-skills by diff content. Each leaf is responsible for deciding whether the task is applicable inside its own execution.
+
+Sub-skills that fail either check are not invoked and are recorded in `skipped-sub-skills` with reason `configuration` or `not-applicable`.
+
+## Worklist
+
+The worklist is the list of sub-skills judged relevant by the previous step. Every sub-skill in the worklist is invoked in the Action step.
+
+## Action
+
+Execute the worklist as discrete iterations, one sub-skill at a time.
+
+For each sub-skill:
+
+1. Invoke the sub-skill with the orchestrator-supplied inputs, passing only the subset declared by that sub-skill.
+2. Capture the sub-skill's full findings-report verbatim in `sub-results`.
+3. If the sub-skill outcome is `failed`, keep its report in `sub-results` but do not roll its findings up into the top-level result.
+4. Otherwise, append each finding from the sub-skill's `findings[]` to the super-skill's top-level `findings[]`, setting `from-sub-skill` to the producing sub-skill's `skill.id`. If the rolled-up finding uses a slug `id` rather than a reference path, prefix it with `<from-sub-skill>:` to avoid collisions.
+
+After every sub-skill completes, perform one self-review pass over the same diff to look for concrete cross-cutting concerns that no single leaf could surface on its own. Validate each candidate against the BCQuality knowledge already loaded by the leaves:
+
+- If a loaded knowledge file matches the concern, upgrade it to a knowledge-backed finding and attribute it to the owning sub-skill.
+- If loaded knowledge explicitly contradicts the concern, suppress it.
+- Otherwise, emit it as an agent finding with `references: []`, an `id` prefixed with `agent:`, `confidence` no higher than `medium`, and `severity` no higher than `minor`.
+
+Populate `suggested-code` whenever the fix is small, local, and mechanical.
+
+Aggregate summary counts and coverage across invoked sub-skills whose outcome is not `failed`. Agent findings produced by the super-skill contribute to counts but not to coverage.
+
+## Output
+
+Output conforms to the DO findings-report contract in [skills/do.md](skills/do.md), including `sub-results` and `skipped-sub-skills` for this super-skill.

custom/skills/review/al-performance-review.md

@@ -0,0 +1,60 @@
+---
+kind: action-skill
+id: al-performance-review
+version: 1
+title: AL performance review
+description: Reviews AL source changes against performance guidance from BCQuality, including tCOG-specific conventions.
+inputs: [pr-diff, file-path]
+outputs: [findings-report]
+bc-version: [all]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# AL performance review
+
+Reviews AL source changes against the `performance` knowledge domain in BCQuality and emits a findings report. This custom layer variant keeps the standard BCQuality performance review behavior while allowing tCOG-specific performance knowledge to participate through the same worklist.
+
+## Source
+
+Read the BCQuality knowledge index once and take the entries whose `domain` is `performance` across every enabled layer. Do not open individual knowledge files at this step.
+
+## Relevance
+
+Apply the frontmatter matching rules from READ against the task context:
+
+- `bc-version` from the target app version or orchestrator context.
+- `technologies` must include `al`.
+- `countries` from the consuming app context.
+- `application-area` from the changed objects.
+
+Discard files that explicitly do not match. Retain conditionally applicable files only when consumer configuration allows them; any findings derived from unknown dimensions must cap confidence at `medium` and name the unknown dimensions in the message.
+
+## Worklist
+
+Narrow the relevant files to the subset that applies to the changes under review. Weight heavily toward changed procedures and triggers that perform loops, reads, partial-record optimization, and concurrency-sensitive access patterns.
+
+Key tokens include `SetLoadFields`, `FindSet`, `FindFirst`, `FindLast`, `Get`, `ReadIsolation`, `LockTable`, `ReadCommitted`, `ReadUncommitted`, `repeat`, `until`, `CalcFields`, and `CalcSums`.
+
+A knowledge file enters the worklist when its keywords intersect the extracted tokens or its topic matches the changed object type or code path. After the candidate worklist is known, resolve layer precedence per READ and record any suppressed lower-precedence files.
+
+When no applicable performance knowledge survives filtering and precedence, emit `outcome: "no-knowledge"`. When relevant performance knowledge exists but nothing matches the reviewed changes, emit `outcome: "completed"` with an empty `findings` array.
+
+## Action
+
+For each worklist entry, evaluate the diff against the file's `## Best Practice` and `## Anti Pattern` sections.
+
+- Emit `major` or `blocker` for clear anti-pattern matches, using `blocker` only when the referenced guidance states a platform-level guarantee is violated.
+- Emit `minor` when the code contradicts a best practice without being a full anti-pattern.
+- Emit `info` when a file is clearly applicable but no concrete violation is detectable.
+
+Set confidence to `high` for unambiguous syntax or identifier matches, `medium` for heuristic matches or unknown frontmatter dimensions, and `low` for advisory applicability-only observations.
+
+The skill may also emit conservative performance-domain agent findings when the diff shows a concrete, material performance defect that no loaded knowledge file covers. Agent findings must use `references: []`, an `id` prefixed with `agent:`, `severity` no higher than `minor`, `confidence` no higher than `medium`, and a self-contained message with a concrete recommendation.
+
+Populate `suggested-code` whenever the fix is small, local, and mechanical. Omit it only when the right replacement depends on unavailable context, multiple defensible choices exist, or the fix spans non-contiguous code.
+
+## Output
+
+Output conforms to the DO findings-report contract in [skills/do.md](skills/do.md).