Skip to content

Code quality improvements - #3

Merged
karrmagadgeteer2 merged 8 commits into
masterfrom
code-quality-improvements
Sep 22, 2026
Merged

karrmagadgeteer2 merged 8 commits into
masterfrom
code-quality-improvements

Conversation

@karrmagadgeteer2

@karrmagadgeteer2 karrmagadgeteer2 commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

A round of code-quality improvements identified from a general review of the codebase, plus the version bump the API addition motivates.

  • Typed errors: four throw sites used plain Error instead of the library's typed error hierarchy. Added InvalidArgumentError and CaptorApiError, and put the previously-unused IncorrectArgumentComboError to work, so callers can instanceof-discriminate all failure modes consistently.
  • JSDoc coverage: types.ts (0/14 exports documented), utils.ts (4/10), and roughly half of OpenTimeSeries's methods in series.ts were undocumented, which showed up directly in the generated typedoc site. Filled in the gaps and added @throws tags.
  • Type-aware ESLint: parserOptions.project was already enabled (paying the type-checking cost) but the config only extended tseslint.configs.recommended, so none of the type-aware rules (no-unsafe-*, etc.) actually ran. Switched to recommendedTypeChecked and fixed what it surfaced (two any[] typing gaps from new Array(n), an implicit any from JSON.parse in a script, ~90 already-redundant non-null/type assertions removed by --fix).
  • CI formatting gate: npm run format:check existed as a script but nothing ran it in CI - added it alongside the existing lint/typecheck step.
  • Docs build/deploy automation: docs/ (typedoc output) was manually rebuilt and committed to master, so it could drift from source and every regeneration produced a large, low-signal diff. Added a workflow that builds and deploys to GitHub Pages via GitHub Actions on every push to master/main, and stopped tracking the generated output. (The repo's Pages source setting has been switched to "GitHub Actions" accordingly.)
  • Branch coverage: was gated at 76% while statements/functions/lines sat at 95%. Investigated per-file with istanbul's HTML report and closed the largest, cheapest gaps - unused default-parameter branches (simulation.ts, now 100%), a vol=0 test that didn't actually produce zero variance, an untested singular-covariance-matrix case, and ~15 risk-metric methods in series.ts that share a dates.length < 2 guard never exercised by a short series. Overall branches: 81.6% -> 85.73%; raised the threshold to 84%. Remaining gaps are documented in the commit as defensive/edge-case code not worth chasing to 100%.
  • Version bump: 1.1.0 -> 1.2.0. The new InvalidArgumentError/CaptorApiError exports are an additive, backwards-compatible public API change, so minor per semver.

Test plan

  • npm run check (lint + typecheck) passes
  • npm run test - 306 tests passing
  • npm run format:check passes
  • npm run test:coverage meets the new 84% branch threshold (85.73% actual)
  • npx typedoc builds cleanly with the new JSDoc

🤖 Generated with Claude Code

karrmagadgeteer2 and others added 6 commits September 22, 2026 21:11
Four throw sites used plain `Error`, so callers couldn't discriminate
them from the library's typed error hierarchy the way they can for
DateAlignmentError, NoWeightsError, etc. Add InvalidArgumentError (bad
argument values) and CaptorApiError (Captor API fetch failures), reuse
the existing but previously unthrown IncorrectArgumentComboError for
the report generator's constituent-count check, and update tests to
assert on error type in addition to message.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
types.ts had zero doc comments across its 14 exports, utils.ts left
6 of 10 functions undocumented, and roughly half of OpenTimeSeries's
methods/getters in series.ts had no JSDoc. Since typedoc publishes
this as the library's docs site, these gaps showed up directly in
the generated reference. Document the remaining exports and add
@throws tags where a typed error can be raised.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
parserOptions.project was already set (full type info was being
computed on every lint run) but the config only extended
tseslint.configs.recommended, so none of the type-aware rules
(no-unsafe-*, no-floating-promises, etc.) were actually enabled.
Switch to recommendedTypeChecked.

Fixes surfaced by the switch: new Array(n).fill(x) resolves to
any[], not number[], causing unsafe-return/unsafe-argument in
frame.ts's and portfoliotools.ts's weight calculations - type the
Array constructor calls explicitly. In scripts/compare-metrics.ts,
narrow JSON.parse's result to `unknown` before use instead of
implicitly typing it `any`. The remaining ~90 violations were
non-null assertions and type assertions that were already
redundant under the current tsconfig and are removed by --fix.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
npm run format:check existed as a script but nothing ran it, so a
PR with unformatted code could merge and only get caught locally
if someone happened to run prettier by hand. Run it alongside the
existing lint/typecheck step.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
docs/ (typedoc output) was manually rebuilt and committed to
master, so it could drift from source between rebuilds and every
regeneration produced a large, low-signal diff. Add a workflow that
builds and deploys the docs to GitHub Pages via GitHub Actions on
every push to master, and stop tracking the generated output.

Manual follow-up needed: this repo's GitHub Pages source (Settings
> Pages) is presumably still set to "Deploy from a branch" against
/docs on master. It needs to be switched to "GitHub Actions" for
this workflow to actually publish the site - that's a repo setting
this commit can't change.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Branches sat at a 76% threshold while statements/functions/lines
were gated at 95%. Investigated with istanbul's HTML report rather
than guessing from the summary table:

- simulation.ts (72.7% branches): entirely unused default-parameter
  branches (trading_days_in_year, seed, jump params) - every call
  site passed every argument explicitly. Added tests that omit them.
- portfoliotools.ts (64.9%, the worst file): the existing "covers
  vol=0 branch" test used a strictly increasing price series, which
  doesn't actually produce zero variance - fixed it to use flat
  prices. Added coverage for efficientFrontier's default parameters,
  a singular-covariance-matrix input (duplicate assets, which the
  analytic solver can't resolve into a frontier), and
  preparePlotData's too-few-valid-returns guards.
- series.ts (74.6%): ~15 risk-metric methods share a
  `dates.length < 2` (etc.) guard that was never exercised. One
  single-point-series test closes all of them at once.

Net: branches 81.6% -> 85.73% overall (portfoliotools.ts alone
64.9% -> 80.7%, simulation.ts to 100%). Raised the threshold to 84%
to lock in the improvement. Remaining gaps in frame.ts/report.ts/
plot.ts are mostly defensive `??` fallbacks and matrix-math edge
cases (singular/degenerate inputs) that would need contrived inputs
disproportionate to their value; left as documented residual rather
than chased to 100%.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@codecov-commenter

Copy link
Copy Markdown

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

karrmagadgeteer2 and others added 2 commits September 22, 2026 21:44
The typed-error commit on this branch adds two new exported error
classes (InvalidArgumentError, CaptorApiError) to the public API -
an additive, backwards-compatible change, so minor per semver.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The manual "run npm run docs, commit docs/, configure Settings >
Pages to deploy from a branch" instructions were superseded by the
GitHub Actions docs workflow added earlier on this branch, which now
builds and deploys docs automatically on push to master. npm run
docs itself is still useful for a local preview, so it stays.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@karrmagadgeteer2
karrmagadgeteer2 merged commit fc3e63d into master Sep 22, 2026
3 checks passed
@karrmagadgeteer2
karrmagadgeteer2 deleted the code-quality-improvements branch September 22, 2026 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants