al-testing-review.md's cue presented GenerateRandomCodeWithLength and
GenerateRandomCode20 as interchangeable options for "a shorter field
needing real verified uniqueness." They aren't: verified against
LibraryUtility.Codeunit.al, GenerateRandomCode20 truncates
GenerateGUID()'s sequential value down to the target field's length by
keeping the leftmost (slowest-changing) characters via PadStr, so its
retry loop against a field shorter than 20 can churn through the same
truncated prefix for a long time. GenerateRandomCodeWithLength has no
such problem (it generates exactly the requested length of random
text). The knowledge article itself already scoped GenerateRandomCode20
to Code[20] correctly - only the skill cue needed narrowing to match.
- use-generateguid-for-unique-test-fixture-values.md: narrowed the
collision rule to primary-key/unique-lookup fields - an ordinary
descriptive field carries no uniqueness constraint, so a hardcoded
or deterministic value there isn't the anti-pattern (the article and
its al-testing-review.md worklist cue both said "primary-key or
descriptive field"). Also corrected GenerateRandomCode: it opens its
target table as a temporary RecordRef that starts and stays empty,
so its repeat/until loop always exits after one iteration and never
retries even within a single test run - the "non-colliding within a
test run" claim was false. It's the rightmost N characters of
GenerateGUID()'s sequential series, so a short field's value cycles
(Code[1] repeats every 10 calls, Code[2] every 100). Verified against
LibraryUtility.Codeunit.al in the BCApps reference clone.
- item-ledger-entry-document-no-follows-last-shipping-no: both
fixtures called FindSet() without consuming its optional Boolean,
which raises a runtime error on an empty result set - the opposite
of the article's own claimed "silently matches zero rows, no error"
behavior. Wrapped in `if ... then;` per the existing
guard-database-reads.good.al idiom.
- al-data-modeling-review.md: widened both not-applicable scope
clauses (intro and outcome) to include dimension wiring, posting-
routine structure, and Item Ledger Entry document-number lookups -
the leaf declared itself not-applicable outside setup/master/key/
numbering/block/audit surfaces despite having a targeted cue for
this PR's own new article.
- Converted this PR's 8 plain-backtick "See sample: `x.good.al`."
references (across all 4 new articles) to the READ-convention
markdown-link form required by Knowledge-Retrieval.ps1.
Rebased onto upstream/main (one conflict in al-data-modeling-review.md
intro wording, merged).
- use-generateguid-for-unique-test-fixture-values.md/.good.al: documented
each LibraryUtility helper's actual behavior, verified against
LibraryUtility.Codeunit.al. GenerateRandomCode opens the target table as
a temporary RecordRef, so despite taking TableNo it never checks real
data. GenerateRandomXMLText performs no table lookup at all. Only
GenerateRandomCodeWithLength/GenerateRandomCode20 (capped at Code[10]/
Code[20]) genuinely verify against the real table. Fixture switched to
GenerateRandomCodeWithLength where the comment claims verified
uniqueness.
- al-testing-review.md: rewired the cue to catch the actual anti-pattern
(hardcoded literals, hand-built uniqueness, short-field GUID truncation)
instead of only matching the compliant GenerateGUID()+CopyStr shape;
broadened tokens to include TestPage, Library - Utility, and
.Visible()/.Enabled()/.Editable().
- al-data-modeling-review.md: restricted the Item Ledger Entry
Last-Shipping-No. cue to sales combined posting; purchase combined
posting is Receive+Invoice and uses different fields entirely.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- use-generateguid-for-unique-test-fixture-values.md: GenerateGUID()
is a Code[10] number-series value, not a real GUID; truncating it
with CopyStr for a shorter field cuts off the changing digits. Point
to GenerateRandomCode/GenerateRandomCodeWithLength/GenerateRandomXMLText
instead, which verify uniqueness against the actual table.
- Split use-testpage-visible-enabled-to-verify-field-ui-state.md: drop
its editability claim (the sample opens with OpenView() and asserts
Enabled(), which verifies enabled state, not editability — Editable()
and Enabled() are distinct TestField methods). New companion article
use-testpage-editable-to-verify-field-editability.md covers Editable()
with OpenEdit() specifically.
- Wire GenerateGUID/CopyStr and TestPage Visible/Enabled/Editable cues
into al-testing-review.md, and the Item Ledger Entry/Last Shipping No.
posting cue into al-data-modeling-review.md.
The Item Ledger Entry article itself was independently verified against
current BCApps source and needs no changes.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Add bounded knowledge retrieval
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0d8b7764-f15a-49ea-8d50-d9147334f8af
* Fix bc-version overflow and pathless-row identity in bounded retrieval
Catalog matching compared an Int32 -BCVersion against a bigint range bound.
PowerShell coerces the right operand to the left operand's type, so a bound
wider than Int32 threw a conversion error and failed the whole domain catalog
rather than the single row. Metadata validation already accepts such bounds,
so compare as bigint on both sides.
The shared pager built its oversized-row message with $row.path, which
throws under Set-StrictMode -Version Latest when a row carries no path,
replacing the explicit bound failure with a property-lookup error. Resolve the
path defensively for dictionary and object rows so the offset-based fallback
is reachable.
Both paths gain regression coverage that fails without these fixes.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0d8b7764-f15a-49ea-8d50-d9147334f8af
* Document standalone review runner contract
Keep model selection and scheduling outside BCQuality while allowing orchestrators to run isolated review leaves concurrently with deterministic rollup semantics.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Support complete app folder reviews
Define folder-path as a current-state review scope and accept it across the standalone adapter, broad coordinator, and every AL review leaf.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Add walk-up app review quick start
Put the complete app-folder installation and prompt flow directly in the README so partners can discover the standalone experience without reading integration details first.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Organize conceptual guides under docs
Move architecture and standalone runner documentation out of the repository root, add a documentation index, and update all inbound links.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The narrowed UI-handler guidance still stated the execution rule without the
qualifier the linked Microsoft reference uses. The article said every listed
handler must execute at least once, and the testing leaf skill asked for
`[HandlerFunctions(...)]` to match the invoked handlers exactly. The reference
says every *nonoptional* listed handler must execute, and that send-notification
and recall-notification handlers can be optional. As written, an agent could
flag a deliberately unused optional notification handler.
The discriminator is narrower than the handler type. Both
`[SendNotificationHandler([HandlerIsOptional: Boolean])]` and
`[RecallNotificationHandler([HandlerIsOptional: Boolean])]` take an explicit
optionality argument, so `[SendNotificationHandler(true)]` is exempt while the
same attribute written without the argument stays nonoptional like every other
handler type. Keying the exemption on the argument rather than the type keeps it
checkable from the diff and avoids the opposite false positive, where an agent
stops flagging genuinely nonoptional notification handlers.
Changes:
- The article now states the nonoptional qualifier, explains that optionality is
declared rather than inferred, and adds an explicit do-not-flag clause. That
clause also forbids proposing removal, because the listed entry is what keeps
the test passing on the runs where the notification does fire.
- The testing leaf skill carries the same boundary in its `ui-handlers-in-tests`
cue, and its mechanical-fix list no longer allows removing a listed optional
notification handler as a one-click suggestion.
- `SendNotificationHandler` and `RecallNotificationHandler` were missing from the
skill's testing token list, so notification handlers were not reliably
surfaced to the relevance step at all. Both are now listed.
- The good sample gains a test that lists an unreached
`[SendNotificationHandler(true)]`; the bad sample gains the mirror image, an
unreached `[SendNotificationHandler]` with no optionality argument. The pair
differs only by that argument, which is the point.
- `evaluation/review-fixtures.json` pins the testing domain to
`ui-handlers-in-tests` so the boundary is exercised: the good sample is the
clean control at `minimumCleanRate` 1.0 and the bad sample is the expected
finding. Keywords were retagged with `notification` and `optional-handler`.
validate_frontmatter.py reports 0 errors; Test-ReviewFixtures.ps1 passes with
32 cases across 16 leaf domains and resolves the testing fixture to this article.
* 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>
* Emit human-readable domain label on review findings
Add an optional findings[].domain field to the DO review output contract so
each finding carries its own human-readable review-domain display label. Leaf
review skills set it on every finding they emit; the al-code-review super-skill
copies it verbatim during rollup and sets it to "Agent" for its own
cross-cutting agent findings. This decouples consumers from BCQuality's domain
taxonomy: they render finding.domain verbatim instead of maintaining a
sub-skill-id -> label map.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Define domain display-label constraints
Clarify that review domains may contain internal whitespace, punctuation, case-sensitive text, and non-ASCII characters. Require consumers to preserve and safely encode the complete label instead of relying on lossy slugs, matching the replacement BC-ALAgents consumer.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 77d0a40e-8bf5-40ac-a450-40eb0255db03
---------
Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
