Skip to content

Repository navigation

dataset(code-review): add reaction gold answers from BCApps PR 10990-neg-3926331607-source-faithful-20261005 #922

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Wenjie Fan (gggdttt) wants to merge 1 commit into from
Closed

dataset(code-review): add reaction gold answers from BCApps PR 10990-neg-3926331607-source-faithful-20261005 #922

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions .github/self-improvement-selection.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
{
"version": 1,
"mode": "new",
"datasetBase": "6fe2d36319d9198f64696f027eaaef495916531d",
"datasetHash": "39B118C1CDE2A4BA879BA544288C777E80D99B14F7882870045CC993BBE830B5",
"sourceRepo": "microsoft/BCApps",
"requestId": "neg-3926331607-source-faithful-20261005",
"sourceCommentUrls": [
"https://github.com/microsoft/BCApps/pull/10990#discussion_r3926331607"
],
"entryIds": [
"synthetic__upgrade-contoso-demo-data-no-upgrade-wiring-01"
],
"newEntryIds": [
"synthetic__upgrade-contoso-demo-data-no-upgrade-wiring-01"
],
"reusedEntryIds": [],
"selections": [
{
"instanceId": "synthetic__upgrade-contoso-demo-data-no-upgrade-wiring-01",
"origin": "new",
"coverage": [
{
"commentUrl": "https://github.com/microsoft/BCApps/pull/10990#discussion_r3926331607",
"purpose": "false_positive_guard",
"file": "src/Apps/W1/ExpenseAgent/demo data/Demo Data/ExpenseAgentContosoModule.Codeunit.al",
"lineStart": 40,
"lineEnd": 40,
"rationale": "Exact pipeline-validated negative snapshot (base_commit 1e3872168af58518ac2a02e8cf5934cbc28eee32) reproducing the reviewed hunk: Codeunit.Run(Codeunit::\"Create Expense VAT Rates\") added inside CreateMasterData() of a codeunit implementing \"Contoso Demo Data Module\". The maintainer rejected the request for an Upgrade-subtype entry point because this is Preview demo data invoked only through the explicit Contoso Demo Data tool, never automatically on install or version upgrade -- no subscriber or Subtype=Install/Upgrade exists anywhere in this module for any of its demo-data procedures, so the alleged upgrade-coverage gap is not a defect introduced by this change. Limitation: this guard covers only Contoso demo-data-module seeding helpers invoked exclusively through the demo-data tool; it does not generalize to codeunits that genuinely participate in the install/upgrade pipeline, nor does it certify every Preview-labelled feature as upgrade-exempt."
}
]
}
],
"payloadHashes": [
{
"sha256": "B2A8C1E20FEEE3A08984CFC606F74BB8ECF78666DB6649320C6759E1A065E7BA",
"instanceId": "synthetic__upgrade-contoso-demo-data-no-upgrade-wiring-01"
}
],
"sourceSnapshots": [
{
"commentUrl": "https://github.com/microsoft/BCApps/pull/10990#discussion_r3926331607",
"repo": "microsoft/BCApps",
"file": "src/Apps/W1/ExpenseAgent/demo data/Demo Data/ExpenseAgentContosoModule.Codeunit.al",
"prBaseCommit": "b149af232025ef8709bf1d3b6de175a48eabc8e6",
"headCommit": "b473dbdd1077047bec2860841efba8f1c04330b2",
"reviewCommit": "233457072db3f9fbb9e01ad5e425abfa42b8cffa",
"baseCommit": "1e3872168af58518ac2a02e8cf5934cbc28eee32",
"lineStart": 40,
"lineEnd": 40,
"patch": "diff --git a/src/Apps/W1/ExpenseAgent/demo data/Demo Data/ExpenseAgentContosoModule.Codeunit.al b/src/Apps/W1/ExpenseAgent/demo data/Demo Data/ExpenseAgentContosoModule.Codeunit.al\n--- a/src/Apps/W1/ExpenseAgent/demo data/Demo Data/ExpenseAgentContosoModule.Codeunit.al\t\n+++ b/src/Apps/W1/ExpenseAgent/demo data/Demo Data/ExpenseAgentContosoModule.Codeunit.al\t\n@@ -37,6 +37,7 @@ codeunit 8202 \"Expense Agent Contoso Module\" implements \"Contoso Demo Data Modul\n CreateExpenseCountryData: Codeunit \"Create Expense Country Data\";\n begin\n CreateExpenseCountryData.CreateMasterData();\n+ Codeunit.Run(Codeunit::\"Create Expense VAT Rates\");\n end;\n \n procedure CreateTransactionalData()\n"
}
]
}
1 change: 1 addition & 0 deletions dataset/codereview.jsonl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,3 +143,4 @@
{"repo": "microsoft/BCApps", "instance_id": "synthetic__breaking-scope-creep-unrelated-api-01", "base_commit": "397d01199c321e774edaf23a7290fee40f75c6a6", "created_at": "2026-08-18T00:00:00Z", "environment_setup_version": "27.0", "project_paths": [], "metadata": {"area": "breaking-changes"}, "patch": "diff --git a/src/Apps/W1/Subscription Billing/App/Service Commitments/Tables/SubscriptionLine.Table.al b/src/Apps/W1/Subscription Billing/App/Service Commitments/Tables/SubscriptionLine.Table.al\n--- a/src/Apps/W1/Subscription Billing/App/Service Commitments/Tables/SubscriptionLine.Table.al\n+++ b/src/Apps/W1/Subscription Billing/App/Service Commitments/Tables/SubscriptionLine.Table.al\n@@ -906,7 +906,7 @@ table 8059 \"Subscription Line\"\n until BillingLine.Next() = 0;\n end;\n \n- internal procedure UpdateNextBillingDate(LastBillingToDate: Date)\n+ procedure UpdateNextBillingDate(LastBillingToDate: Date)\n var\n NewNextBillingDate: Date;\n OriginalInvoicedToDate: Date;\ndiff --git a/src/Apps/W1/Subscription Billing/App/Billing/Codeunits/BillingProposal.Codeunit.al b/src/Apps/W1/Subscription Billing/App/Billing/Codeunits/BillingProposal.Codeunit.al\n--- a/src/Apps/W1/Subscription Billing/App/Billing/Codeunits/BillingProposal.Codeunit.al\n+++ b/src/Apps/W1/Subscription Billing/App/Billing/Codeunits/BillingProposal.Codeunit.al\n@@ -463,6 +463,7 @@ codeunit 8062 \"Billing Proposal\"\n BillingLine.\"Subscription Contract Line No.\" := ServiceCommitment.\"Subscription Contract Line No.\";\n BillingLine.\"Discount %\" := ServiceCommitment.\"Discount %\";\n BillingLine.Discount := ServiceCommitment.Discount;\n+ ServiceObject.SetLoadFields(Quantity);\n ServiceObject.Get(ServiceCommitment.\"Subscription Header No.\");\n BillingLine.\"Service Object Quantity\" := BillingLine.GetSign() * ServiceObject.Quantity;\n OnAfterUpdateBillingLineFromSubscriptionLine(BillingLine, ServiceCommitment);\n", "expected_comments": [{"file": "src/Apps/W1/Subscription Billing/App/Service Commitments/Tables/SubscriptionLine.Table.al", "line_start": 909, "line_end": 909, "severity": "low", "domain": "breaking-changes", "body": "This performance change also widens the existing UpdateNextBillingDate procedure from internal to public, creating a new external compatibility commitment unrelated to the SetLoadFields optimization. Split the API change into a dedicated review or provide the intended external-consumer and compatibility rationale; otherwise keep it internal. This is an API-scope concern, not a security boundary, so no authorization check is implied.", "articles": ["breaking-changes/choose-access-modifiers-deliberately"]}], "category": "code-review", "description": "Low-severity API-scope concern: a real base-to-head internal-to-public change is bundled with an unrelated SetLoadFields optimization. This differs from synthetic__breaking-access-modifier-01, which tests a newly introduced internal helper that should be local.", "expect_findings": true, "source": "vsoadmin"}
{"repo": "microsoft/BCApps", "instance_id": "synthetic__perf-progress-dialog-deleteall-01", "base_commit": "397d01199c321e774edaf23a7290fee40f75c6a6", "created_at": "2026-08-18T00:00:00Z", "environment_setup_version": "27.0", "project_paths": [], "metadata": {"area": "performance", "articles": ["performance/use-deleteall-for-filtered-bulk-deletion"]}, "patch": "diff --git a/src/Apps/W1/Subscription Billing/App/Billing/Tables/BillingLine.Table.al b/src/Apps/W1/Subscription Billing/App/Billing/Tables/BillingLine.Table.al\n--- a/src/Apps/W1/Subscription Billing/App/Billing/Tables/BillingLine.Table.al\n+++ b/src/Apps/W1/Subscription Billing/App/Billing/Tables/BillingLine.Table.al\n@@ -249,10 +249,11 @@ table 8061 \"Billing Line\"\n var\n BillingLine2: Record \"Billing Line\";\n begin\n if \"Document No.\" <> '' then\n Error(CannotDeleteBillingLinesWithDocumentNoErr);\n+ BillingLine2.SetLoadFields(\"Entry No.\");\n FindFirstBillingLineForServiceCommitment(BillingLine2);\n if (BillingLine2.\"Entry No.\" = \"Entry No.\") then\n ResetServiceCommitmentNextBillingDate()\n else\n Error(OnlyLastServiceLineCanBeDeletedErr, \"Subscription Header No.\");\n RecalculateCustomerContractHarmonizedBillingFields();\ndiff --git a/src/Apps/W1/Subscription Billing/App/Billing/Codeunits/BillingProposal.Codeunit.al b/src/Apps/W1/Subscription Billing/App/Billing/Codeunits/BillingProposal.Codeunit.al\n--- a/src/Apps/W1/Subscription Billing/App/Billing/Codeunits/BillingProposal.Codeunit.al\n+++ b/src/Apps/W1/Subscription Billing/App/Billing/Codeunits/BillingProposal.Codeunit.al\n@@ -557,7 +557,10 @@ codeunit 8062 \"Billing Proposal\"\n ClearBillingProposalOptionsMySuggestionsOnlyTxt: Label 'All billing proposals (user %1 only), Only current billing template proposal', Comment = '%1: User ID';\n ClearBillingProposalQst: Label 'Which billing proposal(s) should be deleted?';\n StrMenuResponse: Integer;\n+ Counter: Integer;\n+ ProgressWindow: Dialog;\n+ DeletingBillingLinesMsg: Label 'Deleted billing lines: #1######';\n begin\n DisplayErrorIfNotAuthorizedToClearProposalOrDeleteDocuments();\n BillingTemplate.Get(BillingTemplateCode);\n if BillingTemplate.\"My Suggestions Only\" then\n@@ -575,12 +578,26 @@ codeunit 8062 \"Billing Proposal\"\n BillingLine.SetRange(Partner, BillingTemplate.Partner);\n if BillingTemplate.\"My Suggestions Only\" then\n BillingLine.SetRange(\"User ID\", UserId());\n- BillingLine.DeleteAll(true);\n+ ProgressWindow.Open(DeletingBillingLinesMsg);\n+ if BillingLine.FindSet() then\n+ repeat\n+ Counter += 1;\n+ ProgressWindow.Update(1, Counter);\n+ BillingLine.Delete(true);\n+ until BillingLine.Next() = 0;\n+ ProgressWindow.Close();\n end;\n 2:\n begin\n BillingLine.SetRange(\"Billing Template Code\", BillingTemplate.Code);\n- BillingLine.DeleteAll(true);\n+ ProgressWindow.Open(DeletingBillingLinesMsg);\n+ if BillingLine.FindSet() then\n+ repeat\n+ Counter += 1;\n+ ProgressWindow.Update(1, Counter);\n+ BillingLine.Delete(true);\n+ until BillingLine.Next() = 0;\n+ ProgressWindow.Close();\n end;\n end;\n end;\n", "expected_comments": [], "category": "code-review", "description": "False-positive boundary: the replaced operation is DeleteAll(true), and the target Billing Line table visibly has substantive OnDelete work that forces the bulk call to execute row by row. The explicit FindSet/Delete(true) loop preserves that trigger behavior while adding per-row progress; this narrow bulk-regression case is not a generic progress-dialog exemption.", "expect_findings": false, "source": "vsoadmin"}
{"repo":"microsoft/BCApps","instance_id":"synthetic__events-ishandled-added-to-shipped-event-01","base_commit":"9c19e09197bc23a6fdbc74bed36ba592f6c30e08","created_at":"2026-08-31T00:00:00Z","environment_setup_version":"27.0","project_paths":[],"metadata":{"area":"events","articles":[]},"patch":"diff --git a/src/Layers/W1/BaseApp/Warehouse/Activity/WarehouseActivityLine.Table.al b/src/Layers/W1/BaseApp/Warehouse/Activity/WarehouseActivityLine.Table.al\nindex 8070179cca..3e56413041 100644\n--- a/src/Layers/W1/BaseApp/Warehouse/Activity/WarehouseActivityLine.Table.al\n+++ b/src/Layers/W1/BaseApp/Warehouse/Activity/WarehouseActivityLine.Table.al\n@@ -1048,8 +1048,13 @@ table 5767 \"Warehouse Activity Line\"\n OutstandingQtyCannotbeLessThanZeroErr: Label 'Outstanding Qty. base cannot be less than 0.';\n \n procedure CalcQty(QtyBase: Decimal): Decimal\n+ var\n+ NewQtyBase: Decimal;\n+ IsHandled: Boolean;\n begin\n- OnBeforeCalcQty(Rec, QtyBase);\n+ OnBeforeCalcQty(Rec, QtyBase, NewQtyBase, IsHandled);\n+ if IsHandled then\n+ exit(Round(NewQtyBase, UOMMgt.QtyRndPrecision()));\n TestField(\"Qty. per Unit of Measure\");\n exit(Round(QtyBase / \"Qty. per Unit of Measure\", UOMMgt.QtyRndPrecision()));\n end;\n@@ -3464,7 +3469,7 @@ table 5767 \"Warehouse Activity Line\"\n end;\n \n [IntegrationEvent(false, false)]\n- local procedure OnBeforeCalcQty(var WarehouseActivityLine: Record \"Warehouse Activity Line\"; QtyBase: Decimal)\n+ local procedure OnBeforeCalcQty(var WarehouseActivityLine: Record \"Warehouse Activity Line\"; QtyBase: Decimal; var NewQtyBase: Decimal; var IsHandled: Boolean)\n begin\n end;\n \n","expected_comments":[{"file":"src/Layers/W1/BaseApp/Warehouse/Activity/WarehouseActivityLine.Table.al","line_start":3472,"line_end":3472,"severity":"medium","domain":"events","body":"OnBeforeCalcQty already shipped as a plain notification event with the signature (var WarehouseActivityLine: Record \"Warehouse Activity Line\"; QtyBase: Decimal). Appending var NewQtyBase: Decimal and var IsHandled: Boolean converts it into an overridable seam. Existing subscribers keep binding, because AL binds event parameters by name and a subscriber may omit parameters it does not declare, so nothing fails to compile - but the contract they were written against changed underneath them: any subscriber can now set IsHandled and suppress the base rounding for every existing caller of CalcQty. Leave the shipped event untouched and publish a separate OnBefore event that carries NewQtyBase and IsHandled at the point you want to make overridable, so the notification contract and the override seam stay distinct.","articles":["events/do-not-add-ishandled-to-an-existing-event"]}],"ignored_comments":[{"file":"src/Layers/W1/BaseApp/Warehouse/Activity/WarehouseActivityLine.Table.al","line_start":1056,"line_end":1057,"severity":"low","domain":"events","body":"Acceptable but not required: the new IsHandled guard also skips TestField(\"Qty. per Unit of Measure\"). CalcQty is a pure, side-effect-free calculation, and events/do-not-bypass-critical-operations-with-ishandled explicitly permits an IsHandled guard around such a calculation, so this observation must not be scored as a false positive nor demanded for recall.","articles":["events/do-not-bypass-critical-operations-with-ishandled"]}],"category":"code-review","description":"Real BCApps change that appends var NewQtyBase and var IsHandled to the already-shipped local IntegrationEvent OnBeforeCalcQty on table 5767, converting a notification event into an overridable seam. Binding is unaffected (AL binds event parameters by name), so the finding is the contract change, not a compile or binding break. The IsHandled guard skipping TestField is carried as an ignored comment because the guarded body is a pure calculation, which do-not-bypass-critical-operations-with-ishandled permits. The fresh local IsHandled needs no explicit reset, consistent with synthetic__events-ishandled-reset-boundary-01, so an initialize-IsHandled finding here is a false positive.","expect_findings":true,"source":"handcrafted"}
{"repo":"microsoft/BCApps","instance_id":"synthetic__upgrade-contoso-demo-data-no-upgrade-wiring-01","base_commit":"1e3872168af58518ac2a02e8cf5934cbc28eee32","created_at":"2026-10-05T08:07:28Z","environment_setup_version":"27.0","project_paths":[],"metadata":{"area":"upgrade"},"patch":"diff --git a/src/Apps/W1/ExpenseAgent/demo data/Demo Data/ExpenseAgentContosoModule.Codeunit.al b/src/Apps/W1/ExpenseAgent/demo data/Demo Data/ExpenseAgentContosoModule.Codeunit.al\n--- a/src/Apps/W1/ExpenseAgent/demo data/Demo Data/ExpenseAgentContosoModule.Codeunit.al\t\n+++ b/src/Apps/W1/ExpenseAgent/demo data/Demo Data/ExpenseAgentContosoModule.Codeunit.al\t\n@@ -37,6 +37,7 @@ codeunit 8202 \"Expense Agent Contoso Module\" implements \"Contoso Demo Data Modul\n CreateExpenseCountryData: Codeunit \"Create Expense Country Data\";\n begin\n CreateExpenseCountryData.CreateMasterData();\n+ Codeunit.Run(Codeunit::\"Create Expense VAT Rates\");\n end;\n \n procedure CreateTransactionalData()\n","expected_comments":[],"category":"code-review","description":"False-positive guard: codeunit 8202 \"Expense Agent Contoso Module\" implements interface \"Contoso Demo Data Module\" and its CreateMasterData() procedure (here extended with Codeunit.Run(Codeunit::\"Create Expense VAT Rates\")) is only ever invoked by the Contoso Demo Data tool's explicit, user-triggered generation flow -- the codeunit declares no Subtype = Install or Subtype = Upgrade and subscribes to no OnInstall.../OnUpgrade... trigger, so the platform never runs it automatically on extension install or version upgrade. Because demo-data master-data seeding in this architecture has never been wired into the automatic upgrade pipeline for any Contoso demo module, adding one more seeding call here does not newly strand already-provisioned tenants; requiring an Upgrade-subtype entry point misapplies upgrade-pipeline guidance to a manually-invoked demo-data helper. Expect no findings (BCApps PR 10990, rejected by maintainer: \"Not needed here, this is Preview demo data\").","expect_findings":false,"source":"vsoadmin"}
Loading