lisa/feat/bch-1311-suggestions-api

[ccf-lisa]

automated implementation by lisa.

[gusfcarvalho]

Findings inline (1 Medium, 4 Low). Build/vet/gofmt clean; suggestions, filter, and evidence integration suites all pass locally; both prior self-review findings (empty-scope 422, ForControl response shape) are addressed and tested; the /filters auth tightening is intentional and covered.

Headline: no blockers — the one that matters is the worker not registered path mapping to HTTP 500 instead of 503/501 (every generate 500s once AI is enabled pre-Phase-5). The rest are a count(DISTINCT uuid) redundancy question, an HTTP-layer coverage gap on Accept/Reject/supersede, a forward-looking transactional-enqueue note, and a confirm-intended on Update clearing the SSP binding.

[gusfcarvalho]

Re-reviewed 6aa0aba. All prior threads addressed and verified: not-registered → 503 + rollback (sentinel added, tested); filter Update full-replacement documented in code + OpenAPI nullable annotation; and solid new HTTP coverage for Accept (filter create + control links + accepted events), Reject, List/Events SSP-scoping, and supersede-only-in-scope. Build/vet/gofmt clean, oscal integration suite green locally.

One non-blocking Low follow-up inline: the not-registered fix didn't extend to the worker's disabled sentinel, which still maps to 500 on an AI-enabled-but-worker-disabled misconfig. Not a blocker — flagging COMMENT, not requesting changes.

[gusfcarvalho]

Approving 8f6aa5f. All review findings addressed and verified:

• not-registered → 503 (sentinel + rollback test)
• disabled sentinel → 503 — the residual from last round, now matched via errors.Is(err, workersvc.ErrDashboardSuggestionWorkerDisabled) with a dedicated 503/rollback test
• filter Update full-replacement documented in code + OpenAPI nullable annotation
• HTTP coverage added for Accept (filter create + control links + accepted events), Reject, List/Events SSP-scoping, and supersede-only-in-scope

The two count(DISTINCT uuid) threads were withdrawn (design intent acknowledged; no-op at those sites but harmless defense-in-depth). Local build/vet/gofmt clean, oscal integration suite green, and CI is fully green (unit, integration, lint, check-diff). Nice work.

[ccf-lisa]

PR approved. Marking as ready for e2e.

[gusfcarvalho]

REQUEST_CHANGES — add a read-only pre-run estimate for planned AI calls

The UI shows a confirmation like "This will make N AI calls covering X controls × Y label-sets" before a run. Today there's no API to get the real N ahead of time, so the UI falls back to X × Y (one call per cell) — e.g. 16 × 47 = 752 — which is wildly over-stated. With the default 40 / 200 chunking that same scope actually plans 1 call: ceil(16/40) × ceil(47/200) = 1.

There is currently no read-only path to the real number:

• GET .../dashboard-suggestions/config returns only { enabled } (dashboard_suggestions.go:100) — no chunk sizes, so the UI can't run the ceil math itself.
• POST .../dashboard-suggestions/generate does compute the correct plannedCalls (dashboard_suggestions.go:154), but only as a side-effect of creating a run, superseding pending suggestions, and enqueuing cell jobs — not usable as an estimate.

Requested change: add a dry-run/preview endpoint, e.g.

POST /oscal/system-security-plans/{id}/dashboard-suggestions/preview   (read-only, no persistence)

that:

1. resolves the scope via svc.ResolveScope(sspID, scopeFromRequest(req.Scope)), and
2. returns { plannedCalls, controlCount, labelSetCount } using the existing suggestionrel.PlannedCalls(len(snapshot.ControlKeys), len(snapshot.LabelSetHashes), h.chunkConfig()).

This is mostly a refactor: extract the resolve-and-plan block already in Generate (lines ~144–157) into a shared helper that both Generate and the new preview call, so the chunking formula stays a single source of truth on the server. The UI then shows an accurate estimate instead of the cartesian product.

Alternative considered & not preferred: expose maxControlsPerChunk / maxLabelSetsPerChunk on the /config response and let the UI compute the estimate. This duplicates the chunking formula client-side, and the UI still can't get the resolved control/label-set counts without a scope-resolution call anyway — so a server-side preview is cleaner.

[gusfcarvalho]

We need a way to show the user an accurate pre-run estimate of planned AI calls. See the inline thread for the requested change.

[gusfcarvalho]

Re-review of the new Preview endpoint (db05e75, 26e439a). The endpoint is otherwise clean — good shared planDashboardSuggestions helper (single source of truth for the chunking formula), correct read-only semantics (no run/cells/events/enqueue, asserted), AI-gated auth, swagger regenerated, and a non-vacuous test. Build/vet/gofmt clean; lint/unit/check-diff green on CI.

One actionable item inline: Preview ignores the maxCallsPerRun cap that Generate enforces, so it returns a 200 "valid" preview for plans Generate will reject with 422 — the preview needs to fail or signal not-allowed for over-limit scopes.

[gusfcarvalho]

Approving 946ec9d. The Preview limit finding is fully addressed and verified: dashboardSuggestionPreviewResponse now carries maxCallsPerRun + exceedsLimit (exceedsLimit = maxCalls > 0 && plannedCalls > maxCalls), so the UI can show an over-limit plan as not-allowed without a second round-trip, and Generate keeps its 422 unchanged. New test covers over-limit (exceedsLimit=true), at-limit boundary (false), and no-limit (false). Swagger regenerated; local build/vet/gofmt clean, oscal integration suite green, and CI fully green (unit, integration, lint, check-diff). Preview stays read-only and reuses the shared plan helper. LGTM.

[ccf-lisa]

PR approved. Marking as ready for e2e.