refactor: extract GEMINI/LLM into feature/llm (part of #778)

[apstndb]

Part of #778 (PR3 of the extraction series, after #779/#780/#781/#782).

Summary

• Move the GEMINI statement family into internal/mycli/feature/llm: the statement + two-phase compose flow, the tool-use Developer Knowledge client, the session-scoped doc cache (now a FeatureState holder, key llm.doccache, closed via io.Closer), and the embedded doc catalog.
• Core no longer imports google.golang.org/genai, github.com/apstndb/genaischema, github.com/apstndb/developerknowledge-go, or github.com/apstndb/spanner-docs-embed (go list -deps ./internal/mycli verified clean of all four; golang.org/x/time/rate remains only as a transitive dep of the Spanner client, with no direct core importer).
• feature/all now registers the final order (llm, cql, bigquery).

First use of the Flags/KongVars/ApplyFlags seam

• --vertexai-project/model/location move to Feature.Flags (kong.Plugins) with identical names/help/placeholders; Feature.KongVars supplies the default-model/location help interpolation.
• Flag application routes through the registry: applyFeatureFlags runs in initializeSystemVariables after the registry is available and before --set processing, so precedence is unchanged (flag < --set; unset pointer flags do not clobber TOML/--set values — Model/Location are pointer flags, Project mirrors the pre-extraction unconditional string assignment).
• CLI_VERTEXAI_* variables move to FeatureVars with verbatim descriptions and identical defaults; docs/system_variables.md is byte-identical.

Disclosed one-time README diff

kong.Plugins renders plugin-contributed flags at the end of the --help flag list, so the three --vertexai-* lines move from mid-list to the end of the regenerated readme-help block. The flag text itself is byte-identical (this is the mechanical proof that the KongVars interpolation reproduces kong's output). In the slim variant these lines will disappear entirely, so optional-feature flags listing last mirrors the statement-table layout established in #780. No other docs change.

Behavior preservation notes

• GeminiStatement keeps implementing no marker interfaces (not detached-compatible, not READONLY-classified) — unchanged from core.
• Doc cache semantics preserved: build-once per Session keyed on first-call apiKey (same as getOrCreateDocCache), env keys DEVELOPERKNOWLEDGE_API_KEY/GOOGLE_API_KEY read per Execute as before.
• Core test adjustments: the TOML nested-typo alias test now uses the [emulator] prefix (the table name must be a non-flag prefix of real flags — vertexai left core); CLI_VERTEXAI_* enumeration/default/Set-Get coverage moves to the external llm_variables_test.go with the real llm.Feature() registered.

Validation

• make check, make check-race, go vet ./...: pass
• make docs-update: docs/system_variables.md byte-identical; README diff is exactly the 3-line flag reposition described above
• Dep check: core import graph clean of the four LLM deps

🤖 Generated with Claude Code

https://claude.ai/code/session_01V28qKRVwUCkpT5pepTksgr

[github-actions]

Code Metrics Report

📊 View detailed coverage report (available for 7 days)

	main (53bd54f)	#783 (36d63dc)	+/-
Coverage	73.2%	73.1%	-0.1%
Code to Test Ratio	1:1.3	1:1.3	+0.0
Test Execution Time	1m57s	1m40s	-17s

Details

  |                     | main (53bd54f) | #783 (36d63dc) |  +/-  |
  |---------------------|----------------|----------------|-------|
- | Coverage            |          73.2% |          73.1% | -0.1% |
  |   Files             |             93 |             93 |     0 |
  |   Lines             |           7933 |           7943 |   +10 |
  |   Covered           |           5808 |           5808 |     0 |
+ | Code to Test Ratio  |          1:1.3 |          1:1.3 |  +0.0 |
  |   Code              |          18517 |          18568 |   +51 |
+ |   Test              |          25146 |          25230 |   +84 |
+ | Test Execution Time |          1m57s |          1m40s |  -17s |

Code coverage of files in pull request scope (78.3% → 77.9%)

Files	Coverage	+/-	Status
internal/mycli/app.go	40.1%	0.0%	modified
internal/mycli/client_side_statement_def.go	86.0%	-0.1%	modified
internal/mycli/config.go	89.3%	-0.3%	modified
internal/mycli/feature.go	92.1%	+1.5%	modified
internal/mycli/feature/all/all.go	100.0%	0.0%	modified
internal/mycli/feature/llm/doc_cache.go	90.8%	0.0%	renamed
internal/mycli/feature/llm/doc_embed.go	94.7%	0.0%	renamed
internal/mycli/feature/llm/llm.go	7.6%	+7.6%	added
internal/mycli/feature/llm/tooluse.go	90.0%	+90.0%	renamed
internal/mycli/session.go	74.1%	+4.3%	modified
internal/mycli/statements_llm.go	0.0%	-4.4%	removed
internal/mycli/statements_llm_tooluse.go	0.0%	-90.1%	affected
internal/mycli/system_variables.go	95.2%	0.0%	modified
internal/mycli/var_defs.go	94.9%	-0.1%	modified

---

Reported by octocov

[gemini-code-assist]

Code Review

This pull request extracts the GEMINI/LLM feature from the core internal/mycli package into a dedicated optional feature package under internal/mycli/feature/llm. This migration moves the associated CLI flags, system variables, and the session-scoped documentation cache to the new package, updating registration and tests accordingly. The review feedback highlights three critical issues in the newly added llm.go file: a recommendation to implement retry-on-error semantics for the lazy initialization of the feature state to avoid permanently poisoning the session on transient failures, and two potential runtime panics due to missing defensive nil checks on the LLM response (composed and composed.Statement) and on the tool-use candidate content (result.Candidates[0].Content).

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.

[gemini-code-assist]

Instead of adding defensive nil checks for the holder returned by mycli.FeatureState, implement retry-on-error semantics for the lazy initialization of the feature state. This ensures that transient initialization failures (such as a temporary network issue during client or cache creation) do not permanently poison the session, allowing subsequent requests to retry.

References

1. Implement retry-on-error semantics instead of sticky error caching for lazy initialization of feature states (e.g., FeatureState). This ensures that transient initialization failures do not permanently poison the session.

[gemini-code-assist]

The composed response or its nested Statement field can be nil if the LLM returns an unexpected response format or an error description. Accessing composed.Statement.Text directly without a nil check can lead to a runtime panic. Please add defensive nil checks for both composed and composed.Statement before accessing their properties.

if composed == nil {
	return nil, fmt.Errorf("GEMINI returned no response")
}
if composed.Statement == nil {
	if composed.ErrorDescription != "" {
		return nil, fmt.Errorf("GEMINI error: %s", composed.ErrorDescription)
	}
	return nil, fmt.Errorf("GEMINI returned no statement")
}

var rows []mycli.Row
if composed.ErrorDescription != "" {
	rows = append(rows, mycli.NewRow("errorDescription", composed.ErrorDescription))
}
rows = append(rows,
	mycli.NewRow("text", composed.Statement.Text),
	mycli.NewRow("semanticDescription", composed.Statement.SemanticDescription),
	mycli.NewRow("syntaxDescription", composed.Statement.SyntaxDescription),
)

[gemini-code-assist]

In the tool-use loop, result.Candidates[0].Content can be nil (for example, if the candidate was blocked or had other generation issues). Accessing result.Candidates[0].Content.Parts directly without a nil check can lead to a runtime panic. Please add a defensive nil check for result.Candidates[0].Content before accessing its parts.

Suggested change

	if len(result.Candidates) > 0 {
	history = append(history, &genai.Content{
	Role: genai.RoleModel,
	Parts: result.Candidates[0].Content.Parts,
	})
	}
	if len(result.Candidates) > 0 && result.Candidates[0].Content != nil {
	history = append(history, &genai.Content{
	Role: genai.RoleModel,
	Parts: result.Candidates[0].Content.Parts,
	})
	}

[apstndb]

Internal review results (orchestrator line review + independent adversarial review): both conclude SAFE TO MERGE.

Verified under attack:

• Flag semantics equivalence (the highest-risk area): TOML resolution through kong.Plugins proven empirically and pinned by the generic seam tests (scalar + pointer shapes); --set > flag ordering preserved (applyFeatureFlags runs before --set processing, matching the pre-extraction order); Project's unconditional set is byte-equivalent to the old unconditional assignment because the resolver fills plugin scalars identically; no env-tag drift (old flags had none).
• Moved code byte-identical modulo package/header (doc_cache, doc_embed, tooluse + tests); Execute equivalent (cfg-backed storage is the same storage the registry binds); doc-cache build-once-per-Session semantics preserved.
• docs/system_variables.md byte-identical; README diff is exactly the disclosed 3-line flag reposition (plugin flags render last in --help); enumeration surfaces covered with the real llm feature (the refactor: extract BIGQUERY into feature/bigquery (part of #778) #781 regression class).
• Core import graph clean of genai/genaischema/developerknowledge-go/spanner-docs-embed.

All four adversarial-review recommendations addressed in-review: merged origin/main (picks up the spannerplan v0.2.0 bump), relocated the dropped GEMINI dispatch test (asserts unquoted prompt + no marker interfaces on the dispatch-built statement), and fixed the two stale/overstated comments (main_flags_test.go coverage note, Feature.ApplyFlags doc).

Provenance note for the reviewer: the implementing subagent was interrupted repeatedly; the final ~15% (two failing-test fixes, the disclosed README reposition, the seam mechanism tests, and these follow-ups) was completed by the orchestrator, so the independent adversarial pass and maintainer review carry extra weight on this PR.

[apstndb]

REQUEST-CHANGES: one blocking nil-response path remains at current head 57bd0ca.

• [P2] internal/mycli/feature/llm/llm.go:246: genaischema.GenerateObjectContent[*output] can return a nil *output for JSON null, or an output with Statement == nil when the structured model response omits/nulls statement. Execute then dereferences composed.ErrorDescription, composed.Statement.Text, and composed.Statement.* while building rows and PreInput, so a malformed/partial model response can panic the CLI instead of returning a controlled error. Please guard both composed == nil and composed.Statement == nil before dereferencing; if ErrorDescription is present without a statement, return/report that error without touching Statement.

I checked the other unresolved retry-on-error thread: FeatureState already retries after init errors (TestFeatureStateRetriesAfterInitError), so I do not consider that one blocking. The tool-use Content nil thread is also not independently blocking from my read because FunctionCalls() returns nil when Content is nil and the loop breaks before appending result.Candidates[0].Content.Parts; adding an explicit nil guard there would still make the invariant clearer.

Validation:

• GOCACHE=/private/tmp/spanner-mycli-pr783-review.k2aNN8/gocache go test ./internal/mycli -run '^(TestFeatureFlagsResolveFromTOML|TestFeatureFlagsCLIOverridesTOML|TestFeatureFlagsUnsetPointerStaysNil|TestGeminiStatementDispatch|TestLLMVariables|TestMainFlagsVertexAI|TestSystemVariablesVertexAI)'
• GOCACHE=/private/tmp/spanner-mycli-pr783-review.k2aNN8/gocache go list -deps ./internal/mycli | rg 'google.golang.org/genai|github.com/apstndb/genaischema|github.com/apstndb/developerknowledge-go|github.com/apstndb/spanner-docs-embed'