refactor: extract BIGQUERY into feature/bigquery (part of #778)

[apstndb]

Part of #778 (PR1 in the migration plan). First extraction on top of the Feature seam (#779) and the def-table reorder (#780).

Scope

Move the BIGQUERY statement family out of core internal/mycli into the new internal/mycli/feature/bigquery package (cloud dep aliased as bq), so core no longer imports cloud.google.com/go/bigquery.

• Statements + classifier + formatter moved: BigQueryStatement, the fix: honor BigQuery safety contracts #775 fail-closed first-keyword classifier (bigQueryStatementMutates and helpers), and formatBigQueryValue. The statement embeds mycli.MutationClassifier (Classify wired in the constructor over its own SQL) and mycli.MarksDetachedCompatible; the ConditionallyMutatingStatement / DetachedCompatible compile-time assertions move with it. The core assertion for BigQueryStatement is removed.
• Def moved into Feature(). Each Feature() call allocates a fresh feature-owned config captured by the def closure, the Variable handlers, and constructed statements — no package-level mutable state (session-isolation commitment from the refactor: introduce the Feature registration seam (part of #778) #779 review thread).
• System variables CLI_BIGQUERY_PROJECT / CLI_BIGQUERY_LOCATION / CLI_BIGQUERY_MAX_BYTES_BILLED become FeatureVars with their descriptions moved verbatim; the nullable-int NULL display for MAX_BYTES_BILLED is preserved via mycli.NullableIntVar. The BigQuery* fields are removed from FeatureVars. The CLI_BIGQUERY_PROJECT -> CLI_PROJECT fallback stays in client-build code (via Session.ProjectID()).
• Client cache reimplemented on mycli.FeatureState (key "bigquery.client") using Session.AuthOptions. It preserves the rebuild-on-project/location-change rule and closes the client at Session.Close via io.Closer. The old deliberately lock-free field cache is replaced by a mutex-guarded cache (atomic check-then-act, per styleguide Concurrency).

Credential re-homing (deletes #775 carry-over machinery)

The raw --credential bytes get a durable home on the startup-config group (StartupConfig.Credential, read-only, not a registered variable), exposed via Session.CredentialBytes() (defensive copy). Because startup config is shared across USE/DETACH session replacement, the credential resolves the same way regardless of session switches by construction. This lets the PR delete:

• SessionHandler.credential, applyBigQueryCredential, newSessionHandlerWithCredential (restoring the simple NewSessionHandler);
• the session.bqCredential assignment in createSession, plus the bqCredential/bqClient/bqClientKey fields, bigQueryClientKey, bigQueryClient(), createBigQueryClientOptions, bigQueryProject(), and the bqClient close block in Session.Close.

The #775 credential-handoff regression is preserved, adapted to the new accessor (TestSessionCredentialBytesFromDurableConfig).

Note: the long-dead Cli.Credential field referenced in #778 section 4.6 no longer exists in the current tree, so there was nothing to delete there (see Deviations).

Order-contract change + disclosed one-time diff

Features append after the core table, so extracting BIGQUERY first moves its statement-help row after CQL's. The final registration order is fixed as (llm, cql, bigquery) (not the pre-extraction core order llm, bigquery, cql), and all.All()'s doc comment is updated accordingly. This front-loads the single 2-row swap (BIGQUERY and CQL) into this PR; PR2 (CQL) and PR3 (LLM) re-append at the same relative positions and stay byte-identical.

• docs/system_variables.md: byte-identical (variables are listed alphabetically; ListVariableInfo now also enumerates feature varDefs and renderSystemVariablesHelp is threaded with the feature set, so the generated reference keeps documenting the full binary's surface).
• README.md: exactly the disclosed BIGQUERY/CQL statement-help row swap, nothing else.

Tests

• Feature package: classifier table, formatBigQueryValue, def dispatch via mycli.BuildStatementWithDefs, FeatureVar Get/Set semantics (incl. NULL display), fresh-per-Feature() isolation, and the project fallback.
• Dispatch-level READONLY guard (fix: honor BigQuery safety contracts #775 regression) moves to an external mycli_test that registers bigquery.Feature() and drives the real guard through Session.ExecuteStatement (both blocks-mutating and allows-read-only, the latter proving the SELECT reaches Execute and fails on config, not READONLY). A small test-only bridge (export_test.go) exposes a READONLY test session and the READONLY-error predicate without widening the production API.
• The Statement-layer hardening: registry invariant tests and memefish-lexer argument parsing #728 invariant tests now iterate MergedStatementDefs(all.All()...), so they keep covering BIGQUERY after extraction.
• createBigQueryClientOptions' emulator-auth-skip coverage is preserved as a core test over the exported Session.AuthOptions(..., false).

Validation

• make check (test + lint + fmt-check): pass
• make check-race: pass
• go vet ./...: clean
• make docs-update then git diff: docs/system_variables.md byte-identical; README.md shows only the BIGQUERY/CQL 2-row swap
• Dep hygiene: go list -deps ./internal/mycli | grep cloud.google.com/go/bigquery -> empty (root main still imports it via feature/all, which is correct)

Tracked debt

The typed-export limitation (BigQuery values export as display strings, not typed cells) is recorded in the feature package doc comment.

Deviations from the spec

• The Cli.Credential field named in Quarantine optional statement families (GEMINI/CQL/BIGQUERY) behind an explicit Feature seam and ship a slim build variant #778 section 4.6 for deletion is not present in the current tree (already removed in an earlier change); no code change was needed for it.
• Two changes beyond the literal file list were required to hold the byte-identity gate, both consequences of moving the vars to feature registration: ListVariableInfo now iterates feature varDefs in addition to the core varDefs table, and renderSystemVariablesHelp is threaded with the feature set. Without these, the three BigQuery rows would drop out of docs/system_variables.md and HELP VARIABLES in the full binary.

Part of #778.

🤖 Generated with Claude Code

https://claude.ai/code/session_01V28qKRVwUCkpT5pepTksgr

[github-actions]

Code Metrics Report

📊 View detailed coverage report (available for 7 days)

	main (3c53157)	#781 (569be50)	+/-
Coverage	73.2%	73.3%	+0.0%
Code to Test Ratio	1:1.3	1:1.3	+0.0
Test Execution Time	1m38s	1m40s	+2s

Details

  |                     | main (3c53157) | #781 (569be50) |  +/-  |
  |---------------------|----------------|----------------|-------|
+ | Coverage            |          73.2% |          73.3% | +0.0% |
  |   Files             |             89 |             91 |    +2 |
  |   Lines             |           7907 |           7917 |   +10 |
+ |   Covered           |           5793 |           5805 |   +12 |
+ | Code to Test Ratio  |          1:1.3 |          1:1.3 |  +0.0 |
  |   Code              |          18426 |          18473 |   +47 |
+ |   Test              |          24867 |          25062 |  +195 |
- | Test Execution Time |          1m38s |          1m40s |   +2s |

Code coverage of files in pull request scope (70.6% → 70.8%)

Files	Coverage	+/-	Status
internal/mycli/app.go	40.1%	+0.3%	modified
internal/mycli/cli.go	60.7%	-0.1%	modified
internal/mycli/client_side_statement_def.go	86.1%	-0.1%	modified
internal/mycli/feature.go	90.6%	+3.9%	modified
internal/mycli/feature/all/all.go	100.0%	+100.0%	modified
internal/mycli/feature/bigquery/bigquery.go	22.6%	+22.6%	added
internal/mycli/feature/bigquery/classifier.go	100.0%	+100.0%	added
internal/mycli/feature/bigquery/format.go	40.7%	+40.7%	added
internal/mycli/session.go	69.6%	+3.7%	modified
internal/mycli/statement_processing.go	90.6%	0.0%	modified
internal/mycli/statements.go	56.9%	0.0%	modified
internal/mycli/statements_bigquery.go	0.0%	-40.0%	removed
internal/mycli/system_variables.go	95.2%	0.0%	modified
internal/mycli/var_defs.go	95.0%	-0.1%	modified
internal/mycli/var_registry.go	93.6%	+1.2%	modified

---

Reported by octocov

[gemini-code-assist]

Code Review

This pull request extracts the BIGQUERY statement family and its associated configuration variables from the core mycli package into an optional feature package under internal/mycli/feature/bigquery. It introduces a feature registration seam to allow optional statement families to register their own statement definitions and system variables. To support this, the raw credential bytes are now stored in the durable StartupConfig so that feature packages can securely retrieve them via Session.CredentialBytes() across session replacements. Additionally, tests for the BigQuery read-only guard, formatting, and variable resolution have been moved and adapted to the new package structure. No review comments were provided, so there is no feedback to address.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[apstndb]

REQUEST-CHANGES: I found one blocking regression.

Finding:

• [P2] Feature variables disappear from SHOW VARIABLES and variable-name completion. ListVariableInfo was updated to include r.sv.featureVarDefs, so HELP VARIABLES and generated docs still see CLI_BIGQUERY_PROJECT, CLI_BIGQUERY_LOCATION, and CLI_BIGQUERY_MAX_BYTES_BILLED. But VarRegistry.ListVariables still iterates only the core varDefs table. SHOW VARIABLES calls session.systemVariables.ListVariables(), and fuzzy variable-name completion uses the same map, so after this extraction the full binary can SET/SHOW VARIABLE the BigQuery vars by exact name but no longer lists them in SHOW VARIABLES or completion. Those rows existed before the move because they were in core varDefs. Please make the variable enumeration path iterate feature var defs as well (and the analogous ListMultiValues path for consistency/future feature vars), then add regression coverage for SHOW VARIABLES or the registry listing with bigquery.Feature().

References:

• internal/mycli/var_registry.go:205-214
• internal/mycli/statements_system_variable.go:46-52
• internal/mycli/fuzzy_finder.go:846-853

Validation:

• Clean merge into current origin/main (3c5315727cf246c7e1c23533cb0b65342f788d07) in a temporary clone.
• go test ./internal/mycli ./internal/mycli/feature/bigquery -run 'TestFeature|TestBigQuery|TestFormatBigQueryValue|TestReadOnlyGuard.*BigQuery|TestClientSideStatementDefsNonShadowing|TestClientSideStatementDefsCompletionConsistency'
• go list -deps -f '{{if eq .ImportPath "cloud.google.com/go/bigquery"}}{{.ImportPath}}{{end}}' ./internal/mycli produced no output, so the core package dependency split holds.

[apstndb]

Addressed the REQUEST-CHANGES finding in abba75f.

• [P2] Feature variables missing from SHOW VARIABLES / completion: fixed at the root rather than per-method — ListVariables, ListMultiValues, and ListVariableInfo now all iterate through a single VarRegistry.forEachDef helper (core varDefs followed by sv.featureVarDefs), so no enumeration surface can silently drift from the others again. Regression coverage added in both suggested forms: TestVariableListingIncludesBigQueryVars (external package, real bigquery.Feature() registered, asserts the exact map SHOW VARIABLES and fuzzy completion consume) and TestFeatureVarsAppearInEnumerationSurfaces (internal, pins the mechanism for single-value and multi-value feature vars).
• Also folded in two non-blocking findings from the independent adversarial review: the stale statements.go comment pointing at the deleted statements_bigquery.go, and MutationClassifier now fails closed (classifies as mutating) on a nil Classify instead of panicking on a zero-value statement, with a test.

Validation: make check pass, make docs-update still produces zero diff (README/docs byte-identity holds), CI running.

[apstndb]

APPROVE: previous blocking finding is resolved; I found no new blocking issues.

Reviewed the delta from b6cdb60 to abba75f. The feature-variable listing fix routes ListVariables, ListMultiValues, and ListVariableInfo through the same core+feature def iterator, and the added tests cover both the mechanism and the real BIGQUERY feature variables behind SHOW VARIABLES / completion.

Validation:

• Clean merge into current origin/main (3c5315727cf246c7e1c23533cb0b65342f788d07) in a temporary clone.
• go test ./internal/mycli ./internal/mycli/feature/bigquery -run 'TestVariableListingIncludesBigQueryVars|TestFeatureVarsAppearInEnumerationSurfaces|TestMutationClassifierDelegates|TestFeature|TestBigQuery|TestFormatBigQueryValue|TestReadOnlyGuard.*BigQuery|TestClientSideStatementDefsNonShadowing|TestClientSideStatementDefsCompletionConsistency'
• go list -deps -f '{{if eq .ImportPath "cloud.google.com/go/bigquery"}}{{.ImportPath}}{{end}}' ./internal/mycli produced no output.