feat: v0.1.0 — the Analyst archetype (data plugin + artifact + notes) - #1
Conversation
…with SQL and one chart Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…SQL too Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
QA panel review — WARN
code-review-structural · head e16e96f18f89 · formal
Low-risk PR: a new CI workflow, bundle manifest, and three utility scripts for managing plugin pin bumps. Both findings are minor and confined to check_bundle_updates.py. Verification confirmed both — the subprocess exception handling gap is real and the missing test coverage is a valid contrast with bump_pins_pr.sh. Fix the error handling first (one-line try/except around the subprocess.run call); the test gap is a follow-up. No panel disagreement, no coverage gaps, no prior requests to disposition.
Findings
| Severity | Location | Finding | Verified | |
|---|---|---|---|---|
| 🟡 | minor | scripts/check_bundle_updates.py:41 |
Unhandled subprocess exceptions in latest_tag crash the script on transient network failures (repo moved, auth expired, DNS failure, or 30s stall). | confirmed |
| 🟡 | minor | scripts/check_bundle_updates.py:1 |
check_bundle_updates.py introduces non-trivial semver caret-comparison and in-place manifest-rewriting logic with no unit tests in this repo, unlike bump_pins_… | confirmed |
findings JSON (machine-readable)
[
{
"file": "scripts/check_bundle_updates.py",
"line": 41,
"severity": "minor",
"category": "correctness",
"claim": "Unhandled subprocess exceptions in latest_tag crash the script on transient network failures (repo moved, auth expired, DNS failure, or 30s stall).",
"evidence": "latest_tag uses subprocess.run with check=True and timeout=30, which raises subprocess.CalledProcessError on any non-zero exit (repo moved, auth expired, DNS failure) and subprocess.TimeoutExpired on a 30s stall. Neither exception type is caught anywhere in the call chain from main() through to latest_tag().",
"source": "protopatch",
"verdict": "confirmed",
"note": "Re-read the file at head: subprocess.run(..., check=True, ..., timeout=30) is present in latest_tag(); no try/except exists anywhere in the file. Line anchor is approximate (call spans ~lines 31\u201337) but the substance is exact."
},
{
"file": "scripts/check_bundle_updates.py",
"line": 1,
"severity": "minor",
"category": "tests",
"claim": "check_bundle_updates.py introduces non-trivial semver caret-comparison and in-place manifest-rewriting logic with no unit tests in this repo, unlike bump_pins_pr.sh which is explicitly covered by protoAgent's test_bump_pins_pr.py.",
"evidence": "bump_pins_pr.sh header: \"protoAgent's tests/test_bump_pins_pr.py drives it with `gh` stubbed on PATH and a real throwaway git repo; this file is a verbatim copy, so re-sync it from there rather than editing.\" \u2014 check_bundle_updates.py has no analogous test reference; its is_compatible() and latest_tag() functions are untested.",
"verdict": "confirmed",
"note": "Quoted header text matches bump_pins_pr.sh verbatim at head. PR diff adds no test file for check_bundle_updates.py; is_compatible() and latest_tag() contain non-trivial logic with no coverage."
}
]Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
QA panel review — FAIL
code-review-structural · head 082970f68716 · formal
Overall risk is low-to-moderate: the code is well-structured, but the CI pipeline silently swallows script failures, which defeats the purpose of the verification job. Fix first: add set -o pipefail (or restructure the pipeline) in the bump job's run block so a non-zero exit from check_bundle_updates.py actually fails CI. The panel disagreed on severity for that item (structural finder called it minor; re-graded to major). Verification confirmed all three surviving findings and refuted the "no unit tests" finding as identical in substance to the round-1 request already on the record. No coverage gaps: all four synthesized findings were annotated by the verifier.
Findings
| Severity | Location | Finding | Verified | |
|---|---|---|---|---|
| 🟠 | major | .github/workflows/verify-bundle.yml:76 |
The run block pipes check_bundle_updates.py into tee without pipefail, so a non-zero exit from the Python script is masked by tee's exit 0 and the CI job passe… | confirmed |
| 🟡 | minor | scripts/check_bundle_updates.py:31 |
Unhandled subprocess exceptions in latest_tag crash the entire pin-bump check on transient network failures (repo moved, auth expired, DNS failure, or 30s stal… | confirmed |
| 🟡 | minor | scripts/check_bundle_updates.py:73 |
is_compatible returns True for any 0.0.y ≥ 0.0.x, but caret semantics for ^0.0.x only permits the exact same patch version, so a 0.0.x→0.0.y (y>x) release is m… | confirmed |
findings JSON (machine-readable)
[
{
"file": ".github/workflows/verify-bundle.yml",
"line": 76,
"severity": "major",
"category": "correctness",
"claim": "The run block pipes check_bundle_updates.py into tee without pipefail, so a non-zero exit from the Python script is masked by tee's exit 0 and the CI job passes silently on script failure.",
"evidence": "python3 scripts/check_bundle_updates.py protoagent.bundle.yaml | tee \"${BUMP_PINS_SCRATCH_DIR:-/tmp}/bumps.txt\"",
"source": "protopatch",
"verdict": "confirmed",
"note": "Pipeline exit code is tee's (0) in bash; set -e does not change pipeline semantics. Script failure is silently swallowed."
},
{
"file": "scripts/check_bundle_updates.py",
"line": 31,
"severity": "minor",
"category": "correctness",
"claim": "Unhandled subprocess exceptions in latest_tag crash the entire pin-bump check on transient network failures (repo moved, auth expired, DNS failure, or 30s stall), preventing remaining members from being checked. Flagged by both conventions and structural (protopatch) review.",
"evidence": "out = subprocess.run(\n [\"git\", \"ls-remote\", \"--tags\", url],\n check=True,\n capture_output=True,\n text=True,\n timeout=30,\n ).stdout",
"source": "protopatch",
"verdict": "confirmed",
"note": "check=True raises CalledProcessError; timeout=30 raises TimeoutExpired. Call site at line 86 (newest = latest_tag(m[\"url\"])) has no try/except."
},
{
"file": "scripts/check_bundle_updates.py",
"line": 73,
"severity": "minor",
"category": "correctness",
"claim": "is_compatible returns True for any 0.0.y \u2265 0.0.x, but caret semantics for ^0.0.x only permits the exact same patch version, so a 0.0.x\u21920.0.y (y>x) release is misreported as compatible and the pin is never bumped.",
"evidence": "return b[0] == 0 and b[1] == 0",
"source": "protopatch",
"verdict": "confirmed",
"note": "Traced: is_compatible('0.0.1','0.0.2') \u2192 a=(0,0,1), b=(0,0,2); skips both if-branches, returns True. npm ^0.0.1 means >=0.0.1 <0.0.2; missing a[2]==b[2] guard."
}
]
The Analyst archetype, v0.1.0: an analyst that answers questions from the operator's local data files (CSV, TSV, Parquet, JSON, Excel, SQLite) with read-only SQL, and shows each answer as one live chart and a one-line takeaway. It says what it assumed, cites the source file, never invents a number, and keeps answers short.
Same layout, CI and release workflow as
brand-launch-archetype(protoagent.bundle.yaml,scripts/verify_bundle.py/check_bundle_updates.py/bump_pins_pr.sh,.github/workflows/verify-bundle.yml, README).What's in the bundle
data→ data-plugin (7data_*tools; skillsexploring-a-datasetandbuilding-a-chart)artifact(builtin) renders the Vega-Lite chartsnotes(builtin, already on by default)config:defaults.archetype:block: label Analyst, iconChartColumn,soul_preset: analyst,requires_tools: [data_query, data_chart].config_inputs: [{ key: data.data_dirs, type: path, label: "Data folder" }]. It's optional. If it's left blank, the persona tells the operator to use Settings ▸ Plugins ▸ Data Analyst ▸ Data folders and never tries to set it. (data_dirsisspawns: true, so the agent'sset_configrefuses it anyway.)ref: feat/v0.1.0(data-plugin PR feat: v0.1.0 — the Analyst archetype (data plugin + artifact + notes) #1, not merged or tagged yet). Change it toref: v0.1.0once that tag exists.min_protoagent_version: 0.192.0(the vega-lite chart kind, protoAgent #4025, isn't released yet). The loader refuses it on 0.191.0, so theverifyjob is red against coremainuntil 0.192.0 ships.PROTOAGENT_REFcan't fix that, because #4025's head also reports 0.191.0.analystsoul preset ships with feat(archetypes): Analyst — answers questions from your local data with SQL and one chart (held) protoAgent#4026 (the held catalog row).Verified
scripts/verify_bundle.pyon a local core with #4025 and the core PR applied, with the version set to 0.192.0 locally: data, artifact and notes all install and load, and the contractdata_queryanddata_chartis bound.anthropic-oauth:claude-sonnet-5-5):data_sourcescall and got back: "I can't see any data yet. Add the folder … in Settings ▸ Plugins ▸ Data Analyst ▸ Data folders …". There was noset_configcall (checked in the audit log).data_dirsset to a synthetic coffee-shop CSV, the same question got a bar chart in the Artifact panel and the answer "Saturday sells the most: $2,351 in average daily revenue, 38.6% above the overall daily average.", with an assumptions line and the source file. The numbers match an independent recompute.main. Before that, I tested it with an instance-local catalog override pointing at this branch.🤖 Generated with Claude Code