# GCP TopSQL Logical Root and Discovery Diagnostics Repair

## Background

The GCP TopSQL Hive-source isolation change correctly models each TiDB or
TiKV instance as a separate Parquet source URI. Two defects remain in the
feature implementation currently exercised in production:

- `extractGCSLocation` removes the trailing slash, while
  `topSQLBaseDataSource` recognizes only component suffixes that retain that
  slash. A logical `topsql` request can therefore append a second
  `component=...` segment and reconcile empty external sources at the wrong
  root.
- GCS API failures and local source-layout validation failures are all exposed
  as `gcs_list_failed`. Structured TopSQL setup fields exist but are not added
  to the table-management warning, so operators cannot distinguish an IAM/API
  response from an invalid instance or date layout.

The source currently also rejects instance partition values containing `.` or
`:`, even though TiDB and TiKV instance identities commonly use DNS names and
ports.

## Goal

- Normalize logical TopSQL component roots independent of a trailing slash.
- Preserve exactly one component segment when resolving all four physical
  TopSQL dependencies.
- Accept bounded DNS/IP-and-port instance partition values while continuing to
  reject unsafe or structurally invalid GCS prefixes.
- Emit safe, trace-correlated setup operation, reason, error source, HTTP
  status, and bounded counts when GCP TopSQL table management fails.
- Distinguish local layout validation reasons from remote GCS/API failures.
- Validate the repaired discovery code through a read-only side-path smoke
  test using the existing production runtime identity, without deploying a new
  image or changing BigQuery/GCS objects.

## Current behavior

Logical requests may resolve a dependency below a path such as
`component=tidb/component=tidb`. Plain validation errors from prefix discovery
are wrapped as `gcs_list_failed`, and the service logs only the vendor at the
table-management boundary. The request-level timeout begins after table
management; this change does not alter that boundary.

## Target contract

- Component-root normalization accepts both `.../component=tidb` and
  `.../component=tidb/` and returns the same common TopSQL root.
- Physical dependency resolution produces one and only one of
  `component=tidb`, `component=tikv`, `component=topsql_sql_meta`, or
  `component=topsql_plan_meta`.
- Instance names accept letters, digits, `_`, `-`, `.`, and `:` with the
  existing 255-character bound. `/`, reserved empty sentinels, and unrelated
  child prefixes remain invalid.
- Local validation uses stable reasons such as `invalid_instance_layout`,
  `invalid_instance_name`, and `invalid_date_layout`.
- A Google API response retains its safe reason and HTTP status. ADC token
  retrieval and network failures use bounded `credential_error` and
  `network_error` categories. Failures that cannot be typed remain
  `gcs_list_failed` with no invented IAM conclusion.
- No credential, GCS URI, object name, SQL text, or customer identifier is
  written to logs or smoke-test output.

## Affected modules and routes

- Go module: `clinic_data_service`.
- Packages: `pkg/engine` and `pkg/service`.
- Executable: `clinic_data_service/cmd/api`.
- Image: `clinic/data-service`.
- Affected APIs: logical and physical GCP TopSQL SQL queries through canonical
  and legacy Data Proxy query routes.
- Deployment ownership: Clinic CD component `clinic-data-service`,
  app-delivery application `clinic-data-service`, Kubernetes workload
  `Deployment/clinic-data-service` in staging and prod-cloud.
- `docs/clinic-module-overview.md` does not require an update because module,
  executable, image, workflow, and app-delivery ownership do not change.

## Implementation steps

1. Normalize component suffixes after trimming trailing slashes and add focused
   table-manager tests for logical and physical roots.
2. Introduce bounded typed layout-validation errors and preserve their stable
   reason through TopSQL setup diagnostics.
3. Extend the instance-name validator for DNS/IP-and-port values and cover both
   accepted and rejected prefixes.
4. Preserve the existing `TOPSQL_BIGQUERY_SETUP_FAILED` structured logging
   path and verify that it receives the new stable layout reasons. The feature
   baseline already attaches `bigQueryTopSQLSetupFailureLogFields` inside
   `ensureBigQueryTablesExist`; no duplicate outer warning fields are needed.
5. Add an explicitly gated, read-only integration smoke test that discovers
   existing TopSQL external roots and calls the repaired source discovery using
   ambient ADC. It must never execute DDL or query customer rows.

## Exclusions

- No caching or background reconciliation optimization.
- No changes to Delta-versus-Parquet semantics, query field aliases, request
  timeout ownership, or public API responses.
- No app-delivery, IAM, GCS, BigQuery, staging, or production mutation.
- No source commit, push, pull request, merge, or workflow dispatch without
  separate authorization.

## Compatibility and safety

Trailing-slash normalization is backward compatible for physical and logical
TopSQL data-source URLs. The instance validator remains fail closed for path
separators and unexpected directory shapes. Diagnostics remain fixed enums and
counts only. The side-path smoke uses existing ADC and performs metadata and
prefix-list reads only.

## Test plan

From `clinic_data_service`:

```bash
go test ./pkg/engine -run 'TestTopSQLBaseDataSource|TestBigQueryTopSQLView|TestDiscoverTopSQL'
go test ./pkg/service -run 'TestBigQueryTopSQLSetup|TestBigQueryTableManagement'
go test ./pkg/engine ./pkg/service ./pkg/handlers
```

The gated side-path test will be compiled locally, copied temporarily to one
Ready production Data Service Pod, run with the Pod's existing ADC against
already referenced TopSQL external roots, and removed immediately afterward.
Expected evidence is safe target hashes, discovery counts, and typed failure
categories. It must not create or replace external tables or views.

## Staging verification and release boundary

This task stops after local tests and the read-only side-path smoke. It will not
dispatch Clinic CD and will not change staging or prod-cloud desired/live
state. A later staging deployment would require explicit authorization and
would use Clinic CD component `clinic-data-service`; production would require a
separate post-merge authorization and the normal prod-cloud review boundary.

## Observed validation

On 2026-08-17, focused and package-level engine, service, and handler tests
passed. A test binary built from this worktree was copied temporarily to one
Ready production Data Service Pod and run with that Pod's ambient ADC. The
read-only test found two existing TopSQL metric roots whose metadata contained
the duplicated component segment. After repairing that segment only inside the
test, the production GCS prefixes returned 15 and 1963 instances respectively;
the repaired discovery and URI validation passed for both targets. Output used
target hashes and counts only. The temporary binary was removed from the Pod.

This evidence validates root normalization, runtime identity access, real
instance-name compatibility, and source-URI construction without changing a
BigQuery table, view, GCS object, app-delivery manifest, or Kubernetes workload.