[DO NOT MERGE] eval: bundle Customer Context Builder (CCB) skills for routing eval - #134
[DO NOT MERGE] eval: bundle Customer Context Builder (CCB) skills for routing eval#134oscarkang24 wants to merge 2 commits into
Conversation
Adds the three CCB Agent Skills (customer-context-builder, gcp-data-qa, wiki-viewer) into skills/ as ccb-* with skill-ccb-* frontmatter names so Gemini CLI discovers them. Descriptions kept verbatim — they are the routing signal under test. No GEMINI.md delineation and no slash commands, to measure raw routing interference against the existing autoctx CUJs (go/db-nl2sql-crema-dev-guide#crema-cujs-evaluation-automation, CCB<>Crema alignment doc Challenge 1).
|
/gcbrun |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive framework for building, viewing, and querying GCP customer context repositories through a suite of agents and scripts. Key features include automated data collection from BigQuery and Google Workspace, a web-based viewer with drift and gap detection, and a natural language Q&A skill grounded in the generated context. Feedback focused on improving the robustness of pre-requisite checks, expanding Google Docs content extraction to include tables, optimizing file system operations in GCS uploads, and refining regex patterns for broader BigQuery table support.
| "GOOGLE_APPLICATION_CREDENTIALS=$GOOGLE_APPLICATION_CREDENTIALS but file does not exist" \ | ||
| "either fix the path or unset the env var to fall back to user ADC" | ||
| else | ||
| adc_path="${HOME}/.config/gcloud/application_default_credentials.json" |
There was a problem hiding this comment.
The path to Application Default Credentials (ADC) is hardcoded to the default Unix location. This may fail if the user has a custom CLOUDSDK_CONFIG directory or is running in an environment where the config path differs. Consider using gcloud info to dynamically resolve the configuration directory.
| adc_path="${HOME}/.config/gcloud/application_default_credentials.json" | |
| config_dir=$(gcloud info --format='value(config.paths.global_config_dir)' 2>/dev/null || echo "${HOME}/.config/gcloud") | |
| adc_path="${config_dir}/application_default_credentials.json" |
| p = el.get("paragraph") | ||
| if not p: | ||
| continue | ||
| for sub in p.get("elements", []): | ||
| tr = sub.get("textRun") | ||
| if not tr: | ||
| continue | ||
| text = tr.get("content", "") | ||
| parts.append(text) | ||
| total += len(text) | ||
| if total >= max_chars: | ||
| break | ||
| if total >= max_chars: | ||
| break | ||
| return "".join(parts).strip()[:max_chars] |
There was a problem hiding this comment.
The excerpt extraction logic only processes paragraph elements. Google Docs can contain other structural elements like table, tableOfContents, or sectionBreak that might hold relevant content for ranking. Consider iterating through all content types or providing a fallback for non-paragraph text.
| # `entrySource.description`. The leading lowercase + ≥1 underscore in the | ||
| # table segment further reduces false positives from prose. | ||
| BQ_TABLE_RE = re.compile( | ||
| r"\b[a-z][a-z0-9_]{2,}\.[a-z][a-z0-9_]+_[a-z0-9_]+(?:\.[a-z][a-z0-9_]+)?\b" |
There was a problem hiding this comment.
The BQ_TABLE_RE regex is overly restrictive as it requires an underscore in the table segment and does not support project IDs containing dashes. This will cause the gap detection to miss many valid BigQuery tables. Consider a more inclusive pattern that supports standard GCP project ID and dataset/table naming conventions.
| r"\b[a-z][a-z0-9_]{2,}\.[a-z][a-z0-9_]+_[a-z0-9_]+(?:\.[a-z][a-z0-9_]+)?\b" | |
| r"\b[a-z][-a-z0-9_]{2,}\.[a-z0-9_]{2,}(?:\.[a-z0-9_]{2,})?\b" |
| except Exception as e: | ||
| # Older networkx versions don't accept `seed` — retry without it. | ||
| try: | ||
| communities = louvain_communities(G, weight="weight") | ||
| except Exception: | ||
| return [] |
There was a problem hiding this comment.
Catching a broad Exception to handle a potential missing argument in louvain_communities is discouraged as it can mask other issues (like NameError if the import failed). Additionally, the nested broad catch on line 438 silences all errors in community detection. It is better to catch TypeError specifically for the version compatibility check and allow other exceptions to propagate.
except TypeError:
# Older networkx versions don't accept `seed` — retry without it.
communities = louvain_communities(G, weight="weight")| file_count = sum(1 for _ in local.rglob("*") if _.is_file()) | ||
| total_bytes = sum(p.stat().st_size for p in local.rglob("*") if p.is_file()) |
There was a problem hiding this comment.
The script performs two full recursive walks of the local directory to calculate file count and total size. This can be inefficient for large wiki trees. Combining these into a single loop would improve performance.
file_count, total_bytes = 0, 0
for p in (f for f in local.rglob("*") if f.is_file()):
file_count += 1
total_bytes += p.stat().st_size| p = el.get("paragraph") | ||
| if not p: | ||
| continue | ||
| line = render_paragraph(p) | ||
| if not line: | ||
| continue | ||
| out.append(line) | ||
| total += len(line) + 1 | ||
| if total >= max_chars: | ||
| out.append(f"\n[truncated at {max_chars} chars]") | ||
| break |
There was a problem hiding this comment.
Similar to the discovery script, this extraction logic only captures paragraph elements. Important technical documentation in Google Docs is often structured in tables, which are currently ignored. Expanding this to extract text from table elements would significantly improve the quality of the generated context.
|
/gcbrun |
1 participant
evaluating