Skip to content

Add data-query category: AL query-generation benchmark - #740

Open
Onat Buyukakkus (onbuyuka) wants to merge 39 commits into
mainfrom
onbuyuka/data-query-category
Open

Add data-query category: AL query-generation benchmark#740
Onat Buyukakkus (onbuyuka) wants to merge 39 commits into
mainfrom
onbuyuka/data-query-category

Conversation

@onbuyuka

@onbuyuka Onat Buyukakkus (onbuyuka) commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

What

Adds a new data-query evaluation category: an offline benchmark for AL query generation. Given a natural-language data question, the agent authors a single AL query object (query.al) and it's scored deterministically — compile + run the generated query and a gold reference query against the container's Contoso demo data, pass if the result sets match. No MCP server, no LLM judge.

This complements the AI Test Toolkit evals in the platform repo: those test the MCP server end-to-end; this benchmarks models/agents on query generation.

How it works

  • Dataset (dataset/dataquery.jsonl) — 11 entries, each with nl_prompt + gold_query (the reference AL query whose result set defines "correct") + environment_setup_version + ordered.

  • Pipeline (evaluate/dataquery.py) — the agent writes query.al; the harness compiles + runs the generated and gold queries and compares result sets.

    • build = generated query compiled and ran; resolved (ResolutionRate) = result set matches gold.

    • Rows compared by value; numeric columns are normalized scale-insensitively (500 == 500.0) while string Code/No. columns are compared verbatim ("001" != "1"). Column names/order ignored; order-insensitive unless the entry marks the question ordered.

  • Run mechanism (operations/bc_operations.py) — wrap_query_as_api turns each query into an API query; execute_al_query publishes a throwaway app to the container and reads its OData endpoint (following @odata.nextLink so large result sets aren't truncated).

  • Execution-based category (requires_container = True, runner GitHub-BCBench) — a stock BC sandbox artifact (Cronus/Contoso data) suffices; no special build needed. Setup-ContainerAndRepository.ps1 skips the git clone for data-query (there's no repo) and just provisions the container.

  • Skill — an agent-facing al-query-authoring SKILL.md (instructions/dataquery-bc/skills/) documenting AL query authoring, including the canonical column(Name) { Method = Count; } idiom (a field-less Count; a source field trips AL0353).

Scoring integrity & robustness

  • Fail loud on broken golds — evaluation runs the gold query first and does not swallow its errors: a gold that doesn't compile/run raises and fails the run, so it gets fixed rather than silently scored. An empty agent output is recorded as a tracked build failure (No query.al produced).
  • Deterministic comparison — numeric-only decimal canonicalization (Code/No. strings preserved verbatim, so "001" != "1"; amounts scale-insensitive, 500 == 500.0), OData @odata.nextLink paging, timeout → BuildTimeoutExpired, malformed agent output → BuildError, case-insensitive query/QueryType wrapping, OrderBy treated as a property, and throwaway apps uninstalled between runs to avoid object-ID conflicts.

How to run

Actions → Evaluation with GitHub Copilot (or Claude Code) → Run workflow → category = data-query. The self-hosted runner provisions the container; no local setup.

New --skills dispatch input (mirrors --al-lsp / --al-mcp) toggles the agent skill per run — default off. Its exact shape is being reconciled with #761's skill handling on rebase.

Tested

  • ✅ Unit tests: result_sets_match (incl. digit-only Code non-collapsing), wrap_query_as_api, run-template wiring, and --skills override (tests/test_dataquery_evaluation.py, tests/test_agent_skills.py); full suite 695 pass; ruff + ty clean.
  • End-to-end on the self-hosted runner: the execute_al_query container round-trip (compile → publish → OData → result-set compare) is validated — generated and gold queries build, publish, and compare, producing real build/resolved scores.

Results

Run 30405864654 (claude-sonnet-4.6, skills on + AL LSP): 9/11 resolved, 11/11 build. Every generated query compiles and runs, and the two misses (avg-invoice-amount-by-country, total-purchase-amount-by-vendor) both build but diverge from gold on metric-definition ambiguity (which amount / net vs incl-VAT, per-invoice vs per-line) — left as documented misses for a future gold/prompt-reconciliation pass rather than harness defects.

Known non-blocking CI caveat: on a data-query run the matrix + summarize + bceval-upload jobs succeed, but the summarize job's Update leaderboard step fails because it checks out main, whose EvaluationCategory enum has no data-query yet. This self-resolves once this PR merges.

Depends on #761: generic non-repo-category support (skip-repo as a category property, optional repo/base_commit) is landed in #761; this PR merges after it and takes up those abstractions on rebase (dropping the data-query clone-skip special-case).

Review feedback has been addressed in follow-up commits and every review thread is resolved (bar the --skills thread, kept open pending the #761 rebase) — see the review-response comments.

Onat Buyukakkus and others added 5 commits July 12, 2026 15:07
Adds a new execution-based `data-query` category that benchmarks models/agents
at generating Business Central AL queries. Given a natural-language data question,
the agent writes a single AL query object to query.al; evaluation compiles and runs
both the generated query and a gold reference query against the container's Contoso
demo data and compares the result sets. No MCP server and no LLM judge.

- types.py: DATA_QUERY -> execution-based (ExecutionBasedEvaluationResult, summary,
  aggregate; resolution_rate/build_rate; ResolutionRate; requires_container; GitHub-BCBench)
- DataQueryEntry: nl_prompt + gold_query + ordered; dataset/dataquery.jsonl (6 tasks)
- DataQueryPipeline + result_sets_match (value-based, order-insensitive; unit-tested)
- operations: wrap_query_as_api (unit-tested) + execute_al_query (wrap as API query,
  publish throwaway app, read OData)
- ExecutionBasedEvaluationResult.create_result for compiled-but-wrong outcomes
- config.yaml: data-query prompt (author query.al); al-query-authoring skill
- Setup-ContainerAndRepository.ps1: skip repo clone for data-query (no repo), just
  provision the sandbox container; a stock Contoso artifact suffices
- Wire data-query into the copilot/claude evaluation workflow category choices + docs

Container round-trip in execute_al_query and the gold AL query bodies need validation
on a runner (no local BC container); pure logic is unit-tested (592 tests pass).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: db34a2a0-7035-4361-b911-becb72f86e21
…atch)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: db34a2a0-7035-4361-b911-becb72f86e21
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: db34a2a0-7035-4361-b911-becb72f86e21
…d into container)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: db34a2a0-7035-4361-b911-becb72f86e21

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Good stuff.

Comment thread scripts/Setup-ContainerAndRepository.ps1 Outdated
Comment thread src/bcbench/types.py Outdated
Comment thread src/bcbench/types.py Outdated
Onat Buyukakkus and others added 7 commits July 13, 2026 10:55
… (AL0124)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: db34a2a0-7035-4361-b911-becb72f86e21
A gold query failing to compile/run is a harness/dataset problem, not the
agent's, so record it as a non-resolved result with a clear message instead
of letting the uncaught BuildError crash the whole matrix job.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: db34a2a0-7035-4361-b911-becb72f86e21
… sets

The object name is irrelevant to a query's result set (we score by comparing
data), but AL requires it be a valid <=30-char identifier and unique in the
tenant. Two of the first real runs failed only on AL0305 (agent chose a long
descriptive name), so normalize the name in wrap_query_as_api to keep the
benchmark focused on query logic. Also give the generated and gold API queries
distinct EntitySetName/EntityName so both can be published to the same tenant
without colliding on the OData route once a generated query finally compiles.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: db34a2a0-7035-4361-b911-becb72f86e21
… fetch

