fix(transformer): flip coerced boolean negations to equality

What does this MR do and why?

transform_expression flipped a negated boolean into an equality (!= true → = false) only when the parsed value was literally Value::Bool. BooleanLike also admits 1, 0, null and the quoted forms, but those are coerced further down the same match — past the flip — so they kept !=, and codegen routed them into not: { … }.

The fix extends the flip to every boolean-like field, as an arm below the literal-Bool one rather than in place of it, so fields that are not boolean-like keep their current handling of a literal true/false — match arms are tried in order, so a literal boolean never reaches the new arm:

(_, NotEqual, Bool(b)) => (f, Equal, Bool(!b)),
(_, NotEqual, _) if field_type.is_boolean_like() => (f, Equal, Bool(!is_truthy(&v))),

This differs slightly from the patch proposed in the issue: truthy() only ever returns Bool(true) or Bool(false), so that version's other => (f, o, other) branch is unreachable — the same kind of dead arm raised in !518 (merged)'s review — and one expression covers it.

The blast radius is wider than the issue describes

The issue notes that standard-mode sources "have the same asymmetry but get away with it because their GraphQL fields accept not". They do not. I enumerated every boolean-like filter field from the analyzers themselves — 15 of them across 5 source/mode pairs — compiled one negated coerced query per field, and validated each against GitLab's schema with graphql-inspector:

Source / field Before the fix Schema verdict before
Analytics.mergeRequests — createdByDuo not: {createdByDuo: [true]} Unknown argument not on field Analytics.mergeRequests
Namespace.projects — includeSubgroups, includeArchived, archivedOnly, hasVulnerabilities, hasCodeCoverage, issuesEnabled, mergeRequestsEnabled not: {…} Unknown argument not on field Namespace.projects. Did you mean sort?
mergeRequests — draft, subscribed, includeSubgroups not: {draft: true} etc. Field draft is not defined by type MergeRequestsResolverNegatedParams
workItems — confidential, subscribed, includeSubgroups not: {confidential: true} etc. Field confidential is not defined by type NegatedWorkItemFilterInput
Project.jobs — withArtifacts not: {withArtifacts: true} Project.jobs takes no not argument (schema-checked)

