fix(release): drupal.org blockers + live-tested bugs + tool-use defaults

[steveworley]

Summary

Clears drupal.org release blockers + three bugs surfaced during live runtime testing against the QuantGov Dashboard API. Follow-up to #8 (settings-form accessibility rewrite) — opens cleanly on top of 1.x post-#8 merge (rebase pre-verified, zero conflicts).

What's fixed

Provider contract

• LSP violation on isUsable() and getConfiguredModels() — add the array type hint required by AiProviderInterface. Fatal under PHP 8.x strict mode.
• Add use ChatTrait; so cost-tracking + tagged-chat helpers don't fail at runtime.

Live-tested bugs (probed against https://dash.quantgov.cloud)

• Structured output payload shape: responseFormat (camelCase) + type: "json" was rejected with 400 on every call. Now sends response_format (snake) + type: "json_schema" + flat schema / name / strict. Verified across all three call paths: buffered chat, streaming chat, and the deprecated chatStream().
• VDB upload response parser: server returns uploadedDocuments[*].documentIds, not top-level documentIds. Without this fix the vdb_mapping state was silently never populated. Lookup-by-Drupal-ID flows now work.
• Token-leak in error logs: AuthService was logging the first 500 chars of OAuth response bodies, and QuantCloudClient was logging Guzzle exception messages (which include the full request payload). Both now log status + reason phrase only. Verified across 30 watchdog rows post-fix: zero token strings leak.

Tool-use friendly default tokens

• Bump default max_tokens 16384 → 32768 across the client constant, install config, and api_defaults. PR Clean up settings form: accessibility, routing, and render patterns #8 set 16384, but Anthropic best practice for tool use is 8–16K per round; multi-round chains (Canvas AI orchestrator, agent loops) easily reach 30K. 32K gives 2× headroom over PR Clean up settings form: accessibility, routing, and render patterns #8 without hitting the form ceiling (65536 stays as the upper limit).
• Add update_10003 hook that bumps existing sites from 16384 → 32768 (only if their value is ≤16384, so custom higher values are preserved). Idempotent — live-verified with `drush updb -y`.

Packaging

• `LICENSE.txt` (GPL-2.0-or-later, matches `composer.json`).
• `.gitattributes` (export-ignore for tests/, .docker/, etc.).
• `.gitignore` (vendor/, .idea/, .vscode/, .claude/, .DS_Store).
• `composer.json`: minimum-stability stable, drupal/ai ^1.2.0, drupal/search_api ^1.36 (the VDB submodule's info.yml declared the dep but composer didn't).
• `info.yml`: `package: 'AI Providers'`, `lifecycle: experimental` + `lifecycle_link`.

Coding standards

• `phpcbf --standard=Drupal,DrupalPractice` swept the module: 389 of 394 errors auto-fixed. Remaining 8 errors + 29 warnings are line-too-long / deprecation-tag format / doc-comment-missing — scoped for a follow-up.

Test plan

Live runtime checks (drush ev, against QuantGov):

• Buffered chat (Nova Lite + Claude Haiku 4.5)
• Streamed chat / SSE iterator (38 deltas reassembled)
• Embeddings (Titan v2 1024d, Cohere v3 English + Multilingual)
• Multimodal (PNG describe via Claude + Nova Pro)
• Tool use (clean schema → correct toolUseId + input)
• Structured output (Nova Lite returns raw JSON; Claude Haiku wraps in fences — model-side compliance issue, not provider)
• OAuth refresh (forced expiry → /oauth/token round-trip, fresh 90-day expiry persisted)
• VDB CRUD (create / upload / queryByText / queryByVector / drop)
• Error paths (bad model → 400, bad token → 401, no leakage)
• `drush updb` runs `update_10003` cleanly: 16384 → 32768

Known follow-ups (Tier 2 / 3 — not in this PR)

• Move `AuthService::validateToken()` out of `buildForm()` (per-render network round-trip).
• Add `validateForm()` override that exercises the token at submit.
• Use `#type => 'key_select'` with `key_type` filter (currently lists all Key entities).
• Fix refresh-token lock race (second `acquire()` return not checked).
• `AuthService::writeKey()` delete-then-create is non-atomic.
• Hardcoded model→vector-dim map should fall back to API.
• Delete deprecated `QuantCloudStreamingClient::chatStream()`.

Upstream / server-side issues (out of scope here)

• `drupal/ai` `ToolsPropertyInput::renderPropertyArray()` emits non-standard JSON Schema fields (per-property `name`, boolean `required`) that Anthropic rejects. File upstream.
• Quant Dashboard `/oauth/token` returns the same access_token on every refresh — only `expires_in` is fresh. Worth confirming intentional vs. token-rotation being skipped.
• Quant Dashboard accepts `response_format` payloads but doesn't enforce the schema. Workaround: default `chat_with_structured_response` to Nova Lite, which complies natively.

🤖 Generated with Claude Code

[quantcode-agent]

PR #10 Review — fix(release): drupal.org blockers + live-tested bugs + tool-use defaults

Branch: fix/release-blockers → 1.x | Commit: 1cc7204

---

Summary of Changes

This PR bundles five distinct categories of change:

1. drupal.org release blockers — .gitattributes, lifecycle: experimental, package rename, LICENSE.txt, README.md, CHANGELOG.md
2. response_format key rename — responseFormat/jsonSchema → response_format/json_schema to match the actual Quant Cloud API contract
3. Tool-use response parsing fix — dual-path extraction for toolUse from both nested (response.toolUse) and flat (toolUse) Lambda response shapes
4. VDB document-ID path fix — uploadedDocuments[0].documentIds → corrected null-coalesce path
5. max_tokens bump — 16 384 → 32 768 via update_10003, plus a new logPotentialChatFailure() diagnostic helper
6. ChatTrait adoption — drops the manual $chatSystemRole property in favour of the base-class trait; isUsable() type-hint tightened to array; AiModelCapability enum used for capability comparison

---

Blockers

None. No exploitable security vulnerabilities, data-loss paths, or ISM control violations were introduced by this diff.

---

Warnings

W1 — QuantCloudStreamingClient import removed but class still injected at runtime
src/Plugin/AiProvider/QuantCloudProvider.php

The use statement for QuantCloudStreamingClient was removed from the imports, but the class is still referenced in the body via @var docblock and create() factory (using the FQCN string). PHP won't fatal at runtime, but removing the use statement while keeping the FQCN in a docblock is misleading and will cause IDE type-inference to break. If a future refactor adds a type-hint or instanceof check against the short class name, it will fail silently. The use statement should be restored, or the docblock updated consistently.

---

W2 — response_format.strict defaults to FALSE — may silently produce non-conforming JSON
src/Plugin/AiProvider/QuantCloudProvider.php (~line 287)

$options['response_format'] = [
    'type'   => 'json_schema',
    'schema' => $schema['schema'],
    'name'   => $schema['name'] ?? 'json_schema',
    'strict' => $schema['strict'] ?? FALSE,   // ← defaults to FALSE
];

When strict is FALSE the model is not required to conform to the supplied schema; it may return additional properties or omit required ones. For a government platform where downstream code deserialises the response into typed objects, a silent schema violation is a data-integrity risk. The safer default is TRUE, with callers explicitly opting out. At minimum, document why FALSE is the chosen default.

---

W3 — update_10003 silently skips all configs if listAll() returns nothing
ai_provider_quant_cloud.install

If listAll() returns an empty array (e.g. config stored in DB but config factory bootstrapped before DB is available during drush updb), the update silently succeeds without touching any config. There is no post-condition check or log message. Consider logging the count of updated configs, or asserting at least one was found.

---

W4 — logPotentialChatFailure() logs raw $content at WARNING level unconditionally
src/Plugin/AiProvider/QuantCloudProvider.php (~line 432–480)

if ($tools_requested && !$tool_use_data) {
    $this->logger->warning(
        'Tool-use requested but no tool call returned. ...',
        ['@content' => print_r($content, TRUE), ...]
    );
}

$content is the raw model response text. If the model returns a long conversational reply (valid when the model decides no tool is needed), the full text is dumped into Drupal watchdog at WARNING level. On a busy site this will bloat the watchdog table and may inadvertently log user-supplied content or PII that appeared in the conversation. Truncate $content to a safe length (e.g. 500 chars) before logging, and consider whether WARNING is the right severity for a valid "model chose not to call a tool" scenario (NOTICE may be more appropriate).

---

W5 — VDB document-ID path fix is correct but fallback is an empty array with no error signal
modules/ai_provider_quant_cloud_vdb/src/Plugin/VdbProvider/QuantCloudVdbProvider.php (line 448)

$document_ids = $response['uploadedDocuments'][0]['documentIds'] ?? [];

If the API returns an unexpected shape, $document_ids silently becomes [] and the caller receives no indication the upload may have failed. A log warning or exception when $document_ids is empty after a nominally successful upload would make failures observable.

---

Nits (non-blocking)

• ai_provider_quant_cloud.info.yml: lifecycle_link URL points to https://www.drupal.org/node/3369575 — worth verifying this is the correct node for the experimental lifecycle definition.
• QuantCloudProvider.php: Several inline comments still end without a full stop after this PR — minor consistency issue.
• ai_provider_quant_cloud.module: The hook_help() output still uses concatenated $output .= rather than a render array. Not introduced by this PR, but worth noting for future cleanup.
• CHANGELOG.md / README.md: Good additions for drupal.org release hygiene.

---

What's Done Well

• AiModelCapability enum adoption — comparing $capability->value against the API string avoids subtle type-mismatch bugs present in the base branch.
• ChatTrait adoption removes a hand-rolled $chatSystemRole property that duplicated base-class state.
• Dual-path toolUse extraction correctly handles both the nested Lambda wrapper format and flat format, with a clear comment explaining the two shapes.
• isUsable() type-hint tightened from untyped to array $capabilities — small but correct.
• .gitattributes correctly excludes dev artefacts from drupal.org release archives.
• update_10003 iterates all provider configs rather than hard-coding a single config name — correct for multi-instance setups.

---

Overall Risk Assessment

The three live-tested bug fixes (response_format key rename, toolUse dual-path extraction, VDB document-ID path) are all straightforward and correct. The drupal.org release-blocker changes are mechanical. The main risks are the silent failure modes in W3 and W5, the PII-in-logs risk in W4, and the misleading removed import in W1. None are merge-blocking on their own, but W4 (logging raw model content) is worth addressing before a public drupal.org release given the government/PROTECTED data context this module operates in.

---

Verdict: COMMENT — no blockers; the fixes are correct and the release-blocker changes are appropriate. Address W4 (truncate logged content) and W1 (restore the QuantCloudStreamingClient import) before the drupal.org release tag.