fix: preserve response projection semantics

[chaim0m]

Summary

• apply --exclude to response items while honoring explicitly named wrapper fields
• keep report schemas and positional rows aligned when columns are excluded
• preserve unrelated nested fields and pagination metadata unless explicitly named
• report each missing --fields value using matches collected by the real projection traversal
• cover list wrappers, report rows, partial typos, object rows, and empty responses

Why

Follow-up to Alfredo's review comment on #33. Recursive exclusion could remove pagination tokens and unrelated nested fields, while misspelled fields and positional report rows behaved silently or inconsistently.

Test methods

go test ./...
go vet ./...

Could this break things?

Risk: medium, intentional semantics correction. --exclude no longer removes matching keys recursively; it targets response items, report columns, and explicitly named wrapper fields. Missing projection fields now produce a stderr warning without changing output or exit status.

Jira

CMP-48644

[josuesilva-doit]

Understanding

The core fix is correct and addresses real data loss. Verified by building both branches:

--exclude rowCount on {"budgets":[...], "rowCount":1}
main:  {"budgets":[...]}                 <- pagination token destroyed
PR:    {"budgets":[...],"rowCount":1}    <- preserved

An agent paginating through results lost the cursor with no way to continue. Making --exclude mirror the projection paths is the right call, and the code reads better for it.

go vet ./... is clean and go test ./... passes (14s).

Three findings below, each verified empirically. Two of them I'd like addressed before merge; details are in the inline comments.

Issues Identified

1. --exclude is a silent no-op on the report (schema/rows) shape — high impact. Report rows are arrays of positional cells, not objects, so excludeObject returns them untouched. This is the DCI API's primary report shape, and extractGetReportRows (main.go:1964) already handles all three row variants exhaustively — object, positional cells, plus a defensive scalar fallback — feeding both toTableRows and toonPrepare. The projection side handles it too via projectSchemaRows. Exclusion is now the only stage of the output pipeline that doesn't convert cells. Because the shaped body flows into renderTable and dciToonContentType.Marshal, the excluded column keeps showing up in the default table output — this is a user-visible bug, not just a JSON concern.

2. Undocumented regression: excluding the list key itself no longer works. --exclude budgets used to drop the key; now it's ignored because listWrapperRows matches before the root is ever considered. Dropping the list to keep only pagination metadata is a plausible use, and this is a first-level field, so it's outside the depth-semantics change the description covers.

3. The --fields warning misses the case that motivated it. objectHasAnyField uses any-match semantics, so a single correct field silences the warning. The realistic typo — --fields id,amont — produces no warning at all and amont vanishes silently.

Technical Risks

Risk	Severity	Note
Exclusion silently ineffective on reports	High	Visible in default table output; users assume the field was dropped
Loss of --exclude on a first-level list key	Medium	Regression vs main, absent from the description
Breaking for callers pruning nested fields	Medium	Acknowledged in the description; heavy nested fields return to agent context
Scope wider than JSON	Medium	The shaped body reaches the table and TOON renderers, not just JSON output
Duplicated structural navigation	Medium	projectionFieldStatus reimplements projectNestedRows' traversal; finding 1 is that divergence already surfacing

Suggested Improvements

Blocking:

1. Handle positional cells in exclusion, mirroring projectSchemaRows.
2. Report the specific missing fields instead of all-or-nothing. This fixes finding 3 and removes code: projectionFieldStatus, projectionRowsFieldStatus and the (bool, bool) pair all become unnecessary, which also removes the duplicated traversal.

Non-blocking:

3. Check the root before branching into the wrapper (finding 2), or document the loss under risk.
4. Name the (bool, bool) return values — return true, false communicates nothing at the call site. comparable also shadows the Go 1.18+ predeclared identifier; it compiles, but it reads oddly.
5. Extend the --exclude help text (main.go:1359) to state the scope, e.g. "(applies to response items, not nested objects)".
6. Consider splitting the commits. A medium-risk data-loss fix and a low-risk UX feature in one commit make selective revert harder; #37 and #38 kept a tighter scope.