All 14 documents that compile were invalid before this MR; all 14 are valid after. (jobs needs a project scope, so it is verified against the schema's argument list rather than in the same batch.) MergeRequestsResolverNegatedParams and NegatedWorkItemFilterInput define no boolean fields at all, and Namespace.projects and Project.jobs have no not argument — so every boolean-like field was affected, not just the analytics one that surfaced it.

This also settles what looked like a semantics question. subscribed maps false to an enum token, so the shape changes from not: {subscribed: EXPLICITLY_SUBSCRIBED} to subscribed: EXPLICITLY_UNSUBSCRIBED, which are not logically equivalent — but the former was rejected by the API, so there is no result set anyone could have depended on. Same for the other 13. Nothing here can regress a working query.

Delta, measured rather than remembered

Produced by removing only the new arm and re-running the same probe, for every boolean-like field. Representative rows:

Filter Before After
createdByDuo != true / != false createdByDuo: [false] / [true] unchanged
createdByDuo != 1 not: {createdByDuo: [true]} createdByDuo: [false]
createdByDuo != null not: {createdByDuo: [false]} createdByDuo: [true]
draft != 1 not: {draft: true} draft: false
confidential != 1 not: {confidential: true} confidential: false
subscribed != 1 not: {subscribed: EXPLICITLY_SUBSCRIBED} subscribed: EXPLICITLY_UNSUBSCRIBED
includeSubgroups != 1 (work items) not: {includeDescendants: true} includeDescendants: false
archivedOnly != 1 not: {archivedOnly: true} archivedOnly: false
withArtifacts != 1 not: {withArtifacts: true} withArtifacts: false
every = spelling, and every literal != true / != false — unchanged

Each coerced spelling now agrees with its literal equivalent: includeSubgroups != 1, != true and = false all compile to includeDescendants: false.

Tests

No test covered != on any boolean-like field except draft != true, which is why this survived. Added:

  • MergeRequests analytics — 7 rstest cases for the coerced spellings on createdByDuo, each also asserting no not: appears anywhere in the compiled query.
  • MergeRequests standard — draft != 1, != 0, != "false".
  • Work items — subscribed != 1, the enum-mapped case that renders a token rather than a bool.
  • Projects — archivedOnly across all four spellings, on a source with no not argument whatsoever.

One note against the issue text

createdByDuo != "0" compiles to [false], not [true]. I wrote the test expecting [true] from the BooleanLike description, which advertises "0" as an accepted spelling, and it failed: is_falsey special-cases the quoted form "false" but not "0", so quoted "0" is truthy — createdByDuo = "0" already means true on main. Pre-existing, neither caused nor cured here; pinned as current behaviour with a comment, since changing is_falsey would alter = semantics for every boolean field on every source. If the docstring is the wrong side, that is a separate MR.

schema.json is also unaffected — no analyzer declaration moves, and regenerating produces an empty diff.

Example Usage

mode: analytics
query: type = MergeRequest and project = "gitlab-org/gitlab" and createdByDuo != 1
dimensions: createdByDuo
metrics: totalCount, acceptanceRate

Generated GraphQL on this branch — the filter reaches the engine as a value it accepts, where on main the same config produces mergeRequests(not: {createdByDuo: [true]}) and the API rejects it:

query GLQL($before: String, $after: String, $limit: Int) {
  project(fullPath: "gitlab-org/gitlab") {
    analytics {
      mergeRequests(createdByDuo: [false]) {
        aggregated(before: $before, after: $after, first: $limit) {
          count
          nodes {
            dimensions {
              createdByDuo
            }
            totalCount
            acceptanceRate
          }
        }
      }
    }
  }
}
Test via ruby extension
cd glql_rb
bundle install
bundle exec rake compile

bundle exec ruby -Ilib - <<'RUBY'
require "gitlab_query_language"
require "json"
require "net/http"

query   = "type = MergeRequest and project = \"gitlab-org/gitlab\" and createdByDuo != 1"
context = { mode: "analytics", dimensions: "createdByDuo", metrics: "totalCount, acceptanceRate" }

compiled = Glql.compile(query, context)
puts "== Generated GraphQL =="
puts compiled["output"]

uri  = URI("https://gitlab.com/api/graphql")
http = Net::HTTP.new(uri.host, uri.port); http.use_ssl = true
req  = Net::HTTP::Post.new(uri, "Content-Type" => "application/json")
# Required: analytics needs read_cycle_analytics, even for a public project.
req["PRIVATE-TOKEN"] = ENV.fetch("GITLAB_TOKEN")
req.body = { query: compiled["output"], variables: { limit: 3 } }.to_json
response = JSON.parse(http.request(req).body)

transformed = Glql.transform(response["data"], { mode: "analytics", fields: compiled["fields"] })
puts "\n== Transformed rows =="
puts JSON.pretty_generate(transformed["data"])
RUBY

Rows are unchanged by this MR — it alters only the filter argument the query is built with, not the response shape or the transform.

How to set up and validate locally

  1. Check out the branch and build:

    git checkout bug/219-coerced-boolean-negation
    cargo build
  2. Run the new coverage:

    cargo test --test merge_request_analytics_tests coerced
    cargo test --test merge_requests_tests draft_not_equals
    cargo test --test work_items_tests subscribed_not_equals
    cargo test --test project_tests archived_only_not_equals
  3. Confirm the whole suite and the lints:

    cargo test
    cargo clippy --all-targets -- -D warnings
    cargo fmt --check
  4. Confirm schema.json does not move:

    cargo run --bin generate-schema && npm run lint:prettier:fix
    git diff --stat src/schema/schema.json   # expected: no output
  5. Validate every compiled query against GitLab's real schema:

    DUMP_GRAPHQL=1 cargo test -- --test-threads=1 && npm run test:graphql

    1571 documents, all valid. The --test-threads=1 keeps #217 (closed) — a variable-name race under concurrent compiles — out of the corpus; without it two unrelated work-item documents can fail.

Edited by Chandra Saripaka

Merge request reports

Loading
Loading