feat: Claude Code plugin with a quantstats-migration skill, plus the CVaR and doc bugs it uncovered - #922
Merged
Merged
Conversation
The public wrapper took (sigma, confidence, **kwargs) and computed alpha = 1 - confidence, so an `alpha=` keyword landed in **kwargs and was discarded: `alpha=0.01` silently returned the 95 % figure with no error and no warning. The docstring described the opposite contract on all three counts — alpha as the real parameter, confidence as legacy-deprecated, and a TypeError on unexpected keywords that **kwargs made unreachable. Both spellings are now first-class parameters defaulting to None. Passing both raises ValueError rather than silently preferring one, and dropping **kwargs means a misspelled keyword raises TypeError as the docstring always promised. Default behaviour is unchanged at alpha=0.05, so the QuantStats parity tests are unaffected. confidence is kept rather than deprecated: it is the QuantStats spelling used by the parity test and the risk_metrics notebook, and demoting it would be an API decision rather than a bug fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Nine statements in the guide named symbols or semantics that the package does not have. Each was checked against the installed package by introspection rather than by reading: - data.reports.summary() and data.reports.to_html() do not exist; Reports offers metrics() and full(). summary() lives on the stats accessor. - Portfolio does not expose the same plots as Data. PortfolioPlots is a separate, smaller set; the guide now lists all ten and routes everything else through pf.data.plots. - The reports accessor is Data.reports but Portfolio.report. - from_risk_position takes vola as an EWMA lookback in periods (int | dict[str, int]), not a volatility frame. - conditional_value_at_risk accepts confidence or alpha, not one or the other, and rejects both at once. - information_ratio is raw by default and matches QuantStats; it does not annualise unless asked, so the "roughly sqrt(252) times larger" warning was backwards. - lead_lag_ir_plot takes (start, end), not max_lag. - turnover_summary returns a pl.DataFrame of metric/value rows, not a dict. Also adds data.plots.assets() to the plots table. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…kill
Models carry a strong prior for QuantStats — pandas, module-level
functions, qs.reports.html(returns) — and write that shape when asked for
jquantstats. Documentation on the site does not reach a coding session;
a skill loads automatically when the topic comes up, so it can correct the
prior at the moment it would otherwise fire.
The skill covers the function-to-object shift, the pandas-to-Polars
conversion including null_strategy, the Data vs Portfolio split and their
asymmetric accessors, the aliases QuantStats has that jquantstats does not,
and the handful of same-name-different-number cases. It closes by telling
the reader to introspect the package rather than trust the mapping, which
is the durable defence against this file going stale.
The plugin lives in plugin/ rather than the repo root so it does not
collide with the Python project's directories; hatch's include list is an
allowlist, so none of it reaches the wheel. Both manifests pass
`claude plugin validate --strict`.
Install with:
/plugin marketplace add Jebel-Quant/jquantstats
/plugin install jquantstats@jebel-quant
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a Claude Code plugin for jquantstats (starting with a QuantStats→jquantstats migration skill), fixes a CVaR parameter-handling bug uncovered while writing that skill, and updates the migration documentation accordingly.
Changes:
- Add a Claude Code plugin manifest and a
quantstats-migrationskill document to guide correct API usage and migrations. - Fix
conditional_value_at_risksoalpha/confidenceare handled explicitly (mutually exclusive) and unknown keywords are no longer silently ignored; add targeted tests. - Correct multiple inaccuracies in
docs/MIGRATION.mdto reflect the actual API surface.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/jquantstats/_stats/_basic_core.py |
Updates CVaR API to accept alpha and/or confidence explicitly and reject ambiguous/unknown kwargs. |
tests/test_jquantstats/test__stats/test_stats.py |
Adds regression tests ensuring alpha is honored, dual tail args error, and unknown kwargs raise TypeError. |
docs/MIGRATION.md |
Updates migration guide examples/mapping to reflect actual jquantstats API behavior. |
plugin/skills/quantstats-migration/SKILL.md |
Adds the migration skill content for Claude Code sessions. |
plugin/.claude-plugin/plugin.json |
Adds Claude Code plugin manifest for the jquantstats plugin. |
.claude-plugin/marketplace.json |
Adds marketplace manifest entry pointing to the plugin under plugin/. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
353
to
355
| Raises: | ||
| TypeError: If unexpected keyword arguments are passed. | ||
| ValueError: If both ``confidence`` and ``alpha`` are given. | ||
|
|
Comment on lines
+328
to
331
| | — | `data.plots.assets()` | | ||
|
|
||
| All `data.plots.*` methods return an interactive **Plotly figure** instead | ||
| of a static matplotlib figure. |
| @@ -455,7 +486,9 @@ pf.tilt_timing_decomp # side-by-side NAV comparison | |||
| # Turnover analytics | |||
| pf.turnover # daily one-way turnover (fraction of AUM) | |||
| pf.turnover_weekly() # weekly aggregate | |||
| pf.lag(1) # shift positions — execution-delay study | ||
| pf.plots.lead_lag_ir_plot() # IR across lags | ||
| pf.tilt, pf.timing, pf.tilt_timing_decomp # allocation vs timing skill | ||
| pf.turnover, pf.turnover_weekly(), pf.turnover_summary() |
Comment on lines
+29
to
+32
| # jquantstats | ||
| data = jqs.Data.from_returns(returns=returns_pl) # once | ||
| sharpe = data.stats.sharpe()["MyStrategy"] # {"MyStrategy": 1.23} | ||
| ``` |
The same false statement appeared in three places: annualise defaults to False in the signature, but the docstring said True, docs/benchmark.md said jquantstats "annualises by default (annualize=True)" — also misspelling the parameter — and the parity test's own docstring claimed it scales by sqrt(252) while asserting the default equals the raw QuantStats value. The code is correct and unchanged: both libraries return a raw information ratio by default and agree without adjustment, with annualise=True offered as an extra that QuantStats has no equivalent for. Also fixes the periods_per_year docstring, which claimed a default of 252 where the parameter defaults to None and infers the factor from the data. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds a test that resolves every accessor reference in docs/*.md, README.md and the bundled Claude Code skill against the installed package, so a rename breaks the suite instead of silently shipping a guide that tells the reader to call something imaginary. It checks names, not behaviour — cheap, and precisely the failure mode the docs had. Turned up three more broken names on the first run, all in getting_started.md, the first page a new user reads: - pf.plots.rolling_sharpe -> rolling_sharpe_plot - data.plots.returns_distribution -> histogram - data.reports.to_html -> full The same page also carried the from_risk_position(vola=vola_df) error already fixed in the migration guide; vola is an EWMA lookback in periods, not a frame. That one is a keyword rather than an accessor, so the checker does not see it — a reminder of what this guard does not cover. Four names are allowlisted in KNOWN_ABSENT: two "foo" placeholders from translation rules, and data.reports.summary / to_html, which the guides name precisely to say they are *not* the method you want. A companion test asserts the allowlist entries are still genuinely absent, so an entry cannot outlive its reason and quietly stop checking a real name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Path.read_text() without an encoding uses the locale codec, which is cp1252
on the Windows CI runner. Four docs carry em dashes, so the checker died with
UnicodeDecodeError there while passing on macOS and Linux:
docs/index.md docs/MIGRATION.md docs/cost_models.md docs/STABILITY.md
All four decode cleanly as UTF-8; the encoding is now explicit.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-Quant/jquantstats into feat/claude-plugin-migration-skill
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Exposes jquantstats as a Claude Code plugin, starting with a single skill that
teaches the QuantStats → jquantstats migration — plus the bugs that writing the
skill uncovered, and a test so they cannot come back.
Why a skill
Models carry a strong prior for QuantStats — pandas, module-level functions,
qs.reports.html(returns)— and write that shape when asked for jquantstats.The migration guide on the docs site serves humans reading the site; it does not
reach a coding session. A skill loads automatically when the topic comes up, so
it corrects the prior at the moment it would otherwise fire.
What is in the PR
plugin/skills/quantstats-migration/SKILL.md— the function-to-objectshift, the pandas→Polars conversion including
null_strategy, theDatavsPortfoliosplit and their asymmetric accessors, the aliases QuantStats hasthat jquantstats does not, and the same-name-different-number cases. It closes
by telling the reader to introspect the package rather than trust the mapping.
fix(stats)—conditional_value_at_risktook(sigma, confidence, **kwargs)and computedalpha = 1 - confidence, so analpha=keyword landedin
**kwargsand was discarded.alpha=0.01silently returned the 95 % figurewith no error and no warning. Both spellings are now first-class parameters;
passing both raises
ValueError, and dropping**kwargsmakes a misspelledkeyword raise
TypeErroras the docstring always promised. Default behaviour isunchanged at
alpha=0.05, so QuantStats parity is unaffected.docs(migration)— nine wrong statements in the migration guide, includingtwo methods that do not exist (
data.reports.summary(),data.reports.to_html()), alead_lag_ir_plot(max_lag=)parameter that wasnever there, and
turnover_summary()documented as returning a dict when itreturns a
pl.DataFrame.docs— theinformation_ratioannualisation claim was wrong in threeplaces at once: the docstring said
Defaults to Truewhere the signature saysFalse,docs/benchmark.mdsaid jquantstats annualises by default (misspellingthe parameter as
annualize), and the parity test's docstring claimed it scalesby
sqrt(252)while asserting the default equals the raw QuantStats value. Thecode is correct and unchanged.
test— a test that resolves every accessor reference indocs/*.md,README.mdand the bundled skill against the installed package. It found threemore broken names on its first run, all in
getting_started.md, the first pagea new user reads:
pf.plots.rolling_sharperolling_sharpe_plotdata.plots.returns_distributionhistogramdata.reports.to_htmlfullThat page also repeated the
from_risk_position(vola=vola_df)error.volaisa keyword rather than an accessor, so the checker does not see it — worth
knowing what this guard does and does not cover.
Sixteen documented claims in total did not survive contact with the package.
Building the skill from the source rather than from the guide is what forced
each one to be checked.
Verification
make test— 1263 passed, coverage 100.00 %make typecheck— clean under both ty and mypy strictmake fmt— clean, markdownlint includedmake book— builds; the new cross-file anchor resolves in the generated HTMLclaude plugin validate ./plugin --strict— passes, as does the marketplace manifestInstalling the plugin
The plugin lives in
plugin/rather than the repo root so it does not collidewith the Python project's directories. Hatch's
includelist is an allowlist,so none of it reaches the wheel.
🤖 Generated with Claude Code