fix(grafana): make YAML->NDJSON compile opt-in on native upload (#279)

[giorgi-imerlishvili-elastic]

Issue

Fixes #279 — the Grafana migration unconditionally compiled YAML → NDJSON via kb-dashboard-cli before every upload, even though the default native Dashboards API upload never consumes that NDJSON.

Was the issue valid?

Yes. The default native Dashboards API upload maps YAML / NativeDashboard IR straight to API panels and never reads the compiled NDJSON. Dashboards the typed API rejects already recompile lazily inside the legacy fallback (adapter._native_upload_file). So the pre-upload [5/7] compile was pure overhead on the default path. The unified CLI additionally hard-coded --compile for Datadog, contradicting the Datadog dedicated CLI, which had already made compilation opt-in.

Root cause

• observability_migration/adapters/source/grafana/cli.py ran the [5/7] Compiling YAML -> Kibana NDJSON step with no gate.
• observability_migration/app/cli.py forwarded --compile inconsistently across sources (hard-coded for Datadog).

Related code paths checked

• targets/kibana/adapter.py — native upload + per-dashboard legacy compile fallback (_native_upload_file / _legacy_upload_file).
• targets/kibana/compile.py — compile_all / layout validation.
• adapters/source/grafana/cli.py, adapters/source/datadog/cli.py (the reference opt-in implementation), app/cli.py.
• core/reporting/report.py, core/reporting/summary_md.py, adapters/source/datadog/report.py — compile status reporting.
• NDJSON consumers: parity-rig/verifier T3 layer, parity-rig/upload-all.sh, scripts/run_migration.sh, and the local render-audit flow (already --upload with no --compile).

Fix implemented

• Grafana CLI: added --compile / --no-compile; the compile step now runs only when --compile is set explicitly, or when --upload --legacy-import is used (that path imports the compiled saved objects and auto-enables compile). The default native path skips compilation and uploads directly.
• Unified obs-migrate migrate: forwards --compile only when explicitly supplied, for both Grafana and Datadog (removes the hard-coded Datadog --compile); still forwards --legacy-import so the dedicated CLIs auto-enable compile for legacy import.
• Reporting: no longer prints the misleading Compilation results: 0/0 line; Markdown summaries render compilation not run (instead of 0/0 compiled) and do not emit a false "Blocking errors" compile verdict when compilation was skipped; per-dashboard stdout uses — instead of ? for not-run; detailed report gains compile_attempted / compiled_total so automation can distinguish "skipped" from "compiled and failed".
• Scripts: scripts/run_migration.sh and parity-rig/upload-all.sh now pass --compile explicitly because they consume the compiled NDJSON.
• Docs: command-contract.md, architecture.md, dashboards/README.md, targets/kibana.md, sources/grafana.md, sources/datadog.md updated so compilation is documented as opt-in; YAML lint always runs, compiled-layout validation only runs when compilation runs.

Side effects considered

• Reporting fields: skipped compile now maps to not_run, never fail; the Datadog Markdown summary no longer shows a false blocking verdict on default no-compile runs.
• Parity-rig T3 verifier and the run-migration pipeline still get NDJSON via the added explicit --compile.
• CLI parity between Grafana and Datadog (both dedicated and unified) is now consistent.
• Scope intentionally excludes retiring obs-migrate compile / the YAML→NDJSON path (tracked separately in Retire the bespoke YAML->NDJSON compile path once native Dashboards API coverage is proven #279); this is the incremental "make it lazy" step.

Tests run

• New/updated regression tests: Grafana lazy-compile flag matrix (test_native_upload_default_skips_yaml_compile, --compile-without-upload, --upload --legacy-import auto-compile), unified --compile forwarding for both sources (incl. Datadog default-omit), parse_args defaults, report guards (compilation not run, compile_attempted), Datadog default no-compile summary, and doc-contract guards.
• pytest tests/ --ignore=tests/e2e/ — pass (100%).
• ruff check . — pass. mypy — pass. Source-header + skill-mirror/structure checks — pass.

Kibana visual verification

Ran the default command (--upload --ensure-data-views, no --compile) against a local stack:

• Console showed [5/7] ... skipped (native Dashboards API upload consumes YAML directly...).
• Upload results: 1/1 dashboards uploaded successfully; dashboards/compiled/ contained 0 files.
• The migrated "obs-migrate canary (kitchen sink)" dashboard rendered in Kibana with all panels/layout intact (the timeseries and barchart panels showed data; other panels were empty only because telemetry seeding was skipped in that run — a data-readiness artifact, unrelated to this change).

[giorgi-imerlishvili-elastic]

I don't think this is ready to approve yet; two items need addressing:

1. observability_migration/app/cli.py appends --compile twice for the unified Grafana path when the flag is set (the new block around line 906 duplicates the existing block after --validate). The dedicated parser currently tolerates duplicate store_true flags, but the wrapper is forwarding the wrong argv and the regression is hidden because test_run_grafana_migration_forwards_compile_flag only uses assertIn. Please remove one append and tighten the test to assert exactly one occurrence (or the complete expected argv).

2. The canonical docs/command-contract.md still says at lines 1393-1394 that dashboard YAML lint and compiled-layout validation run automatically inside every migrate. With this change, default migration runs lint, but compiled-layout validation only runs when compilation is requested (--compile or --upload --legacy-import). Please update that statement to match the new behavior and the updated dashboards README.

The rest of the lazy-compile path looks consistent: Datadog no longer receives an unconditional --compile, skipped compilation is represented as not run in the updated summaries, and the NDJSON-consuming scripts opt in explicitly. CI is green, but it does not catch the duplicate because of the membership-only assertion above.

[giorgi-imerlishvili-elastic]

Re-reviewed after b430ddc. Both prior findings are resolved: Grafana now forwards --compile exactly once with a regression assertion, and docs/command-contract.md correctly limits compiled-layout validation to compile-enabled runs. I found no remaining issues in the full PR, and all current CI checks pass. This is ready for approval by another reviewer (GitHub does not allow the PR author's account to submit an approval).

[shmsr]

Thanks for the fix — the core reasoning is correct and the test coverage is solid.

One thing worth flagging before this merges: the YAML→NDJSON compile path (kb-dashboard-cli compile step, --compile/--no-compile flags, compiled/compile_error/compiled_path result fields, obs-migrate compile subcommand) is being retired in favour of the native Dashboard-as-Code path (Dashboards API, *.native.json artifacts). The behavioral fix here (making compile opt-in rather than unconditional) already landed on main via the 217967b docs/CLI alignment commit, so the default-path regression is already addressed for operators today.

That means this PR's remaining value is the test coverage and reporting cleanup — both worthwhile — but the --compile additions to scripts/run_migration.sh and parity-rig/upload-all.sh will need to come back out when the compile path is cut (tracked in #279). The T3 verifier in the parity-rig will also need to move from NDJSON diffing to native artifact comparison before that removal is safe.

Recommendation: merge this as-is for the reporting/test fixes, but treat the --compile in those two scripts as temporary scaffolding pending the full compile-path removal in #279. Tagging this for follow-up there.