docs(jsdoc): add jsdoc to tier 1 surface (TIB-2026-05-04)#595
Merged
Conversation
… (TIB-2026-05-04)
Phase 0 — TIB, style guide, and tooling
* docs/tibs/TIB-2026-05-04-jsdoc-coverage-on-exported-symbols.md: 6-phase
rollout from a 16% baseline to 100% on every Tier 1–4 export.
* docs/jsdoc-style.md: canonical shape (description → on-chain reads →
itemized @param leaves → @returns shape → @throws by class identity →
runnable @example) with side-by-side bad/good drawn from marketV1Borrow.
* AGENTS.md §6: cross-reference to the style guide and the TIB.
* typedoc.json + scripts/jsdoc-coverage.js: informational TypeDoc config
and a regex-based burndown printed via `pnpm jsdoc:coverage`.
Phase 1 — Tier 1 backfill (morpho-sdk + evm-simulation)
* Every action builder (vaultV1, vaultV2, marketV1, requirements/*).
* MorphoClient class, the three factory methods, morphoViemExtension.
* All 45 exported error classes in src/types/error.ts (one-liners).
* evm-simulation: simulate(), screenAddresses(), SimulationPackageError.
* Coverage: morpho-sdk 12% → 64%, evm-simulation 49% → 59%, repo 8% → 16%.
Phase 5 — Automated enforcement (scoped to Phase 1 surface)
* eslint.config.js: flat config with eslint-plugin-jsdoc rules
(require-jsdoc, require-param, require-returns, require-example) gated
to morpho-sdk action builders + client + types/error.ts and
evm-simulation simulate / screenAddresses / errors. Future phases widen
the globs.
* package.json: `pnpm lint` now chains biome → eslint → lint:address.
* Scoped pnpm overrides for ajv 6.12.6 inside @eslint/eslintrc and eslint
to coexist with the repo-wide ajv 8.18.0 pin.
* TypeDoc strict gate deferred — Addendum on the TIB explains the @param
dotted-leaf vs TypeDoc validator conflict and the resolution path.
Doc-only at runtime; no public API changes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ESLint adds a second linter on top of biome with the explicit goal of
replacing one of biome's responsibilities, plus brittle scoped pnpm
overrides for ajv to coexist with the repo-wide pin. Footprint is
significant for a single rule family.
Reverts:
* eslint.config.js (deleted).
* eslint, eslint-plugin-jsdoc, typescript-eslint devDependencies.
* `lint:jsdoc` script and the eslint hop in `pnpm lint`.
* Scoped pnpm overrides for `@eslint/eslintrc>ajv` and `eslint>ajv`.
* `eslint --no-error-on-unmatched-pattern` lint-staged entry.
Updates the TIB:
* Goals: replace "automated CI gate that fails…" with "observable
burndown so phases are measurable".
* Non-Goals: explicitly out — adding a second linter to the toolchain.
* Proposed Solution pillar 3: rewritten as "Observable progress,
automated gate deferred." Burndown + reviewer enforcement; gate lands
only when biome ships JSDoc rules or a lighter in-repo check emerges.
* Phase 5: now "Reassessment" rather than "Enforcement."
* Considered Alternatives: ESLint adoption added as Alternative 6 with
explicit rejection rationale (ajv pin friction, second-linter cost);
TypeDoc strict gate added as Alternative 7 with the @param dotted-leaf
conflict captured.
* Dependencies, Observability, Open Questions, Addenda updated to match.
Burndown still runs informationally via `pnpm jsdoc:coverage`.
Coverage unchanged: morpho-sdk 64%, evm-simulation 59%, repo 16%.
JSDoc backfill is comment-only — no published-package behavior changes, so no changelog entry is needed.
…API in client examples Address findings from local-review on the doc backfill: Critical (RAY vs WAD) Vault deposit, vault deposit V2, vault migrateToV2, marketV1 repay, and marketV1 repayWithdrawCollateral all had `maxSharePrice: 1.01e18` (WAD) in @example blocks. The on-chain check is RAY (1e27); the entity layer computes the bound via MathLib.wToRay(WAD + slippage). Copy-paste of the WAD value was ~1e9× too tight and would revert every transaction. Replace with `1_010_000_000_000_000_000_000_000_000n` (RAY-scaled, 1.01x). Critical (style guide) docs/jsdoc-style.md canonical example used a non-existent `new MorphoClient({ client })` shape and a `marketV1(marketParams).borrow` call that doesn't match the entity API. Rewrote to match the actual positional MorphoClient ctor and the lazy `{ buildTx }` entity pattern. Same fix for the eslint cruft on line 242 (now points at TIB-2026-05-04 Considered Alternative 6). High (entity API examples) morphoClient.ts and morphoViemExtension.ts @example blocks invoked `await vault.deposit(...)` / `await market.borrow(...)` returning a Transaction directly. The actual entity methods are synchronous and return `{ buildTx, getRequirements }`, requiring a pre-fetched `accrualVault` / `positionData`. Rewrote all four examples to follow the lazy two-step pattern. High (missing @throws) marketV1Borrow + marketV1SupplyCollateralBorrow propagate five reallocation-validation errors from buildReallocationActions; none were documented. Added all five to both, plus the two from getRequirementsAction (DepositAssetMismatchError / DepositAmountMismatchError) on supplyCollateralBorrow. vaultV2ForceRedeem missed NonPositiveAssetAmountError raised by encodeForceDeallocateCall on per-deallocation amount; added it. vaultV2ForceWithdraw clarified: same class fires for both withdraw.amount <= 0n and any deallocation.amount <= 0n. vaultV1MigrateToV2 missed Deposit{Asset,Amount}MismatchError on the permit/permit2 path; added. Medium (missing @throws on permit/permit2 paths) marketV1 repay, repayWithdrawCollateral, supplyCollateral; vaultV1 and vaultV2 deposit. Each propagates Deposit{Asset,Amount}MismatchError from getRequirementsAction when requirementSignature is supplied. Added to all five. Medium (description fixes) marketV1 repay + repayWithdrawCollateral: NonPositiveRepayAmountError also fires when either of `assets` / `shares` is negative, not only when both are zero. Reworded to cover both branches. Low (polish) marketV1 borrow: clarified NonPositiveMinBorrowSharePriceError as "negative; zero is allowed despite the class name". marketV1 repay: reordered @throws to firing order in validateRepayParams. getRequirements: noted ApprovalAmountLessThanSpendAmountError is unreachable through this entry point. evm-simulation/simulate: replaced `data: "0x..."` placeholder with the named identifier `encodedCalldata` per the style guide. Tooling cleanup - typedoc.json: dropped `excludeProtected: false` (TypeDoc default). - package.json: renamed `docs:check` -> `docs:build` (it generates docs into docs/api/, doesn't check). - scripts/jsdoc-coverage.js: switched to readdirSync({ withFileTypes }), log swallowed dir-read errors to stderr, broadened EXPORT_RE to accept multi-modifier exports. - docs/jsdoc-style.md operational rules: replaced the "Enforcement gates flip on after Phase 1" claim (which contradicted the eslint revert) with a pointer to TIB Considered Alternative 6. - TIB Goal #3: explicitly named `pnpm jsdoc:coverage` and called out Phase 5 reassessment. Pre-existing `throw new Error("Expiration is not defined")` in getRequirementsAction violates §2.2 but is a runtime change out of scope for this doc-only PR; tracked for follow-up. No runtime behavior changes. Coverage unchanged: morpho-sdk 64%, evm-simulation 59%, repo 16%.
…internal helpers Address findings from local-review and PR bot comments (Devin, Codex): High — runnable examples Style guide canonical example and MorphoClient constructor @example used createPublicClient (no account). Both downstream snippets call market.borrow({ userAddress }), and MorphoMarketV1.borrow invokes validateUserAddress(client.viemClient.account?.address, userAddress) which throws MissingClientPropertyError when the client has no account. Switched both to createWalletClient with `account: borrower` (or `user`) so the lazy two-step pattern actually runs as documented. High — transferAmount @param accuracy marketV1Repay and marketV1RepayWithdrawCollateral @param said "Must be at least the repay amount". In assets mode, validateRepayParams throws TransferAmountNotEqualToAssetsError when transferAmount !== assets — must equal exactly, not "at least". Reworded to split assets vs shares modes explicitly. Low — error class doc widened NonPositiveRepayAmountError class JSDoc said "when both `assets` and `shares` are zero". validateRepayParams also throws on negative inputs (the action-level @throws was already widened in the prior commit, but the class-level doc lagged). Updated to "non-positive amounts: both zero, or either negative". Low — internal helpers acknowledged buildReallocationActions and getRequirementsAction had @example blocks showing `import ... from "@morpho-org/morpho-sdk"`, but neither function is re-exported from the package barrel. The example would fail at import time. Marked both `@internal` and removed the misleading @example — these are consumed only by other action builders, not directly by integrators. Errors they raise are still documented on the public builders' @throws lists. Low — operational rule self-contradiction Style guide and TIB both said "Doc-only changesets are patch ... add the changeset in the same PR" but this PR explicitly drops the changeset. Carved out an exception for repo-meta-only PRs (TIB, style guide, root tooling) so the rule no longer contradicts the PR that publishes it. Low — TypeDoc config minimisation Dropped excludePrivate: true (TypeDoc default), matching the prior removal of excludeProtected: false. Skipped (out of scope for doc-only PR): - Pre-existing `throw new Error("Expiration is not defined")` in getRequirementsAction (§2.2 violation, runtime change). - Verbose @throws lists on marketV1 borrow / supplyCollateralBorrow (kept explicit class enumeration for pattern-matching). - Burndown script polish (analyzeFile pre-allocation, EXPORT_RE self-check) — minor, not worth a follow-up commit on its own. No runtime behavior changes. Coverage unchanged: morpho-sdk 64%, evm-simulation 59%, repo 16%.
…h burndown
High — accuracy fixes
- repay.ts / repayWithdrawCollateral.ts: inline `transferAmount` field
comments still said "Must be greater than or equal to the repay amount"
after the prior commit fixed only the function-level @param. Both now
split assets-mode (must equal exactly, TransferAmountNotEqualToAssetsError)
from shares-mode (upper-bound estimate, residual skimmed).
- encodeErc20Permit2 @example: literal `4_294_967_295n` was labeled
`MAX_UINT_48`, but that value is 2^32-1 (MAX_UINT_32). MAX_UINT_48 is
281_474_976_710_655n. Replaced with the correct literal so an integrator
copying the example actually gets an effectively-indefinite Permit2
expiration instead of one that resolves to 2106-02-07.
Medium — internal consistency
- getRequirements: routing rule #2 omitted the DAI exclusion. Source
excludes DAI (`address !== dai`) so it falls through to Permit2 even
with `useSimplePermit: true`. Documented inline.
- TIB Considered Alternative 7 referenced `pnpm docs:check`; renamed to
`pnpm docs:build` (the actual script after the prior commit).
- Changeset carve-out wording across the style guide and TIB widened
from "repo-meta-only" to "JSDoc-only changes inside packages/*/src/ —
and repo-meta-only PRs". The prior carve-out didn't actually exempt
this PR (which touches published packages' src/), creating a
rule-vs-PR self-contradiction.
Low — style polish
- marketV1 repay + repayWithdrawCollateral @throws reordered to match
validateRepayParams' firing order: NonPositiveRepayAmountError now
appears before MutuallyExclusiveRepayAmountsError (validator throws
the former first when assets/shares is negative).
- getRequirementsPermit @example: replaced `DAI` with `USDC`. DAI uses
a non-standard permit signature and is excluded by getRequirements;
the example now matches the function's intended use.
- buildReallocationActions doc: replaced "Caller must ensure
reallocations is non-empty" with the actual contract — "returns
`{ actions: [], fee: 0n }` for an empty input".
- "Optional analytics metadata attached to the bundle" -> "transaction"
on the seven direct-call / multicall builders that don't go through
bundler3 (vaultV1 redeem/withdraw, vaultV2 redeem/withdraw,
marketV1 withdrawCollateral; "multicall transaction" for vaultV2
forceWithdraw / forceRedeem).
- typedoc.json: dropped three more values that match TypeDoc defaults
(`treatWarningsAsErrors`, `logLevel`, `skipErrorChecking`).
- scripts/jsdoc-coverage.js:
* `--json` output mode for the future Phase 5 CI gate.
* `--self-check` mode runs 15 EXPORT_RE / hasParams / isFunctionLike
cases, prints pass/fail, exits 1 on any failure (cheap guard
against TypeScript syntax drift).
* `analyzeFile` now skips the per-file undocumented[] allocation
unless `--verbose` is set.
* Honor `@internal` JSDoc — internal helpers (buildReallocationActions,
getRequirementsAction) no longer count against coverage, matching
typedoc's excludeInternal behavior.
No runtime behavior changes. Coverage shifts:
morpho-sdk 64% (98/153 -> 96/151 after @internal exemption shrinks denominator)
evm-simulation 59% (unchanged)
Repo total 16% (152/937).
…heck High — example runnability - encodeErc20Permit @example used `token: DAI` while the @param says the token "must support EIP-2612". DAI uses a non-standard permit; the prior pass fixed getRequirementsPermit but missed the lower-layer encoder. Switched to USDC with a clarifying comment. Medium — invented identifier + self-check truthfulness - getRequirementsPermit2 @example referenced `PERMIT2_ADDRESS`, which doesn't exist anywhere. Replaced with `getChainAddresses(1).permit2` destructure so the snippet uses the real symbol path. - scripts/jsdoc-coverage.js --self-check claimed to cover EXPORT_RE + hasParams + isFunctionLike but only exercised EXPORT_RE. Expanded to 35 cases (15 EXPORT_RE + 9 hasParams + 7 isFunctionLike + 4 @internal). Low — ordering, units, wording, polish - vaultV1/deposit.ts & vaultV2/deposit.ts @throws reordered to firing order: ZeroDepositAmountError now appears AFTER DepositAssetMismatchError / DepositAmountMismatchError (the latter fire inside getRequirementsAction at line 144; the former fires at line 161). - vaultV1/deposit.ts & vaultV2/deposit.ts @param maxSharePrice now states "(in RAY, slippage protection enforced on-chain by GeneralAdapter1)" matching every other action's unit hint. - repayWithdrawCollateral.ts inline field comment AND @param now mention that residual loan tokens are skimmed back to `receiver` in shares mode, matching repay.ts symmetry. - getRequirements.ts: @throws rationale tightened from "approvalAmount === spendAmount" to "approvalAmount >= spendAmount" — Permit2 inner path actually passes MAX_UINT_160 so equality wasn't quite right. - TIB Phase 0 line refs (deposit.ts:60, borrow.ts:47) dropped — those line numbers were pre-backfill. Function names are unique within their files; line suffix added no information. - scripts/jsdoc-coverage.js: * hasParams regex now consumes optional `<T>` generic arrow params, e.g. `export const foo = <T>(x: T) => x;`. Latent fix; no Tier 1 export uses this form today, but the regex would have silently returned hasParams=false otherwise. * Wired `pnpm jsdoc:coverage:check` so the self-check is invokable as a script (and a future CI step can call it). * Dropped the redundant `verbose &&` guard on undocumented[] iteration — analyzeFile already returns an empty array when not collecting. - typedoc.json: trimmed `validation.notExported` and `validation.invalidLink` (both TypeDoc defaults — keeping only `notDocumented` which is non-default). Skipped (low-value-vs-cost): - `export const enum Foo {}` form not handled by EXPORT_RE — no occurrences in the codebase, would require a special-case branch. No runtime behavior changes. Coverage unchanged: morpho-sdk 64%, repo 16%.
…espace support High — close prior-round regressions - getRequirementsPermit2 @example destructured `permit2` directly into a required Address arg, but ChainAddresses.permit2 is `Address | undefined` (~19 chains lack it). Added an explicit guard: `if (!permit2) throw new Error("Permit2 not configured for this chain");` so the example type-checks and demonstrates the safe-handling pattern. - `pnpm jsdoc:coverage:check` was defined last round but never invoked. Wired it into `pnpm lint` so CI + lint-staged + pre-commit all run the 35-case (now 40-case) self-check on every change. Medium — example caveats and missed export form - marketV1Borrow / marketV1SupplyCollateralBorrow / vaultV1MigrateToV2 @example values for minSharePrice / minSharePriceVaultV1 were `0n`, which technically passes validation but disables slippage protection. Annotated each with a comment pointing to `computeMinBorrowSharePrice` so integrators don't ship slippage-free borrows. - encodeErc20Permit @example caveat tightened: USDC works on L1 but bridged USDC.e on most L2s does not implement EIP-2612. Added the L2 warning so an integrator swapping chainId doesn't get a silent runtime failure. - scripts/jsdoc-coverage.js EXPORT_RE was missing the `namespace` keyword. Tier 2/3 packages (bundler-sdk-viem, blue-sdk-viem, migration-sdk-viem) export ~10 namespace blocks that were invisible to the burndown. Added `namespace` to the kind alternation, treated like class/interface in the description-suffices branch, and added a self-check case. Burndown denominator: 937 -> 970. Low — script polish + symmetric @throws preconditions - vaultV1/deposit.ts and vaultV2/deposit.ts @throws for DepositAssetMismatchError / DepositAmountMismatchError now state "when amount > 0n AND requirementSignature is provided" — the mismatch errors only fire inside the if (amount > 0n) branch in source, so the prior wording over-promised on the native-only path. - scripts/jsdoc-coverage.js: isFunctionLike now consumes optional generic-arrow `<T>(...)` symmetrically with hasParams. Without this, a generic-arrow exported function const was misclassified as a plain constant and skipped @param/@returns/@example checks entirely (latent today; no Tier 1 export uses this form). - scripts/jsdoc-coverage.js: extracted `isInternal(jsdocBlock)` helper so the production check (analyzeFile:152) and the self-check (internalCases) share one regex literal. Prior layout was tautological — the test created a fresh regex inline. - Added 3 negative re-export self-check cases (`export { foo } from`, `export * from`, `export type { Foo } from`) so a future contributor can't accidentally make EXPORT_RE start counting re-exports. - Documented the known nested-generic limitation (`<T extends Foo<Bar>>`) inline; no occurrences in Tier 1 today. Skipped (design choice, not a bug): - typedoc.json single-key validation block — JSON has no comments and the wrapper shape is a TypeDoc API requirement; further trimming would change behavior. No runtime behavior changes. Coverage: 16% (rounded; 159/970 vs prior 152/937 — the namespace fix expanded the visible denominator).
Codex P2 (`scripts/jsdoc-coverage.js:86`)
EXPORT_RE was anchored at line start with `^export`, so indented exports
inside `export namespace Foo { ... }` blocks were silently skipped. The
morpho-ts `Time` namespace alone has 13+ such members. Loosened to
`^\s*export` so namespace members count toward the burndown. Burndown
denominator: 970 -> 1258 (+288 indented exports now visible across
blue-sdk, blue-sdk-viem, simulation-sdk, bundler-sdk-viem,
liquidation-sdk-viem, migration-sdk-viem, morpho-ts). Added two
positive self-check cases for indented exports.
Devin (`typedoc.json:17`)
`docs/api` is the typedoc output directory but wasn't in `.gitignore`.
Anyone running `pnpm docs:build` would produce HTML/JS files that could
be inadvertently committed (violates AGENTS.md §2.7). Added to
`.gitignore` under "generated docs".
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
Local review pass #6 — convergenceRange: No actionable findings. Targeted review of the latest commit (which loosened
After 6 review rounds + GitHub bot reviews (Codex, Devin) all closed, the PR has converged. Lint passes (including the wired-in `pnpm jsdoc:coverage:check`), the burndown reflects the real Tier 1–4 surface (159/1258 → 13%), and every unresolved review thread is replied + resolved. 🤖 Posted by /ben-local-review |
Foulks-Plb
approved these changes
May 4, 2026
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.
Summary
Establishes the JSDoc coverage rule formally and backfills the public Tier 1 surface to it. No automated gate ships in this PR — automated enforcement is deferred until Biome ships JSDoc rules or a lighter in-repo check emerges.
docs/jsdoc-style.mdas the canonical shape (description → on-chain reads → itemized@paramleaves →@returnsshape →@throwsby class identity → runnable@example), backfillsvaultV1Deposit+marketV1Borrowas reference exemplars, ships apnpm jsdoc:coverageburndown script and an informationaltypedoc.json.vaultV1,vaultV2,marketV1,requirements/*), theMorphoClientclass with its three factory methods,morphoViemExtension, all 45 exported error classes, andevm-simulation'ssimulate()/screenAddresses()/SimulationPackageError.Coverage burndown (
pnpm jsdoc:coverage)The 36% remaining gap in morpho-sdk is entities + helpers — explicitly Phase 2/3 scope per the TIB.
Notable engineering decisions
eslint-plugin-jsdocwas implemented and reverted: it adds a second linter on top of Biome with the explicit goal of replacing one of Biome's responsibilities, and the repo's ajv pin (pnpm.overrides.ajv: 8.18.0) requires brittle scoped sub-overrides for ESLint to coexist. See Considered Alternative 6 for the full rejection rationale.@paramvalidator does not accept the dotted leaf-field notation (@param params.args.amount) that the style guide mandates. See Considered Alternative 7.pnpm docs:checkcontinues to surface the warnings informationally.patchformorpho-sdkandevm-simulation.Test plan
🤖 Generated with Claude Code