DEV-205661: Add dq_get_job_run_profile and dq_get_job_run_monitors tools

[regmimridul]

🎯 What does this PR do?

DEV-205661. Adds two read-only MCP tools over the public DQ job-run API, so an agent can inspect what a data-quality run actually observed without going through the job-details UI. Both are keyed by run_id (jobRunId).

• dq_get_job_run_profile — column-level profiling statistics for a run. New client function clients.GetDqJobRunProfile over GET /rest/dq/1.0/jobRuns/{jobRunId}/profile. Per column: the type declared by the source schema vs. the type inferred from the values, counts of values/nulls/empties/distinct values, min/max/mean, quartiles for numeric columns, and the top observed value shapes. Paginated (limit/offset, 100 per page, API caps at 500); the tool always requests includeTotal and returns a derived hasMore so a caller can tell whether to page.
• dq_get_job_run_monitors — per-monitor results for a run, over the existing clients.GetDqJobRunMonitors. Adaptive monitors report the observed value against the learned expected range plus that range's sensitivity tier; custom rules report score, breaking/passing row counts and tolerance. Adds a summary block counting monitors by state so a failing run can be triaged without walking both lists.

Both tools derive nullPercent/emptyPercent and the state counts rather than leaving the model to compute them, and both map HTTP 400/401/403/404/500 and transport failures to a status/message/guidance triple rather than a Go error, matching dq_get_job_run.

A run that produced no profile or no monitor results returns error with an explanation (the run did not complete, or profiling/monitors are not configured) instead of an empty success, and points at dq_get_job_run to check the run's status.

Two notes for review:

1. Overlap with dq_get_job_run. That tool already returns the adaptive + custom monitor breakdown alongside a run's lifecycle details. dq_get_job_run_monitors is the focused read for when only the monitors are wanted, and it additionally surfaces each monitor's tolerance — the threshold the observed value was judged against — which dq_get_job_run drops. Both tool descriptions cross-reference each other so the model can pick between them. Flagging in case you would rather fold tolerance into dq_get_job_run and drop the separate tool.
2. README wording. The existing dq_get_job / dq_get_job_run entries are still marked Experimental (data-quality feature flag), but that flag was removed in DEV-215225. The two new entries omit the marker since it no longer exists; the stale neighbouring entries are left alone as out of scope for this PR.

Impact Analysis

Low. Purely additive: two new read-only tools, one new client function, and two lines in RegisterAll. No existing tool, client function or type is modified, so there is no behaviour change for anything already shipped.

Both tools are ReadOnlyHint: true / DestructiveHint: false and perform a single GET each — no writes, no confirm checkpoint. They are registered ungated, consistent with the other DQ tools on main since the data-quality experimental flag was removed.

The profile endpoint is paginated and defaults to 100 columns; the tool passes includeTotal=true, which costs the DQ API one extra count query per call. Response size is bounded by the limit cap of 500 columns.

chip-service needs no change — it picks these up on the next chip module bump.

Verified with gofmt -l, go build ./..., go vet ./... and go test (the two new package test suites plus pkg/tools and pkg/clients, all passing). The existing pkg/tools annotation test covers both new tools.

✅ Checklist

• My code follows the style guidelines of this project.
• I have performed a self-review of my own code.
• I have commented my code, particularly in hard-to-understand areas.
• I have made corresponding changes to the documentation (if needed).
• My commit messages follow the Conventional Commits standard.

🤖 Generated with Claude Code

[regmimridul]

@EricWarnerCollibra I had connected with @aberkowCollibra on this PR. He had mentioned that there is going to be a new flag specifically for DQ related tools. Marking this as draft until we get a confirmation on that. Kindly review this PR and #134 once ready to go.

[aberkowCollibra]

Reviewed against docs/TOOL_CONTRIBUTION_STANDARDS.md as of this PR's head (4418835) — all 41 rules, in four passes over the section groups.

One blocking item: §2, the data-privacy hard rule. min/max on the profile tool are not scoped to numeric columns, so on a text column they return a real cell value to the LLM; observedValue and exception on the monitors tool are the same channel. Comments inline.

Also flagged: §6.6 (no 422 arm in either lookupError), §7 description gaps in both tools, and two mismatches around the new data-quality flag — the --help text says it gates DQ authoring tools when it gates only these two reads, and the README's known-features list doesn't mention it.

Passing, and worth saying: the flag mechanism itself is correctly wired end to end with both-direction tests. §3.1-3.4 all pass. §4.1 passes — this is the first DQ tool to actually spell out the abbreviation in the tool name. §5.3, §6.1, §6.2, §6.7, §8.1, §8.4, §10.3 pass. Validation precedes every network call and the URL is built with url.PathEscape + url.Values.

