Add read-only AL development plan guidance - #155
Jesper Schulz-Wedde (JesperSchulz) wants to merge 7 commits into
Conversation
Add read-only planning and repository-changing development skills so BCQuality knowledge can guide features, bug fixes, refactors, upgrades, and maintenance before the existing AL review gate runs. Track Microsoft Learn ingestion and add development and BCApps-shaped guidance evaluation fixtures. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 638b66d2-9f06-4f60-8781-808709e1485c
|
👋 Heads up Jesper Schulz-Wedde (@JesperSchulz) — and cc maintainers — this PR introduces new top-level entries that aren't part of BCQuality's known repository structure:
This isn't a block — just a flag. 🚩 New top-level folders and files are usually unintended (a stray export, a tool's scratch dir, or content that meant to land inside an existing layer like If this was intentional and the new entry genuinely belongs at the repo root, a maintainer can review and merge as normal — no action needed beyond a quick sanity check. If it wasn't, please move the content into the right existing layer (or drop it) and push an update. 🙏 A maintainer will take a look before merging. |
Bound post-implementation review rounds, expose output kinds in Entry dispatch, enforce capability coverage, map BCFIX-HANDOFF v1, clarify no-knowledge behavior, and reject repository-escaping skill paths. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 638b66d2-9f06-4f60-8781-808709e1485c
Retain shared knowledge enrichment and review guidance; defer standalone implementation and source-ingestion tracking. Add runner-owned baseline evidence, contract regressions, and explicit consumer/pilot boundaries. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Reconcile the read-only plan-enrichment contracts with main's folder-review inputs and documentation structure. Record Windows alternate streams in runner evidence and clear the regression harness exit status after expected negative probes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 638b66d2-9f06-4f60-8781-808709e1485c
Accept main's removal of deterministic compiler and analyzer duplicates, including the ApplicationArea rule, while retaining the read-only development guidance contract and non-mechanical Learn knowledge. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 638b66d2-9f06-4f60-8781-808709e1485c
Jesper Schulz-Wedde (JesperSchulz)
left a comment
There was a problem hiding this comment.
Blocking self-review finding: the read-only plan-enrichment contract and evaluator corrections look coherent, but the current-main merge is not clean under the repository's exact retrieval contract. After merging current main (852a676) into this head, tools/Test-KnowledgeRetrieval.ps1 fails because all six new sample-backed articles mention .good.al/.bad.al as code spans rather than the exact READ links ([filename](filename)). Please update the 12 sample references in the new data-modeling, interfaces, UI, and upgrade articles to the required relative Markdown-link form, then rerun the merged-tree validators. The other current-head and merged-tree contract/schema/guidance tests I ran passed; Git reports the branch mergeable, and there are no prior review threads to resolve.
GitHub does not permit CHANGES_REQUESTED on one's own pull request, so this is submitted as a formal COMMENTED review; treat it as a request for the necessary change above.
|
The core idea of this new skill is to generate a new guidance document that should be sent to the implementer (normally a less capable model) alongisde the plan so that the implementer is "guarded" against some of the pitfalls that are covered in the BCQuality knowledge? So this would make us "proactive" instead of "reactive" (inject guidance instead of reviewing against it) right? I like this, still the review skill will still need to be run after but I would expect that the review skill would then catch less and less issues :) |
That’s exactly the hypothesis! One nuance: the skill does not generate or replace the development plan. It produces a read-only guidance report from the existing plan and repository context. The consuming workflow then persists that guidance and supplies it alongside the plan to the implementer. So the intended flow is: That shifts BCQuality from purely reactive to proactive, while keeping the final review as a safety net. We hope this means fewer findings, fewer correction rounds, and potentially good results from smaller or faster implementation models, but that still needs evidence. I did just now run one more small experiment, with promising results:
The guided candidate had one fewer top-level review comment, but the larger difference was severity: it had no Critical or High findings. Of course that's just a small experiment, but I've run a couple of those and they do point in the same direction. But we need more validation! Pulling it into your fork and comparing results would hence be greatly appreciated! If possible, keep the task, repository, plan, model, and tools identical, changing only whether the guidance is supplied. Positive, neutral, and negative results are all valuable! Please share what you learn. 🙏 |
Reconcile the read-only guidance output with the machine-readable skill index, adopt linked sample references required by bounded retrieval, and update the guidance regression fixture for the retrieval helper dependency. Permit only the known endpoint-DLP metadata stream during read-only evidence capture. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 638b66d2-9f06-4f60-8781-808709e1485c
Keep plan enrichment internal and read-only pending consumer agreement and runtime pilot evidence. Move knowledge to its independent PR and remove the consumer-owned forensic evaluator. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 638b66d2-9f06-4f60-8781-808709e1485c
Summary
Define a provisional, internal, read-only contract for enriching an existing AL development plan with applicable BCQuality constraints.
al-development-plan, requiring a non-empty development plan and optionally using repository context to confirm symbols and applicability.development-plan,development-guidance-report, and output-aware dispatch metadata.This PR does not register a new standalone plugin skill, change plugin version
0.2.0, or claim a stable public integration surface. Consumer-owner agreement and a runtime pilot are prerequisites to freezing that interface.Sequencing and retrieval design
The independently shippable seven-article Learn-derived knowledge slice and its four review-skill mappings moved to #195. That PR includes the corrected interface-casting runtime (
15.0) and can land independently.Plan enrichment intentionally does not dispatch the existing review leaves. Review leaves inspect source through domain-specific code signals; pre-implementation plan enrichment retrieves cross-domain constraints from plan vocabulary and confirmed repository symbols. Both paths still use the same knowledge index, applicability rules, layer precedence, and full normative article bodies.
Consumer-specific integrations own normalized plan input, state persistence, phase injection, immutable checkout verification, and runtime evidence.
no-knowledgemeans no additional applicable BCQuality constraints; it is not a refusal to implement. Missing repository context producespartialonly when it materially prevents reliable selection.Deliberately excluded
The pilot or consuming workflow should own runtime mutation controls and effectiveness evidence rather than imposing platform-specific forensics on every BCQuality change.
Validation
These checks validate the retained contract and existing behavior; they do not claim improved implementation outcomes.