Missing test coverage for all three findings: exclusion on schema/rows, exclusion of the list key, and a partial --fields typo. Each is a case this PR makes relevant.

Overall Assessment

Two findings I would like resolved before merge, but the disagreement is narrow. The pagination fix is genuinely good — on its own it would go in today.

What holds it back is that the PR sets out to give --exclude the same structure as projection and stops short of the API's main response shape, with the gap visible in the default output format. And the warning, as written, stays quiet in exactly the scenario it was built for.

Suggestion 2 is the cheapest path: rewriting the warning to name missing fields deletes code, drops the duplicated traversal, and fixes the false negative in one go. Suggestion 1 is the real remaining work.

One question beyond this repo: is anyone relying on recursive --exclude internally? The new behavior is the more defensible one, but that's a contract call worth confirming before merge — and worth writing down, since this semantics has now shifted twice (#33, #39).

[apgiorgi]

Both problems this follows up on are real, and the wrapper-aware approach is the right direction. Three things need to change before this lands.

1. The new warning fires falsely.

projectionFieldStatus (output_contract.go:79-89) decides "did anything match" against the schema column names, but the actual projection (projectSchemaRows, output_contract.go:174-188) uses the row's own keys when a row is already an object. So the two disagree:

{"result":{"schema":[{"name":"colA"}],"rows":[{"service":"BQ"}]}}   --fields service

projects correctly to {"result":{"rows":[{"service":"BQ"}],...}} and prints warning: none of the requested fields exist in the response: service.

The point of this warning was to let an agent distinguish "no data" from "wrong field name". A diagnostic that contradicts the output it accompanies is worse than the silent no-op it replaced. Root cause is structural: projectionFieldStatus is a hand-mirrored second copy of projectResponseValue's traversal, so it will keep drifting out of sync. Please have the projection itself report what it matched rather than re-deriving it.

2. --exclude no longer reaches the result container — regression.

excludeNestedRows (output_contract.go:270-287) only rewrites container["rows"]. Comparing main against this branch on {"result":{"schema":[...],"rows":[["BQ",12.5]]}} with --exclude schema:

• main: {"result":{"rows":[...]}}
• branch: schema still present

Dropping the verbose schema block is a legitimate token-saving use in agent mode and it silently stops working here. Relatedly, excludeObject (output_contract.go:297) is inert on cell-array report rows, so --exclude does nothing at all to report data.

3. --exclude silently ignores keys the caller explicitly named.

The test at output_contract_test.go:55-84 asserts that --exclude rowCount keeps rowCount; same for nextPageToken. Given the flag is documented as "Comma-separated response fields to exclude" (main.go:1359), that makes it a no-op for a whole class of keys.

My objection was to recursing into nested structures like alertThresholds[].amount and to stripping unnamed pagination keys. Honoring a wrapper key the caller named explicitly satisfies that without the flag lying about what it does.

Nits

• comparable as a variable name (output_contract.go:17,105) shadows the predeclared constraint.
• Unnamed (bool, bool) returns are opaque at the call site.
• Missing tests: warning must not fire when fields match; the result.rows schema path; empty response producing no warning.
• --fields warns on no-match but --exclude stays silent — asymmetric. And the narrowed --exclude scope needs a help-text/README update.

CI is green and go vet ./... / go test ./... pass, so none of the above is caught by the current suite.

[chaim0m]

Addressed the blocking findings and nits in 5c707bd/b5f90fd. Report exclusions now filter positional cells and schema together; explicit wrapper/container fields are honored; projection reports each missing field from the actual traversal; object-row and empty responses no longer produce false warnings. Added focused regression coverage, updated help/risk documentation, and replied to each inline thread. go test ./... and go vet ./... pass.

[chaim0m]

All requested projection and exclusion changes remain present after merging current main; the branch is conflict-free. The focused suite and go vet ./... pass, and all inline threads are resolved. Re-requesting review on the current head.