Three rules I couldn't reach a verdict on, rather than passing them silently: §1.2 (whether the monitors tool is inside DEV-205661's agreed surface — the PR body defers this itself); §8.2 (the structs cite dq-v1-public-oas-spec.yaml, which isn't in this repo and is referenced with no version or link, so five asserted behaviours — the includeTotal param, the 100/500 page limits, and two ordering guarantees the tool promises the model — can't be checked); §8.3 (a contract test on the DQ service side isn't visible from here).

§9 (personas and permissions) is held back deliberately — Permissions: []string{} on both tools needs the actual scope identifiers from the DQ team, and I'd rather leave it open than guess at values.

One housekeeping note: the PR description is stale against this head SHA. It says the tools "are registered ungated" and that the data-quality flag "no longer exists" — this SHA reintroduces the flag, gates both tools and marks both README entries Experimental. It also calls the tools dq_get_job_run_profile/dq_get_job_run_monitors, while the code registers get_data_quality_job_run_profile/get_data_quality_job_run_monitors. Anyone reviewing from the description will read §3 wrongly.

[aberkowCollibra]

§2 Data privacy — hard rule. min and max are declared as "Minimum value observed." / "Maximum value observed." with no restriction to numeric columns — note that mean, median, q1 and q3 on the following lines all carry "Numeric columns only." and these two do not.

So for a text column (customer_email, patient_name, account_number) these return one real customer's cell value, verbatim, to the LLM. §2 is unconditional:

No tool may return live customer data (actual rows, cell values, sample records, file contents) to the LLM.

Everything else in ColumnProfile is genuinely metadata and fine — the counts, the percentages, definedType/inferredType, and the masked topShapes patterns.

To satisfy the rule: drop min/max, or reduce them to a non-value signal (present/absent, in-range verdict). §2 also asks that this be raised with the CHIP maintainers before the code is written rather than caught in review, so worth a note on the ticket either way.

[aberkowCollibra]

§2 Data privacy — hard rule. observedValue is "The value this run actually observed." For the monitor types listed three lines up — MIN VALUE, and MAX/MEAN variants — that observed value is a cell value from the customer's table, not a count. expectedMin/expectedMax carry the same exposure via the learned range.

No tool may return live customer data (actual rows, cell values, sample records, file contents) to the LLM.

A verdict plus a deviation ("above the learned range") would carry the same triage signal without the value.

For context, not a request to fix it here: pkg/tools/get_dq_job_run/tool.go:47-49 already ships these three fields ungated on main, so the existing exposure is wider than this PR. Flagging so maintainers can scope it.

[aberkowCollibra]

§2 Data privacy — hard rule. exception passes the DQ engine's failure message through verbatim. Engine and JDBC errors routinely echo the offending value (invalid input syntax for integer: '...'), which makes this an unbounded channel for customer data rather than metadata.

Same shape in both tools' error paths: the %v on the client error reaches the model, and pkg/clients/dgc_client.go:245 builds that error as fmt.Errorf("HTTP %d: %s", response.StatusCode, string(responseBody)) — the entire non-2xx body.

Redacting or truncating both would close it.

[aberkowCollibra]

§6.6 Map downstream errors to structured statuses. The rule names four codes — "400 / 403 / 404 / 422 become typed outputs with readable messages." 422 has no arm here (the switch covers 404, 401, 403, 400), so it lands in default, whose guidance says "This is likely a server-side error. Retry shortly; if it persists, contact your Collibra administrator."

That is the opposite of true for an unprocessable entity, and it tells the agent to retry a request that will fail identically every time.

Second case reaching this branch: clients.GetDqJobRunProfile returns code=200 with a non-nil error when the JSON fails to parse (pkg/clients/dq_job_run_profile_client.go:88-90), which renders as (HTTP 200) — a status the agent can't act on.

case http.StatusUnprocessableEntity: with a "request was rejected as invalid, fix X" message, and routing the 2xx-parse-failure somewhere that doesn't print an HTTP code, would satisfy it.

[aberkowCollibra]

§6.6 Map downstream errors to structured statuses. Same gap as the profile tool: no 422 arm, so it falls to default and the agent is told to retry a request that cannot succeed. §6.6 names 422 explicitly alongside 400/403/404.

Worth noting the sibling get_dq_job_run/tool.go has the identical gap, so §6.5 (be consistent with sibling tools) pulls against §6.6 here. §6.6 still applies to a new tool — consistency shouldn't propagate the gap.

[aberkowCollibra]

§7 Tool descriptions. The paragraph bar (§7.1) is cleared, but three of the seven required coverage points are missing:

• Item 3, the tool it is most likely confused with — no neighbour is named. The two real confusions both take the same run_id: dq_get_job_run and the sibling monitors tool. (The PR body says "Both tool descriptions cross-reference each other" — only the monitors one does.)
• Item 4, prerequisites and ordering — nothing says where run_id comes from. dq_search_job_runs is the tool that supplies it and isn't named. It appears in the runtime Guidance strings, but the model doesn't see those when choosing a tool.
• Item 7, side effects and permissions — read-only isn't stated, and the required permission isn't either, though the 403 path at line 275 knows it. Read-only is asserted in the package comment and the README; neither is LLM-facing.

§7.2 — "job", "job run" and "dataset" are used unglossed. The standard's own worked example glosses exactly these terms for a model with zero Collibra knowledge.

§7.4 — three example prompts are present, but the rule's bolded "including vague ones" isn't met: all three hand over the run id. Something like "is there anything odd about the data in this run?" is what that clause is asking for.

§7.5 — runDate (line 82) has no description, format or timezone, and the client flattens upstream DqPublicRunDate{Kind, Value} to Value alone, discarding the kind discriminator that tells the model how to read the string. min/max/mean/median/q1/q3 are typed string without saying so, and min/max on a non-numeric column don't say whether the ordering is lexicographic or by inferred type.

[aberkowCollibra]

§7 Tool descriptions. §7.3 is clean — this description stands alone and the dq_get_job_run reference is disambiguation, not a dependency. Gaps:

§7.2 — "monitor" is the subject of the tool and is never defined. The description separates adaptive from custom monitors but never says a monitor is a single data-quality check on a table's data — which is precisely the gloss the standard's own example gives ("Collibra calls it a 'monitor'"). dimension is likewise Collibra-coded, described only as "Data quality dimensions the monitor contributes to" with no gloss and no value list. DQ is used as a bare abbreviation in LLM-facing prose here and on lines 91, 101-102, 128.

§7.4 — all three example prompts spell out the artifact and supply the run id; none is vague, which the rule asks for explicitly.

§7.5, concrete items:

• state (lines 61, 75) lists eight values with no meanings. USER_PASSED, STALE, SKIPPED, LEARNING aren't self-explanatory, and the model has no basis to decide whether LEARNING or SUPPRESSED counts as a failure.
• monitorType (line 59) mixes separators — MIN VALUE with a space, among ROW_COUNT, DATA_TYPE, SCHEMA_CHANGE. A model echoing one back can't tell which form is real.
• monitorSummary.total (line 88) is "Total monitor results for the run" while countState only counts PASSING/BREAKING/EXCEPTION. Nothing tells the model the remainder is a non-empty residue of other states, so "triage at a glance" invites total - (passing+breaking+exception) as a wrong inference.
• score (line 76) gives the range but not the direction — is 0 or 100 good? — nor what a "point" is.

Worth saying that tolerance at line 81 is the standard the rest should match: "count of breaking rows allowed before the rule is judged as failing" is exactly the disambiguation §7.5 asks for, and the two same-named tolerance fields are each distinguished.

[aberkowCollibra]

The flag description doesn't match what the flag gates. This says "Data quality authoring and job-run inspection tools", and it's what --help prints (via formatExperimentalForHelp → cmd/chip/config.go:168).

But data-quality gates exactly two tools — the two read tools added here, at pkg/tools/register.go:134-137. Every DQ authoring tool registers unconditionally at register.go:110-128: create_dq_job, create_dq_rule, deploy_dq_rule_template, update_dq_job, delete_dq_job, delete_dq_job_run, cancel_dq_job_run.

So --help tells an operator that enabling data-quality switches on DQ authoring. It doesn't, and those tools are already on whether the flag is set or not. Something like "Data quality job-run profile and monitor inspection tools" would describe the actual gate.

The wiring itself is correct, for the record: const → knownExperimentalFeatures → gate → both-direction tests in register_test.go:32-53, and all three input channels (--experimental, COLLIBRA_MCP_EXPERIMENTAL, mcp.experimental) resolve through the unchanged IsExperimentalEnabled. The defect is only in what it claims to cover.

[aberkowCollibra]

data-quality is missing from "Known experimental features". That section (README:241-247) is hand-written prose, not generated from knownExperimentalFeatures — it documents only context-specifications and skills. This PR introduces a third feature name and doesn't add it, so the README now under-documents the flag these two entries reference.

Related and worth a decision on this PR, though it didn't create it: lines 48-55 already mark eight DQ tools Experimental (data-quality feature flag) — create_data_quality_rule, deploy_data_quality_rule_template, dq_cancel_job_run, dq_delete_job, dq_delete_job_run, dq_update_job, dq_get_job, dq_get_job_run — while register.go:110-128 registers all eight ungated.

Before this PR those markers pointed at a flag that no longer existed, so they read as obviously stale. DataQualityFeature is new here (it isn't on main), which makes the name real again and the markers newly plausible — a reader now has no way to tell which of the ten marked tools are actually gated. Either correct the eight stale markers or gate those tools.