Root cause of the 0/4 build rate: the agents were writing valid AL (e.g. a
correct Vendor/Purch. Inv. Header query) but the compiler reported base tables
as missing (AL0185, '26.0.0.0 could not be found in the database'). The custom
Compile-AppInBcContainer -UpdateSymbols path did not load Base Application
symbols reliably (intermittent across containers).

Switch execute_al_query to the same Invoke-AppBuildAndPublish helper the passing
categories use (explicit cleared .alpackages symbol folder, GenerateReportLayout
No, ForceSync, dependencyPublishingOption ignore). Also fetch the query rows from
*inside* the container (Invoke-ScriptInBcContainer -> http://localhost:7048/BC/api)
so we no longer depend on host->container name resolution or published ports,
which the runner does not set up.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: db34a2a0-7035-4361-b911-becb72f86e21
…a fetch

Run 3 showed the compile+publish now works (Base App symbols resolve via the
proven helper), but the in-container OData fetch failed: PowerShell 7 refuses
Invoke-RestMethod -Credential over plain HTTP ('cannot protect plain text
secrets sent over unencrypted connections'). Build the Basic Authorization
header manually instead, which works on both Windows PowerShell 5.1 and
PowerShell 7. Add regression tests asserting the run template uses the proven
build helper, fetches from inside the container, and never passes -Credential
over HTTP.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: db34a2a0-7035-4361-b911-becb72f86e21
Run 4 proved the harness works end-to-end (build=2, real gold-vs-generated
result-set comparisons). The remaining resolved=0 was down to prompt ambiguity
and one buggy gold, not the harness:

- Tighten all prompts so a correct interpretation deterministically matches the
  gold: specify the measure and whether it is net of VAT, the source (line vs
  header, posted vs open), inner-join inclusion ('...that has at least one...'),
  and grouping. E.g. the vendor prompt now pins line-level Amount net of VAT
  (a model had reasonably summed header Amount Including VAT -> 5 vs 6 rows).
- Replace 'items on both open orders': its gold expressed a set intersection as
  a join with no aggregate column, so an AL query returns one row per matching
  (sales line x purchase line) pair instead of the distinct item set and cannot
  be scored deterministically (13 vs 12 rows). Swap in a clean aggregate join
  (open sales order count per customer).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: db34a2a0-7035-4361-b911-becb72f86e21
Broaden the dataquery benchmark with single-table and clean-join aggregates that
are deterministically scorable via result-set comparison:
- customer-count-by-country (single-table Count)
- outstanding-purchase-value-by-vendor (join + Sum, open POs, net of VAT)
- total-posted-sales-amount-by-customer (2-level join + Sum, net of VAT)
- line-count-per-open-sales-order (single-table Count, child rows per parent)
- total-purchased-quantity-by-item (single-table Sum)

Prompts pin the source table and net-of-VAT measure to avoid the interpretation
ambiguity that made earlier tasks noisy. Field names verified against the W1 Base
App. Gold queries to be confirmed against the container by the evaluation run.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: db34a2a0-7035-4361-b911-becb72f86e21
Copilot AI review requested due to automatic review settings July 28, 2026 20:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds the data-query benchmark for deterministic AL query generation and execution against Business Central demo data.

Changes:

  • Adds 11 query-generation dataset entries and agent guidance.
  • Implements query wrapping, execution, comparison, and result reporting.
  • Integrates container setup, workflows, tests, and documentation.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 12 comments.

Show a summary per file
File Description
dataset/dataquery.jsonl Adds benchmark entries and gold queries.
src/bcbench/evaluate/dataquery.py Implements evaluation and result comparison.
src/bcbench/operations/bc_operations.py Adds query wrapping and OData execution.
src/bcbench/dataset/dataset_entry.py Defines data-query entries.
src/bcbench/dataset/__init__.py Exports the new entry type.
src/bcbench/types.py Registers category runtime behavior.
src/bcbench/results/base.py Adds a general execution-result factory.
src/bcbench/evaluate/__init__.py Exports the pipeline.
src/bcbench/operations/__init__.py Exports query operations.
src/bcbench/commands/evaluate.py Supports mock data-query evaluation.
src/bcbench/agent/shared/config.yaml Adds the agent prompt template.
src/bcbench/agent/shared/instructions/dataquery-bc/skills/al-query-authoring/SKILL.md Adds AL query authoring guidance.
scripts/Setup-ContainerAndRepository.ps1 Creates clone-free workspaces.
scripts/BCBenchUtils.psm1 Resolves the new dataset category.
.github/workflows/copilot-evaluation.yml Enables Copilot runs.
.github/workflows/claude-evaluation.yml Enables Claude runs.
tests/test_dataquery_evaluation.py Tests comparison and wrapping logic.
tests/conftest.py Adds data-query fixtures.
tests/test_type_exhaustiveness.py Covers category type dispatch.
docs/data-query.md Documents the benchmark.
docs/index.md Links the new category.
Comments suppressed due to low confidence (1)

src/bcbench/evaluate/dataquery.py:115

  • execute_al_query also raises BuildTimeoutExpired on a gold-query timeout, and it is not a BuildError. This exception escapes instead of taking the intended harness/container failure path.
        except BuildError as e:
            logger.exception(f"Gold query failed to compile/run for {context.entry.instance_id}")
            self.save_result(
                context,

Comment thread src/bcbench/evaluate/dataquery.py Outdated
Comment thread src/bcbench/operations/bc_operations.py Outdated
Comment thread src/bcbench/operations/bc_operations.py Outdated
Comment thread src/bcbench/operations/bc_operations.py Outdated
Comment thread src/bcbench/evaluate/dataquery.py Outdated
Comment thread src/bcbench/types.py Outdated
Comment thread src/bcbench/operations/bc_operations.py Outdated
Comment thread src/bcbench/operations/bc_operations.py Outdated
Comment thread dataset/dataquery.jsonl Outdated
Scoring integrity:
- result_sets_match: canonicalize numbers with Decimal.normalize() instead of
  rounding through float to 4 decimals, so 1.00001 and 1.00002 are no longer
  scored equal (removes false positives) while 500 == 500.0 still holds.
- OData fetch: follow @odata.nextLink until exhausted so result sets larger than
  one page are not silently truncated (which could score different sets as equal).
- Gold-query failure is now recorded as unscorable (new ExecutionBasedEvaluationResult
  scorable flag) and excluded from resolved/total/build/instance_results, so a
  harness/dataset issue no longer counts against the agent's ResolutionRate.
- Catch BuildTimeoutExpired (not a BuildError) around both generated and gold
  query execution so a timeout is recorded instead of escaping and breaking
  summarization.
- wrap_query_as_api raises BuildError (handled downstream) instead of ValueError
  when the generated output has no query declaration or no object body.

Robustness:
- wrap_query_as_api matches the query keyword and QueryType removal
  case-insensitively and without requiring a leading newline, so cased/compact
  AL (Query 50123, { QueryType = Normal; ... }) no longer breaks ID reassignment
  or produces a duplicate QueryType property.
- execute_al_query uninstalls/unpublishes the throwaway query app before and
  after each run so re-running locally against the same container doesn't fail
  with an object-ID conflict on the fixed 50100/50101 range.

Docs/cleanup:
- SKILL.md: OrderBy is a property (OrderBy = descending(Col);), not a block.
- types.py: drop the stale MCP/seed-app comments; fold DATA_QUERY into the
  existing same-value match arms.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: db34a2a0-7035-4361-b911-becb72f86e21
Copilot AI review requested due to automatic review settings July 28, 2026 21:23
@onbuyuka

Copy link
Copy Markdown
Collaborator Author

Review feedback addressed (commit 129c5ef)

Thanks for the thorough review. Summary of what changed:

Scoring integrity

  • Numeric rounding false-positivesresult_sets_match now canonicalizes numbers with Decimal.normalize() (full precision, scale-insensitive) instead of rounding through float to 4 decimals. 1.000011.00002; 500 == 500.0.
  • OData pagination — the fetch now follows @odata.nextLink until exhausted, so large result sets aren't silently truncated to the first page.
  • Gold-failure biasing scores — a gold/harness failure is now recorded as unscorable (new scorable flag) and excluded from resolved/total/build/instance_results, so it no longer counts against the agent's ResolutionRate.
  • Timeout escapingBuildTimeoutExpired is now caught around both generated and gold execution (it isn't a BuildError), so a timeout is recorded instead of breaking summarization.
  • ValueError on malformed outputwrap_query_as_api now raises BuildError (handled downstream) when there's no query declaration or no object body.

Robustness

  • Case sensitivity — the query keyword reassignment and QueryType removal are now case-insensitive and don't require a leading newline, so Query 50123 / { QueryType = Normal; ... } no longer break ID reassignment or create a duplicate QueryType.
  • Object-ID conflict on re-runexecute_al_query now uninstalls/unpublishes the throwaway app before and after each run, so re-running locally against the same container doesn't hit a 50100/50101 conflict.

Docs/cleanup

  • SKILL.md: OrderBy corrected to a property (OrderBy = descending(Col);), not a block.
  • types.py: stale MCP/seed comments removed (folded DATA_QUERY into the existing match arms).
  • PR description: validation scope updated to the current 11 gold queries; the 5 new ones are in runner shakeout now.

Added unit tests for the precision fix, the case-insensitive/malformed wrap_query_as_api paths, and the paging/cleanup wiring. Full suite: 635 pass, ruff + ty clean.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

src/bcbench/operations/bc_operations.py:293

  • AL escapes a quote inside a quoted identifier by doubling it ("A ""quoted"" query"), not with a backslash. This regex stops at the first doubled quote, leaves the rest of the original name behind, and turns an otherwise valid query into invalid AL. Match doubled quotes in the quoted-name branch.
    text, replaced = re.subn(
        r'(\bquery\s+)\d+\s+("(?:[^"\\]|\\.)*"|\w+)',
        rf"\g<1>{object_id} {safe_name}",

src/bcbench/evaluate/dataquery.py:31

  • This converts every numeric-looking string to a number, so distinct AL text/code values such as "001" and "1" compare equal (and the earlier None conversion similarly equates null with ""). That can award resolution to a query returning the wrong identifier. Preserve string/null identity and normalize only values known to be numeric, or carry type information into comparison.
    try:
        # Canonical decimal form: scale/trailing-zero-insensitive (500 == 500.0) but full precision
        # preserved, so distinct values like 1.00001 and 1.00002 are NOT collapsed. No float rounding.
        return str(Decimal(text).normalize())
    except (InvalidOperation, ValueError):

Comment thread src/bcbench/results/base.py Outdated
AL query Count columns take no source field: `column(RowCount) { Method = Count; }`,
not `column(RowCount; "No.") { Method = Count; }` (the latter fails AL0353). The
four Count-based golds used the invalid form, and SKILL.md taught it — so the agent
reproduced the mistake and its query failed to compile before the gold was ever
reached, which is why these golds went unvalidated (see PR review comment #15).

- Remove the source field from the Count columns in customer-count-by-country,
  open-sales-order-count-by-customer, opportunity-count-by-status, and
  line-count-per-open-sales-order gold queries.
- SKILL.md: clarify that Count takes no source field, unlike Sum/Average/Min/Max.

Validated by the runner shakeout: the Sum-based new golds (outstanding-purchase-value
-by-vendor, total-purchased-quantity-by-item) already compiled, ran, and resolved.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: db34a2a0-7035-4361-b911-becb72f86e21
Copilot AI review requested due to automatic review settings July 28, 2026 21:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

src/bcbench/results/base.py:116

  • scorable=False is not propagated to the bc-eval records. category_metrics exports only resolved=False and build=True, and ResolutionRate/BuildRate score those values directly, so the externally reported headline metrics still count a gold-query failure as a resolution failure (and a build success), contrary to the new unscorable semantics. Export scorable and make the downstream evaluators skip these records, or omit unscorable records from the bc-eval export.
    def create_unscorable(cls, context: "EvaluationContext", output: str, error_message: str) -> Self:
        """A harness/dataset failure (not the agent's fault) that must not count toward the resolution rate."""
        return cls(**cls._base_fields(context), output=output, build=True, resolved=False, scorable=False, error_message=error_message)

src/bcbench/operations/bc_operations.py:426

  • The OData JSON is parsed through Python float before _normalize_value sees it, so high-magnitude BC Decimal values can lose precision and distinct results can compare equal (for example, adjacent cent values near BC Decimal's upper range). Parse JSON decimal literals directly as Decimal to preserve the deterministic comparison promised by the matcher.
    rows = json.loads(result_file.read_text(encoding="utf-8-sig") or "[]")

src/bcbench/evaluate/dataquery.py:88

  • The new pipeline's outcome logic is not covered by the added tests: there are no tests that mock execute_al_query and verify match, mismatch, generated build failure, and gold-query unscorable results. These branches define the benchmark's scores, and the current ordering/export issues are examples that helper-only tests do not catch. Add focused pipeline tests like those used for the existing evaluation pipelines.
    def evaluate(self, context: EvaluationContext[DataQueryEntry]) -> None:

Comment thread src/bcbench/evaluate/dataquery.py Outdated
Follow-up to the scorable flag: the local summary already excluded unscorable
results, but write_bceval_results() still exported them, so the uploaded/core
ResolutionRate counted a gold-query harness failure against the agent. Skip
unscorable results in the export path as well.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: db34a2a0-7035-4361-b911-becb72f86e21
Copilot AI review requested due to automatic review settings July 28, 2026 21:40
Establish gold validity independent of agent output: run the gold query first, so
a broken gold entry is recorded as unscorable regardless of whether the agent's
query compiled. Previously, if the agent query failed first, a broken dataset
entry was counted against that agent instead of being flagged as a harness issue.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: db34a2a0-7035-4361-b911-becb72f86e21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

src/bcbench/operations/bc_operations.py:305

  • The transform is not comment-aware. In a valid query with a preceding comment such as // QueryType = Normal;, this substitution removes the comment occurrence because count=1, leaves the real property, and then injects a second QueryType, causing compilation to fail. Likewise, text.find("{") can select a brace in a leading comment. Locate the declaration/body with comment-aware parsing and remove the actual object-level property rather than the first textual match.
    text = re.sub(r"\bQueryType\s*=\s*\w+\s*;", "", text, count=1, flags=re.IGNORECASE)

    brace_index = text.find("{")
    if brace_index == -1:
        raise BuildError("query-wrap", f"Generated query has no object body ('{{' not found):\n{query_text}")

src/bcbench/agent/shared/instructions/dataquery-bc/skills/al-query-authoring/SKILL.md:27

  • This example is effectively the gold solution for dataquery__outstanding-sales-value-by-customer-1: it uses the same Customer → Sales Line join, Order filter, and Outstanding Amount sum. Any run with this skill enabled receives the answer to a benchmark entry (including the first test-run entry), inflating that experiment's score. Replace it with a valid query pattern that is not represented in the dataset.
            dataitem(SalesLine; "Sales Line")
            {
                DataItemLink = "Sell-to Customer No." = Customer."No.";
                DataItemTableFilter = "Document Type" = const(Order);
                column(OutstandingAmount; "Outstanding Amount") { Method = Sum; }

src/bcbench/results/bceval_export.py:53

  • This scoring-critical skip path has no regression coverage: tests/test_result_writer.py comprehensively exercises write_bceval_results, but no test creates an execution result with scorable=False. Add mixed and all-unscorable cases to verify these records never reach the bc-eval JSONL output.
            # Unscorable results (harness/dataset failures, e.g. a gold query that didn't compile) must
            # not reach the uploaded/core score, or they'd count against the agent's ResolutionRate.
            if isinstance(result, ExecutionBasedEvaluationResult) and not result.scorable:
                logger.info(f"Skipping unscorable result from bceval export: {result.instance_id}")
                continue

Comment thread scripts/Setup-ContainerAndRepository.ps1 Outdated
Copilot AI review requested due to automatic review settings July 30, 2026 11:30
@haoranpb
Sun Haoran (haoranpb) changed the base branch from main to temp/pr740-rebase-base July 30, 2026 11:36
@haoranpb
Sun Haoran (haoranpb) changed the base branch from temp/pr740-rebase-base to main July 30, 2026 11:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Not ready to approve

Result normalization can produce false positives, and gold validation plus query wrapping still have correctness gaps.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Comments suppressed due to low confidence (2)

src/bcbench/evaluate/dataquery.py:114

  • This early return runs before the gold query, reintroducing the case where a broken gold is recorded as an ordinary agent build failure whenever query.al is missing. Move gold validation ahead of every generated-output failure so the documented “fail loud on broken golds” guarantee is independent of agent output.
        if not generated_query:
            logger.warning(f"Agent produced no {GENERATED_QUERY_FILE} for {context.entry.instance_id}")
            self.save_result(context, ExecutionBasedEvaluationResult.create_build_failure(context, output="", error_message=f"No {GENERATED_QUERY_FILE} produced"))
            return

src/bcbench/operations/bc_operations.py:306

  • The opening brace is searched from the start of the file rather than from the matched query declaration. A valid query preceded by a comment containing { gets the API properties injected into the comment, and the otherwise valid agent output fails compilation. Scope both brace discovery and QueryType removal to the matched object body.
    text = re.sub(r"\bQueryType\s*=\s*\w+\s*;", "", text, count=1, flags=re.IGNORECASE)

    brace_index = text.find("{")
  • Files reviewed: 26/26 changed files
  • Comments generated: 1
  • Review effort level: Medium

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Comment on lines +21 to +38
def _normalize_value(value: object) -> str:
if value is None:
return ""
if isinstance(value, bool):
return str(value).lower()
if isinstance(value, (int, float, Decimal)):
# Only values that arrived as numeric JSON types are canonicalized: scale/trailing-zero-
# insensitive (500 == 500.0) with full precision preserved (1.00001 != 1.00002) and no float
# rounding (Decimal built from the value's string form). Both gold and generated rows come
# through the same OData->JSON pipeline, so amounts are numbers on both sides.
try:
return str(Decimal(str(value)).normalize())
except (InvalidOperation, ValueError):
return str(value)
# Strings (and anything else) are preserved verbatim apart from a whitespace trim. Business Central
# Code/No. fields are JSON strings even when digit-only, so "001" must NOT collapse to "1" — coercing
# them through Decimal would let a wrong result be scored as matching the gold.
return str(value).strip()
@haoranpb
Sun Haoran (haoranpb) force-pushed the onbuyuka/data-query-category branch from dbc1caa to f0921f5 Compare July 30, 2026 11:49
Copilot AI review requested due to automatic review settings July 30, 2026 12:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Not ready to approve

Type-erasing normalization can produce false passes, while comment-sensitive query wrapping can reject valid AL queries.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Comments suppressed due to low confidence (2)

src/bcbench/evaluate/dataquery.py:25

  • Normalization still erases the JSON type: a Code value "1" and numeric 1 both normalize to the same string (likewise "true"/true and ""/null). Because column names are intentionally ignored, a query returning a value from a wrong-typed column can therefore be marked resolved. Keep a type tag in the normalized value while applying decimal canonicalization only within the numeric tag, and cover the "1" versus 1 case.
def _normalize_value(value: object) -> str:
    if value is None:
        return ""
    if isinstance(value, bool):
        return str(value).lower()

src/bcbench/operations/bc_operations.py:304

  • These regex/string searches are not scoped to AL syntax. A valid leading comment containing { makes this select the comment brace, and a comment containing QueryType = Normal; can consume the one removal and leave the real property duplicated with the injected API property. Such otherwise-valid agent queries are then scored as build failures. Locate the opening brace from the matched query declaration and remove the actual top-level QueryType property (ideally via token-aware parsing), rather than matching comments/strings.
    text = re.sub(r"\bQueryType\s*=\s*\w+\s*;", "", text, count=1, flags=re.IGNORECASE)

    brace_index = text.find("{")
  • Files reviewed: 28/28 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Copilot AI review requested due to automatic review settings July 30, 2026 12:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Not ready to approve

Gold-answer exposure and result-normalization collisions can invalidate benchmark scores.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Comments suppressed due to low confidence (4)

src/bcbench/evaluate/dataquery.py:75

  • The agent runs with unrestricted filesystem tools while repo_path is the checkout's testbed child, so it can read ../dataset/dataquery.jsonl (or recover it from ../.git) and copy the exact gold_query. This makes resolved scores vulnerable to direct answer leakage. Withhold the gold and the Git object database during the agent phase, or run the agent in an isolated workspace that cannot access the checkout.
        with github_log_group(f"{context.agent_name} -- Entry: {context.entry.instance_id}"):
            context.metrics, context.experiment = agent_runner(context)

src/bcbench/evaluate/dataquery.py:25

  • Normalization still loses the JSON type: a Code value "1" and numeric 1 both normalize to "1" (similarly "true"/true and ""/null). Because column names are ignored, a query returning the wrong typed field can be accepted. Keep a type tag in the canonical value while normalizing only the numeric payload.
def _normalize_value(value: object) -> str:
    if value is None:
        return ""
    if isinstance(value, bool):
        return str(value).lower()

src/bcbench/evaluate/dataquery.py:94

  • The empty-output return happens before the gold query is validated. If an entry's gold is broken and this agent produces no file, the run records an agent build failure instead of failing loudly on the dataset defect, so gold validity still depends on agent output. Run the gold before this branch.
        if not generated_query:
            logger.warning(f"Agent produced no {GENERATED_QUERY_FILE} for {context.entry.instance_id}")
            self.save_result(context, ExecutionBasedEvaluationResult.create_build_failure(context, output="", error_message=f"No {GENERATED_QUERY_FILE} produced"))
            return

docs/data-query.md:8

  • This says there is no live server in the loop, but the next paragraph and execution section require a running BC container and query its OData server. Clarify that there is no MCP or external production service; otherwise the category's runtime requirements are documented inaccurately.
This category benchmarks an agent's ability to **generate Business Central AL queries** from a natural-language data question — an offline query-generation benchmark. There is **no MCP server and no live server in the loop**: the agent writes an AL query, and the query is evaluated deterministically.
  • Files reviewed: 28/28 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.


return summary.model_copy(
update={
"total": total,

@haoranpb Sun Haoran (haoranpb) Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[recommendation] I think this is a no-op here

)


def execute_al_query(query_text: str, container: ContainerConfig, version: str, work_root: Path, suffix: str) -> list[dict]:

@haoranpb Sun Haoran (haoranpb) Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[recommendation] suffix should have a stronger type, so it's enforced to have value generated or gold. Alternatively, make it a boolean => serialize to $0$ and $1$

Comment thread src/bcbench/operations/bc_operations.py Outdated
Comment on lines +21 to +46
def _normalize_value(value: object) -> str:
if value is None:
return ""
if isinstance(value, bool):
return str(value).lower()
if isinstance(value, (int, float, Decimal)):
# Only values that arrived as numeric JSON types are canonicalized: scale/trailing-zero-
# insensitive (500 == 500.0) with full precision preserved (1.00001 != 1.00002) and no float
# rounding (Decimal built from the value's string form). Both gold and generated rows come
# through the same OData->JSON pipeline, so amounts are numbers on both sides.
try:
return str(Decimal(str(value)).normalize())
except (InvalidOperation, ValueError):
return str(value)
# Strings (and anything else) are preserved verbatim apart from a whitespace trim. Business Central
# Code/No. fields are JSON strings even when digit-only, so "001" must NOT collapse to "1" — coercing
# them through Decimal would let a wrong result be scored as matching the gold.
return str(value).strip()


def _normalize_rows(rows: Sequence[Mapping[str, object]], ordered: bool) -> list[tuple[str, ...]]:
# Compare on values only: drop OData/system metadata keys ('@'-prefixed) and ignore column
# names/order so a correct query still matches the gold even if it names columns differently.
normalized = [tuple(sorted(_normalize_value(v) for k, v in row.items() if not k.startswith("@"))) for row in rows]
return normalized if ordered else sorted(normalized)

@haoranpb Sun Haoran (haoranpb) Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[recommendation] consider extracting the normalization here into a dedicated function, it will be useful to future categories as well

Comment thread src/bcbench/evaluate/dataquery.py Outdated
Comment on lines +91 to +95
# Validate the gold query first, and deliberately do NOT catch its failure: a gold that doesn't
# compile/run is a harness or dataset bug, not the agent's fault, so it must fail the run loudly
# and get fixed rather than being silently scored or excluded.
gold_rows = execute_al_query(context.entry.gold_query, container, version, context.repo_path, "gold")

@haoranpb Sun Haoran (haoranpb) Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[critical] You might want to check if gold_rows is empty, if so, fail the evaluation => requires some design on the dataset.

If the gold_rows is empty, the generated query only need to return nothing.

$$pair = "$$($$cred.UserName):$$($$cred.GetNetworkCredential().Password)"
$$headers = @{ Authorization = 'Basic ' + [Convert]::ToBase64String([Text.Encoding]::ASCII.GetBytes($$pair)) }
$$base = 'http://localhost:7048/BC/api'
$$companyId = (Invoke-RestMethod -Uri "$$base/v2.0/companies" -Headers $$headers).value[0].id

@haoranpb Sun Haoran (haoranpb) Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[recommendation] We are taking an inexplicit dependency on the index 0 company here, not sure how that is resvoled.

You will have an easier time to hardcode a company name that we know will exist.


We could also pin the company name into the dataset, so each entry has to specify the company name. Together with pinned version, this should provide a more deterministic runs.

Sun Haoran (haoranpb) and others added 12 commits August 11, 2026 10:57
…uyuka/data-query-category

# Conflicts:
#	src/bcbench/dataset/__init__.py
#	tests/conftest.py
Give the data-query agent a feedback loop by exposing the Business Central MCP
server (and, separately, the Microsoft Learn MCP) as opt-in capabilities, and swap
in the bc-al-query-mcp skill that drives them.

- AL install app (scripts/al/mcp-config-setup): published at container setup, it
  provisions and activates the 'BCBench' MCP configuration the agent connects to.
  Idempotent by name; app/platform version injected at publish time.
- Setup (Setup-ContainerAndRepository.ps1, BCContainerManagement.psm1): for
  data-query, publish the app, resolve the container IP endpoint + evaluation
  company, and export BC_MCP_URL / BC_MCP_COMPANY to the agent step.
- mcp.py: http servers can now carry headers; --bc-mcp and --ms-learn-mcp are
  independent toggles. The BC MCP server's url + Basic auth + ConfigurationName +
  Company headers are filled from the container connection env vars.
- --bc-mcp / --ms-learn-mcp threaded through evaluate + run (copilot & claude) and
  both evaluation workflows (dispatch inputs + requeue).
- Skill: replace al-query-authoring with bc-al-query-mcp (grounds AL syntax in
  Microsoft Learn, validates via the BC MCP tools before writing query.al).
- Dataset: environment_setup_version 28.3 -> 29.0 (the MCP config API the install
  app relies on is not present before 29.0).

Which tools the BC MCP server exposes is decided server-side by the install app,
so the harness stays capability-agnostic.

Container round-trip (endpoint reachability, Basic auth on /mcp, app compile on
v29) needs a runner shakeout; not testable locally.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: db34a2a0-7035-4361-b911-becb72f86e21
Front-load the runner-only unknowns so one shakeout run classifies any failure:

- Write-BCMCPDiagnostics: host-side probe of the container from the setup step,
  exactly as the agent will connect. GETs the companies API (reachability + Basic
  auth) and POSTs MCP initialize + tools/list (endpoint + config + exposed tools).
  Never throws; output lands in the always-visible setup log.
- Upload **/*.log alongside *.jsonl so the agent's MCP client debug logs survive.
- RUNNER_DEBUG=1 in both eval workflows -> Python DEBUG + PowerShell compile output.

Also a permanent fix (keep): redact the Authorization header in build_mcp_config's
DEBUG dump so verbose logs never leak container credentials.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: db34a2a0-7035-4361-b911-becb72f86e21
…rge)

Data Query tools only exist in BC 29, which is not GA on the public artifact
feed yet, so resolve the sandbox artifact from bcinsider (with insider EULA
acceptance) and pass accept_insiderEula when building the container. Throwaway
alongside the BC MCP diagnostics.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: db34a2a0-7035-4361-b911-becb72f86e21
Compile-AppInBcContainer only accepts a project folder that is shared with the
container; the app was being built under TEMP, which is not mounted, so publish
failed with 'appProjectFolder ... is not shared with the container'. Build under
BuildRoot (\, the mounted folder) like the query harness does, and clean
it up after publish so nothing leaks into the agent workspace.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: db34a2a0-7035-4361-b911-becb72f86e21
…ion canary)

The query-generation contract let the model write a correct AL query from memory,
so the BC MCP feedback loop was never exercised (verified in run 32511827133: 4/4
resolved but zero bc_data_query tool calls). Ask for the actual DATA instead:

- Prompt: retrieve the real data with the BC data tools and write the rows to
  answer.json, plus the query used to query.al. The answer can't be fabricated, so
  a correct result requires genuinely querying the environment.
- evaluate: run the gold query for the expected rows and compare them to the agent's
  answer.json rows (result_sets_match). No longer compiles/runs the agent's query;
  query.al is kept only as an inspection artifact. Gold still fails loud.
- _load_answer_rows tolerates a bare array, a single object, or an OData value wrapper.

TEMPORARY (revert before merge): a canary block in the prompt asks the agent to try
reading ../dataset/dataquery.jsonl and `git show HEAD:dataset/dataquery.jsonl` and
report to canary.txt, so we can settle from the logs whether the agent can reach the
gold (the cwd sits inside the BC-Bench checkout whose root holds the dataset).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: db34a2a0-7035-4361-b911-becb72f86e21
1. Remove the shakeout diagnostics (kept only the v29 insider hack): drop
   Write-BCMCPDiagnostics + its setup call, RUNNER_DEBUG=1, the *.log artifact
   upload, and the canary block in the data-query prompt. The auth-header
   redaction and the shared-folder app fix stay.
2. Bake gold data: DataQueryEntry gains gold_rows (precomputed expected rows).
   evaluate compares the agent's answer.json to gold_rows and only falls back to
   running the gold query live when they are not baked. New
   `bcbench dataset bake-dataquery-gold` command + bake-dataquery-gold.yml
   workflow populate gold_rows once against a container and commit them, so
   evals stop recompiling/publishing the gold query per run (the source of the
   300s query-gold timeouts on insider 29).
3. Bump build_app timeout 300 -> 500s (headroom for the live/bake path).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: db34a2a0-7035-4361-b911-becb72f86e21
tool_usage was always None for data-query: the pre-tool-use hook never sees
sub-agent or MCP tool calls (the agent delegates BC data work to a task subagent,
where bc_data_query runs), so nothing was logged. Parse tool usage from the
authoritative copilot --output-format=json stream instead, counting
tool.execution_start events by toolName (with lsp:<operation> sub-labels for
parity). Verified locally that a subagent's inner tool call surfaces in the same
stream alongside the task delegation. The hook remains a fallback when the stream
carries no tool events.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: db34a2a0-7035-4361-b911-becb72f86e21
The data-query agent was reaching the real data by shelling out (260 powershell
calls vs 1 bc_data_query in the full run), reading BC_SERVER_*/BC_MCP_* from its
environment and hitting BC's API directly -- bypassing the MCP server the benchmark
is meant to exercise. Launch both CLI agents with agent_subprocess_env(), which
drops BC_SERVER_*, BC_MCP_* and BC_CONTAINER_NAME from the process environment. MCP
servers are unaffected: altool receives credentials through its MCP-config env block
and the BC MCP server through its auth header, so connectivity via the MCP path is
preserved while the direct-API side-door is closed.

Also temporarily raise build_app 500 -> 900s: live gold compile/publish still blew
500s on 2/11 insider-29 entries. This is a stopgap until gold data is baked.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: db34a2a0-7035-4361-b911-becb72f86e21
…efore merge)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: db34a2a0-7035-4361-b911-becb72f86e21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants