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).

• get_data_quality_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, mean and 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.
• get_data_quality_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/422/500 and transport failures to a status/message/guidance triple rather than a Go error.

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.

Rollout gating

Both tools are gated behind the data-quality experimental feature flag, off by default, per §3 of docs/TOOL_CONTRIBUTION_STANDARDS.md — §3.2's one-shared-feature-name-per-domain. The flag was previously removed in DEV-215225; this PR reintroduces it as the DQ domain name. Wiring: DataQualityFeature const → knownExperimentalFeatures → a single gated block in RegisterAll → both-direction tests in register_test.go.

Scope is deliberately narrow: the flag gates the tools added in this PR and its sibling #134, not the DQ tools already shipping ungated on main. Bringing those behind the flag would remove them from existing deployments, which wants its own decision.

Review fixes in this PR

• §2 data privacy. min/max were declared without a type restriction, so on a text column they returned a customer's cell value verbatim. They are now returned for numeric columns only — matching how mean/median/q1/q3 were already scoped — with a mixed inferredType ("String, Double") treated as non-numeric. Engine failure text is truncated before it reaches the model, on both the error paths and the custom-monitor exception field, since the DQ client wraps the entire non-2xx body and JDBC errors echo the offending value.
• §6.6. Added 422 arms to both tools (previously fell to default, which told the agent to retry a request that cannot succeed) and a 200-with-parse-error arm (previously rendered as (HTTP 200)).
• §7. Both descriptions now name the tools they're confused with, state that run_id comes from dq_search_job_runs, state read-only plus the required permission, gloss "job"/"job run"/"dataset"/"monitor", and add vague example prompts. Field-level: monitor state values now say which mean failure, monitorType records that its values aren't uniformly separated, summary.total warns the state counts don't sum to it, score states its direction, and runDate documents its format.
• The --help text and the README's known-experimental-features list now describe the flag accurately.

Deferred, with reasoning: observedValue/expectedMin/expectedMax carry the same §2 exposure for the MIN VALUE/MAX/MEAN monitor types, but get_dq_job_run already ships all three ungated on main. Narrowing them here would be a breaking change to a shipped contract while leaving the wider exposure in place, so this wants a maintainer decision on scope rather than a unilateral fix in this PR.

Still open from review: §9 — Permissions: []string{} on both tools needs the real scope identifiers from the DQ team.

Impact Analysis

Low, and lower than before the gating. Purely additive: two new read-only tools, one new client function, and one gated block in RegisterAll. No existing tool, client function or type is modified.

Because both tools sit behind data-quality (off by default), they are invisible unless an operator opts in — so there is no change at all to any existing deployment's tool list. chip-service sets COLLIBRA_MCP_EXPERIMENTAL from env and would need data-quality added there to expose them.

Both tools are ReadOnlyHint: true / DestructiveHint: false and perform a single GET each — no writes, no confirm checkpoint.

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.

Verified with gofmt -l, go build ./..., go vet and go test ./pkg/tools/... ./cmd/chip/... — all passing, including new tests for the privacy scoping (text / mixed-type / numeric columns), the 422 mapping, and error truncation.

✅ 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.

[regmimridul]

Thanks — worked through all of these. Every point was valid; nothing to push back on. Pushed as f6aa1fc.

§2 — min/max. Fixed. They're now returned for numeric columns only, matching how mean/median/q1/q3 were already scoped on the same struct, rather than dropped outright — the numeric case is a genuine statistic and the text case was the leak. A mixed inferredType counts as non-numeric: the existing amount test fixture is "String, Double", so one member is text and the extremes may be too. Three tests cover text / mixed / numeric.

§2 — exception and the error paths. Fixed. Both are truncated at 200 chars before reaching the model. You're right that the client wrapping the whole non-2xx body made this unbounded rather than metadata. An absent exception stays absent, so a passing monitor can't gain one — there's a test for that specifically, since that was the easy way to get this wrong.

§2 — observedValue/expectedMin/expectedMax. Not changed here, and I want to be explicit about why rather than quietly skip it. You noted get_dq_job_run/tool.go:47-49 already ships all three ungated on main. Narrowing them in this PR would break a shipped contract while leaving the wider exposure in place — the inconsistent half-fix. It needs a decision on the whole surface. Happy to do it here if you'd rather; otherwise it wants a ticket covering both tools.

§6.6 — no 422. Fixed in both tools. Also fixed the second case you found: code=200 with a parse error rendered as (HTTP 200), which the agent can't act on — that now has its own arm and doesn't print a status code.

§7 — descriptions. Rewrote both. They now name the confusable neighbours (all three taking the same run_id), state run_id comes from dq_search_job_runs, state read-only and the required permission, gloss "job"/"job run"/"dataset"/"monitor", and add vague prompts alongside the explicit ones. Field-level: the eight states now say which mean failure and which don't, monitorType records that its separators aren't uniform, summary.total warns the state counts don't sum to it, score states its direction, runDate documents its format, and the string-typed statistics say they're strings.

Flag --help text. Fixed — it describes the actual gate now. For context on why it over-claimed: I wrote it generically so the sibling PR wouldn't have to touch the same line. That was the wrong trade, since --help is what an operator reads to decide whether to enable it. #134 now updates it when it adds the template tools to the gate.

README known-features list. Added.

Stale PR description. Rewritten against this head, with the correct tool names and the gating described accurately.

§9 permissions — still open, agreed. I don't have the scope identifiers either and would rather leave Permissions: []string{} visible than guess.

On the flag scope generally: #134 previously extended data-quality to all 23 DQ tools. That's been reverted — the flag now gates only the 5 tools these two PRs add. Bringing the already-shipping DQ tools behind it would remove them from existing deployments, which is a separate decision.