Code Review Skill — obsidian-template-plugin
Use this skill to perform structured, thorough code reviews of this repository. Follow the preparation steps, then check each dimension using its checklist.
Preparation
- Identify the base and head refs (e.g.,
git log --oneline HEAD...main, or check the PR's base branch). - Get a compact diff overview:
git diff <base>..<head> --stat. - Read the full diff:
git diff <base>..<head>. - Identify all changed files and their types (
.ts,.json,.mjs,.md, etc.). - For each changed file, read the full file (not just the diff) to understand context.
- Read related integration points: e.g., if
src/terminal/view.tschanged, also readsrc/terminal/load.tsand relevant interfaces.
Review Dimensions
Correctness
- Logic errors: off-by-one, null/undefined dereference, missing early returns.
- State management: does the change properly read, mutate, and persist state? Are race conditions possible?
- Async handling: are
await/.then()used correctly? Are promises caught? - Edge cases: empty states, boundary values, rapid toggling, concurrent access.
- Backward compatibility: default values preserve existing behavior. Old saved data still loads correctly.
Type Safety
- No
ascasts (check project coding conventions — this repo disallows them). - No
anytypes.unknownused where input is untrusted. - Runtime type guards or
.fix()validators used to narrowunknownbefore use. - Public API has explicit return types and parameter types.
- No deeply inferred anonymous types at package boundaries.
Architecture Fit
- Follows existing patterns:
.fix()for settings validation,linkSetting/resetButtonfor UI,Managerclasses for lifecycle. - Uses
SettingsManagerandLanguageManageras shown insrc/main.ts. - Localization uses
createI18n(PluginLocales.RESOURCES, ...)andlanguage.value.t(...). - New files follow the
src/directory layout and naming conventions. - No new architecture introduced where an existing pattern would suffice.
Test Coverage
- Behavioral changes have tests added (per project convention: "test that fails before implementing the change").
- Follows one test file per source file convention under
tests/. - Unit tests (
*.spec.*) are fast, hermetic, BDD-style. - Integration tests (
*.test.*) are isolated and documented. - Tests use
vi.fn(),vi.spyOn(),vi.mocked()per conventions. -
vitest runis used (never watch mode) in automated flows. - Locale keys have existence tests if adding user-facing strings.
Localization
- New keys added to
assets/locales/en/translation.jsonfirst. -
{{...}}placeholders and$t(...)calls kept intact, not translated. - Icon key added to
assets/locales/en/asset.jsonif a settings icon is needed. - Alphabetical ordering maintained in locale JSON files.
- Sync script run:
node scripts/sync-locale-keys.mjsto propagate to all locales. - Description text is clear about behavior (not just label).
- Follows
.agents/instructions/localization.instructions.md.
Build & Scripts
- Changes to
scripts/build.mjshave corresponding tests intests/scripts/build.test.mjs. - Changes to
scripts/obsidian-install.mjshave corresponding tests intests/scripts/obsidian-install.test.mjs. -
manifest.jsonchanges are reflected inversions.jsonif needed. - No hardcoded paths or environment-dependent behavior in scripts.
Documentation
- Changeset added (
.changeset/) with correct bump type (major/minor/patch) and PR reference. - Changeset follows Conventional Commits style.
-
AGENTS.mdupdated if infra, testing conventions, or agent-visible patterns changed. - In-code docs updated if public API changed.
- Commit message follows Conventional Commits: header ≤72 chars, body wrapped at 100 chars.
Regression Risk
- Default values preserve existing behavior (
false/opt-in for new features). - Changes are backward-compatible with saved user data (old
data.jsonstill loads). - Existing tests still pass (run
bun x vitest run). - The change is minimal — no unrelated refactoring or reformatting.
Completeness
- All integration points checked: settings-data (interface, DEFAULT, fix), settings UI, view behavior, locale keys, changeset.
- No TODO comments or placeholder code left in.
- Every changed file is consistent with the overall feature/change.
- The diff
--statshows all expected files and no unexpected ones.
Step-by-Step Workflow
- Preparation — check out the branch, read the diff, read changed files in full context.
- Architecture check — does the change follow existing patterns? Identify any new patterns introduced.
- Correctness pass — trace the logical flow: entry point → data flow → output. Check edge cases.
- Type safety pass — scan for
as,any, missing type guards. - Test pass — verify tests exist and follow conventions. Check the actual test content if present.
- Localization pass — verify locale keys, sync, and ordering.
- Documentation pass — verify changeset, commit messages, AGENTS.md updates.
- Regression check — are defaults safe? Are old behaviors preserved? Do existing tests pass?
- Completeness check — re-read the diff stat and confirm every piece is accounted for.
- Report — produce a structured review with:
- Overview of the change
- What's done well
- Issues found (with severity and file:line references)
- Concrete fix recommendations
- Completion checklist (✅/⚠️/❌)