[Bug] main is red since #444: ruff F811 plus a test_grafana_issue_440_vector_matching regression

[shmsr]

Summary

main currently fails both Ruff and the unit suite. Bisecting the recent merges
points at #444 (8612df20), and four PRs have merged on top since (#445,
#449, #450, #451), so this has been red for a while.

Two separate problems, and they are independent — fixing one does not fix
the other:

1. Ruff F811 — _strip_promql_comments is both imported into panels.py
   and defined locally in it.
2. test_grafana_issue_440_vector_matching fails (2 subtests) — the native
   eligibility gate now accepts an expression the Native PROMQL: support on()/ignoring()/group_* when the target supports vector matching #440 test expects it to
   decline.

I found this because an unrelated PR (#454) inherited the failure; I have not
changed anything here, because fixing (2) needs a call from the #440/#443
author about which behaviour is correct (see below).

Source type

• Grafana
• Datadog
• Docs / repo structure

Reproduction

git worktree add --detach /tmp/chk origin/main
cd /tmp/chk
python -m ruff check .
python -m pytest tests/test_grafana_issue_440_vector_matching.py -q

Bisect over the merges (each in its own clean worktree, same ruff 0.15.11):

commit	merge	#440 subtest failures	ruff
a6772e20	#442 native-promql-vector-matching	0	clean
8612df20	#444 fix/443-promql-comment-truncation	2	1 error
4dee8954	#445 promql-vector-comparison-filter	2	1 error
cff3472b	#449 otel-collector 0.149.0	2	1 error
da10d746	#450 named seriesOverride stack groups	2	1 error
05b26e0a	#451 counter pin gauge disclosure	2	1 error

Expected behavior

main passes ruff check . and the unit suite.

Actual behavior

F811 Redefinition of unused `_strip_promql_comments` from line 94
    --> observability_migration/adapters/source/grafana/panels.py:1556:5

tests/test_grafana_issue_440_vector_matching.py::NativeEligibilityGateTests::
  test_a_comment_between_the_matcher_and_its_parenthesis_does_not_hide_it
>   self.assertFalse(can_use_native_promql(expr, runtime_features=CAPABLE))
E   AssertionError: True is not false

Notes for whoever picks this up

The two are unrelated. I tried both directions of the duplicate definition
on a clean origin/main worktree:

change	ruff	#443 tests	#440 tests
remove the import at panels.py:94	clean	pass	still fail
remove the local def at panels.py:1556	clean	pass	still fail

So F811 is cosmetic — the local def shadows the import, and the two
implementations behave the same for this case. The #440 regression comes from
elsewhere in #444's changes (69 new lines in promql.py).

The #440 failure is a genuine judgement call, which is why I did not
"fix" it. Its test asserts that on # note\n(device) makes
can_use_native_promql return False even for a capable target, reasoning in
its own comment that _clean_promql_for_native flattens the expression and
would fold the operand after the comment into it. #444's whole purpose was to
strip comments before structure is read — after which on # note\n(device)
becomes on \n(device) and flattens to valid on (device).

Both readings are defensible and they imply opposite fixes:

• the gate is now right → #440's test and its rationale are stale and
  should be updated;
• the test is still right → fix: strip PromQL comments before they are read as query structure #444 strips comments too early on the native
  path, and a capable target can now be handed a native PROMQL query that was
  previously (deliberately) kept on ES|QL.

Guessing wrong either suppresses a real test or emits broken native PROMQL, so
this wants the author of #440/#443 rather than a drive-by fix.

Possibly worth a separate look: four PRs merged onto a red main. If
required status checks are not enforced on main, that is arguably the more
important fix.

Commands and logs

See Reproduction above.

Environment

• Python: 3.11 (CI) / 3.12 (local); reproduced on both
• Kibana / Elasticsearch: n/a — offline lint and unit suite only
• Input fixture: n/a

[shmsr]

Data point for whoever picks this up: the F811 and the test_grafana_issue_440_vector_matching failures are two separate problems. The duplicate definition is tempting to read as the cause, since the shadowed symbol is the one the failing test exercises. It is not.

panels.py imports _strip_promql_comments from promql.py at line 94, then redefines it at line 1556, so the local definition wins for every use inside panels.py. But the two implementations are behaviourally identical: fuzzing both over 20,000 randomly assembled inputs (comment markers, all three quote styles, embedded # inside label values, escaped newlines) produced 0 differing outputs.

differing inputs: 0 / 20000

So removing the duplicate is a clean lint-only fix and will not move the test. The assertFalse(can_use_native_promql(expr, runtime_features=CAPABLE)) failure needs its own look — comment stripping is not where it goes wrong.

Both were introduced by the same author's two commits (35e941cb, f76255b6), both already on main.

Noticed while running the gates for #454, which inherits both failures and nothing else.