NoteThink Coding Standards

September 3, 2026 · View on GitHub

This document defines the coding standards for the NoteThink project. It is the authoritative rulebook: read it end-to-end at the start of any session that touches notethink, and re-read the relevant section before any high-consequence action (commit, push, merge, version bump, refactor, new-file placement) rather than recalling from memory. Don't grep for keywords mid-task in place of reading it, and don't fall back to training-data defaults when a rule seems missing - if an expected rule is absent, flag the gap and ask whether to add it here. Memory files are thin pointers into this document; if a rule belongs anywhere, it belongs here.

Workspace-wide process (story state machine and tracking format, version bumps, commit policy, git workflow, who drives shipping, the never-write-with-git rule, releaseable-state gate, test-failure discipline, edit verification, dev-server lifecycle, browser-snapshot cleanup) lives in ../AGENTS.md.

Shared coding standards live in lightenna-iac/docstech/standards/ (index). Read the one that matches the work:

DoingRead
naming anythingNAMING.md
placing imports / types / constants, structuring a fileCODE_LAYOUT.md
writing types or propsTYPESCRIPT.md
adding a log line, debug statement, or error pathLOGGING.md
writing or changing a testTESTING.md
verifying before handbackVERIFICATION.md

notethink is a VS Code extension, so NEXTJS.md, SECURITY.md and DATABASE.md do not apply: no App Router, no server actions, no lib/lib-client split, no database. It also has no dev server - see Pre-Push Verification below.

notethink holds the workspace's worked examples for <noun>ops.ts library organisation and is the reference write-up for that rule. The rule itself is in ../AGENTS.md.

Where this file restates a shared rule below, the shared document is canonical and this file keeps only notethink's scope carve-outs - of which there are several real ones, because the extension host and the webview are different runtimes with different logging stacks.

Naming Conventions

Variable Naming

Use snake_case for local variables, hook returns, and computed values.

// correct
const user_name = 'Alice';
const is_valid = checkValidity();
const total_count = items.length;
const display_options = props.display_options;
const note_data = useNoteData();

// incorrect
const userName = 'Alice';       // should be snake_case
const isValid = checkValidity(); // should be snake_case
const totalCount = items.length; // should be snake_case

Function Naming

Use camelCase for functions and event handlers.

// correct
function calculateTotal() { }
function handleClick() { }
function parseMarkdown() { }
const onSubmit = () => { };

// incorrect
function calculate_total() { }  // should be camelCase
function handle_click() { }     // should be camelCase

Props and Parameters

Use camelCase for component props and function parameters.

// correct
interface ButtonProps {
    onClick: () => void;
    isDisabled: boolean;
    buttonText: string;
}

function processNote(noteId: string, isVisible: boolean) { }

// incorrect
interface ButtonProps {
    on_click: () => void;    // should be camelCase
    is_disabled: boolean;    // should be camelCase
}

Exception - snake_case data fields and the parameters that carry them. This codebase follows a deliberate snake_case for data, camelCase for behaviour split. Fields on the note/view data shapes and message payloads (NoteProps, ViewProps, NoteOrigin, the *Message types) use snake_case - e.g. doc_path, relative_path, workspace_root, aggregate_total_discovered, include_filter - because they mirror the serialized extension↔webview wire format, which is snake_case end-to-end. Renaming them to camelCase would split the field name from its on-the-wire key. Function parameters that receive these data values therefore also stay snake_case (function processNote(note_id: string)), consistent with the local-variable rule. What stays camelCase: genuine UI event-handler props (onClick, onSubmit, onNoteChange) and function/callback names (including service-callback props like setViewManagedState, postMessage - these are functions, named per Function Naming, not on* UI events).

Types and Interfaces

Use PascalCase for types, interfaces, and components.

// correct
type UserRole = 'admin' | 'user';
interface NoteProps { }
type ViewOptions = { };
class DocumentParser { }
function DocumentView() { }

// incorrect
type userRole = 'admin' | 'user';  // should be PascalCase
interface noteProps { }             // should be PascalCase

Constants

Use SCREAMING_SNAKE_CASE for true constants (values known at compile time).

// correct
const MAX_RETRY_COUNT = 3;
const API_BASE_URL = 'https://api.example.com';
const DEFAULT_TIMEOUT_MS = 5000;

// note: computed values at runtime are not constants
const user_count = users.length;  // snake_case, not a constant

Permanent name check

This rule only applies to names that get stored in some permanent or semi-permanent state outside the notethink codebase. Internal names (variables, function names, types, in-memory data shapes, transient wire-format messages between the extension and webview bundles) do not need the check - pick a name and rename freely later.

The check applies because the names below land in a store this codebase can't unilaterally rewrite. A later rename requires either a migration path for existing data or simply breaks any user / tool that already pinned the old name:

  • VS Code (or any host) config keys (notethink.settings.X.Y) - written to settings.json on disk, on machines we don't own
  • Persisted-state keys (vscode.setState shapes, IndexedDB key names, JSON file keys on user disk)
  • Database table / column names; cookie / header names; URL path segments
  • Public API names that code outside this repo will import (anything exported from a published npm package)
  • File / directory names that other tools, scripts, or workflows target (workflows, glob patterns, lockfiles, manifests, schema files)

Before introducing or renaming anything in those categories, surface the candidate to the user and get explicit sign-off.

Why this rule exists. A config namespace named for the single view its settings happen to serve (notethink.folderView.*) stops describing them the moment the scope grows to every view type - and by then the name is already on disk in users' settings.json, so correcting it costs a rename pass across the codebase plus a user-facing migration wherever there are external users. A 30-second naming check at the moment of introduction is what prevents it. When in doubt, default to asking - the question is cheap, the rename is expensive.

stable_id field: implicit vs explicit provenance

Every note has a stable_id field (the field name is fixed). Its value has two grades:

  • Implicit (transient) - derived from the headline via storyStableIdSlug; present at parse time, never written to the file. Tracks the title, so it changes on rename. Use for React keys, kanban drag projections, and any intra-session matching.
  • Explicit (persistent) - the authored [](?id=slug) linetag; frozen once written, survives renames and reloads. The only grade safe for durable cross-session references.

Implementer rule: transient code uses the implicit derived id and writes nothing to the file. Write an explicit id= linetag only when a durable artifact (a [[…]] cross-reference, a user-authored link) needs to survive across sessions or renames. See AUTHORING_GUIDE.md - Stable ids: implicit vs explicit for the full decision rules.

A seq is valid only within the render pass that derived it

note.seq is a document-order index, reassigned from scratch on every parse and renumbered globally by mergeAggregateRoot whenever the per-file trees are re-interleaved. It addresses a position in the current tree, not a note.

The rule: the moment a seq outlives a re-parse, it is wrong. If a value crosses an update boundary - cached in a useMemo, written to view-managed state, stashed in a ref, or emitted into the DOM to be looked up by a later effect - it must be a stable_id (identity) or a source offset (position), never a seq. Resolve it back to a seq on read, against the tree you are about to render.

Same-pass use is fine and is the cheapest option: derive a seq, use it, discard it. focused_seqs and resolveCaretTarget's v<view>-n<seq> element id are correct because the derivation and the consumption happen in one render.

A React key only looks same-pass. Reconciliation matches this render's key against the previous render's, so a key crosses an update boundary like any other stored value: a bare seq key hands a renumbered note the previous occupant's component instance, and with it that instance's state. DocumentView and renderBodyItems therefore key by stable_id ?? seq - the ?? seq is unreachable defensive cover, since both stamping walks in mergeAggregateRoot reach every note, and it must not be read as licence to key by seq. The raw mdast body nodes in renderBodyItems are the exception that proves the rule - they have no stable_id, so they key by source offset, which is a position rather than a re-derived index.

There is no mechanical check for this. The tell is a seq on the right-hand side of an assignment that outlives the render: a dependency array, a setViewManagedState payload, a useRef, a data- attribute paired with a stored lookup key.

Why this rule exists. It has been rediscovered three times.

  1. Persisted focus/selection stored raw seqs, so a drag-reorder made the highlight jump to whichever note inherited the number. Fixed by storing stable_ids (view_focused_ids / view_selected_ids, see writeViewInteractionState).
  2. useMarkdownNoteBodyScroll cached the first-incomplete-task seq behind a body_raw key. seq is not a function of body_raw, so an edit to any other file in a folder-mode board renumbered the tree and left the card framing an arbitrary completed item further up the story. Fixed by deriving it per render.
  3. parent_context_seq was persisted as the view's note-hierarchy scope, so an edit above the scoped heading silently re-rooted the view on a different note. Fixed by persisting parent_context_id and re-resolving it through resolveParentContextNote.

Each was a real user-visible bug, each looked like a caching or scrolling problem rather than an identity problem, and each cost far more to diagnose than to fix.

Import Organization

Import Placement

  • All import statements must be at the top of the file, before any non-import code
  • The only exception is dynamic import() expressions used inline, which must include a comment explaining why a static import is not suitable

Import Grouping

Import grouping is freeform workspace-wide (operator decision 2026-06-10). Only the positional rules hold: a directive first, then import 'server-only';, then everything else, with one import statement per module. See CODE_LAYOUT.md > Import organisation, which also says why a prescribed grouping order must not be reintroduced here.

One house convention, not a requirement: where a webview file carries import Debug from "debug", it leads the list, and LOGGING.md imposes no position mandate either way.

Correction, 2026-09-02. This section prescribed an eight-tier grouping order with a worked example, which CODE_LAYOUT.md retired on 2026-06-10 and asks project docs not to reintroduce.

Code Style

Function Length and Decomposition

  • Keep functions short - aim for ≤ 30-35 lines of body. When a function grows past that, break it into meaningful, well-named sub-functions rather than letting it sprawl. The threshold is guidance, not a hard cap: a flat data literal or an unavoidable single dispatch can run longer, but branching logic that spills past ~35 lines is almost always hiding extractable steps.
  • Extract by responsibility, not by line count. Each sub-function should do one nameable thing. Split where there is a genuine seam (validation → transformation → output, or a self-contained sub-algorithm), not at an arbitrary line picked to satisfy the number.
  • The caller should read as a sequence of named steps. After decomposition the top-level body reads like a table of contents - firstInvalidChange(...), logEditTextChanges(...), applyEditTextChanges(...) - so a reader follows the flow without diving into every detail. Well-named calls replace narration.
  • Favor function-header comments over interleaved line comments. Put the explanation (what, why, invariants) in the block comment immediately above the function. Both header and inline comments are permitted, but the header is the default home for anything longer than a one-line local note. If you find yourself narrating each step inline, that is a signal the steps want to become named sub-functions - inline comments should stay sparse because the code already reads logically.
  • The point is explicit inputs/outputs and testable seams, not the number. Line count is the symptom you watch; the cause you fix is shared mutable state and implicit dependencies. A 40-line function whose every input is a parameter and whose output is a return value is fine; a 25-line function reaching into ten ambient mutable variables is not. Optimise for "I can read this unit's signature and know its whole contract," and for "I can unit-test it without standing up its whole surrounding context."
  • Closures-as-objects should be real classes. When a long function owns several pieces of mutable state that many inner closures read and write (a per-session/per-panel controller written as one big function), the length is a symptom of missing encapsulation: every nested closure has ambient access to everything, nothing has a contract, and the unit is untestable except through its outer boundary. Promote it to a class - the mutable state becomes private fields, the closures become methods, and each method's dependencies become explicit (this.x). This is the preferred fix for stateful non-React closures.
  • In React, decompose with custom hooks and child components - not plain helpers. The Rules of Hooks forbid lifting a block that calls useState/useEffect/useMemo into an ordinary function. Extract a cohesive hook+derivation cluster into a custom hook (useFolderDocs()), and a JSX subtree into a child component (<KanbanBoard>). A long component body is shortened by moving these out, never by splitting at an arbitrary line.
// avoid - one function doing validation, logging, application, and dispatch inline
async function applyEdit(doc_path, changes) {
    // ...60+ lines mixing offset checks, audit logging, edit application, re-emit...
}

// prefer - the caller reads as named steps; each helper has a header comment
async function applyEdit(doc_path: string, changes: TextChange[]): Promise<void> {
    if (!isWithinWorkspace(doc_path)) { return; }
    const invalid = firstInvalidChange(changes, doc_length);
    if (invalid) { logRejection(invalid); return; }
    logEditTextChanges(document, doc_path, changes);
    await applyEditTextChanges(document, uri, changes);
    reEmit(document);
}

Block Organization

No blank lines within function bodies unless separating logical sections:

// correct
function processNote(note: NoteProps) {
    const note_id = note.id;
    const processed_content = parseContent(note.content);
    return { id: note_id, content: processed_content };
}

// correct (logical sections)
function complexProcess(data: Data) {
    // validation
    const is_valid = validateData(data);
    if (!is_valid) return null;

    // transformation
    const transformed = transformData(data);
    const enriched = enrichData(transformed);

    // output
    return formatOutput(enriched);
}

Comments

See workspace ../AGENTS.md > Code conventions > Comment style for the full rules: single-line comments are lowercase-first while multi-line and header blocks take normal capitalisation, one comment is exactly one line and never two // lines in a row, keep inline comments short (~100 chars), no back-reference comments, no project-management version numbers, and the TODO format (// TODO: description, // TODO(#123): description). The no-consecutive-line-comments rule is mechanically enforced in this repo by the local local/no-consecutive-line-comments ESLint rule.

notethink-specific extras:

  • never use the em or en dash character anywhere in this repo - see workspace ../AGENTS.md > Code conventions > Dashes
  • no period at end (unless multiple sentences)
  • the lowercase-first and no-trailing-period rules are about the // single-line form; a /* */ or /** */ function-header block is prose and takes normal capitalisation and punctuation
  • No per-field comments inside data structures. See workspace ../AGENTS.md > Code conventions > No per-field comments inside data structures.
    • Exception - section dividers. Short comments that group related fields by purpose (e.g. // --- identity ---, // --- mdast passthrough ---, // --- tree links ---, // --- runtime decoration added at React render stage ---) are explicitly permitted and load-bearing as visual structure in long interfaces. Don't strip these when sweeping a data structure for comment violations. Classify each inline comment: a per-single-field explanation gets removed (lift to the type's header comment if useful); a comment that introduces ≥2 conceptually-grouped fields with a single purpose stays. A comment grouping the absence of related fields (e.g. // doc_path/doc_relative_path/doc_text intentionally undefined for the merged view above three undefined assignments) still qualifies as a section divider.
// correct
// calculate tax based on region
const tax_amount = calculateTax(subtotal, region);

// correct (multiple sentences)
// this handles edge cases for empty arrays. it also validates input.
const result = processItems(items);

// incorrect
const tax_amount = calculateTax(subtotal, region); // Calculate tax
const result = processItems(items); // This handles edge cases

/**
 * Correct - a function-header block is prose, so it takes normal capitalisation.
 */
function processItems(items: ItemList): ItemResult {}

Braces and Blocks

Always use braces for control structures:

// correct
if (is_valid) {
    processItem();
}

// incorrect
if (is_valid) processItem();
if (is_valid)
    processItem();

TypeScript Guidelines

Explicit Types

Prefer explicit types over inference for function parameters and returns:

// correct
function calculateSum(values: number[]): number {
    return values.reduce((sum, val) => sum + val, 0);
}

// acceptable (simple arrow functions)
const double = (n: number) => n * 2;

// avoid
function calculateSum(values) {  // missing types
    return values.reduce((sum, val) => sum + val, 0);
}

Avoid any

Use specific types or unknown instead of any:

// correct
function processData(data: unknown): void {
    if (typeof data === 'string') {
        handleString(data);
    }
}

interface ApiResponse {
    data: Record<string, unknown>;
}

// incorrect
function processData(data: any): void { }

Type-Only Imports

Use type-only imports when importing only types:

// correct
import type { NoteProps } from '@/types/NoteProps';

// also acceptable (mixed import)
import { NoteComponent, type NoteProps } from '@/components';

Loop Safety

  • Avoid potentially infinite loops (do-while, while without bounds)
  • Use bounded for-loops with a maximum iteration count instead
  • Exception - while driven by a strictly-progressing finite iterator. A while loop whose continuation is a finite iterator that cannot stall is acceptable without an explicit counter: regexp.exec(str) with a non-zero-width pattern (lastIndex advances each match), TreeWalker.nextNode() (walks a finite DOM and returns null at the end), and a stack/queue drain that only pop()s (strictly shrinks). These terminate by construction; a MAX_ITERATIONS guard would be defensive clutter. The rule targets loops that could run forever (unbounded conditions, zero-width regex matches) - those still need a bound.

Type Placement

  • File-level type definitions (type Foo = ..., interface Bar {...}) should be declared near the head of the file, below imports and above constants/functions
  • Do not declare types in the middle of a file, interleaved with function definitions - even a small helper type used by one function belongs at the top with the others, so the reader has a single place to find all type shapes
  • If a type is only used in one file and has no external consumers, keep it in that file rather than splitting into types/; if it outgrows the file (many variants, many consumers), lift it to src/types/
  • Discriminated-union result types (e.g. type FetchResult = {kind: 'found'; ...} | {kind: 'not_found'}) count as type definitions and must sit at the top too, not above the function that happens to return them

Constants Placement

  • File-level constants (e.g. const MAX_ITERATIONS = 1000) should be declared near the head of the file, below imports and type definitions
  • This makes them easy to find, reuse, review, and change
  • Do not declare the same constant inline within multiple functions - hoist it to module level

Debug Logger Pattern

Canonical: LOGGING.md. debug is an optional development aid: put it where it earns its place, leave it out everywhere else, and never add or remove one as a review action. notethink carries one deviation from the shared rule, and enforces unused bindings mechanically:

  • Deviation - the namespace is area-based, not a directory path. nodejs:<area>:<File>, where <area> is the bundle or package the file belongs to (notethink for the webview app, notethink-views for the component library) and <File> is the source basename - e.g. Debug("nodejs:notethink-views:KanbanView"). The shared rule's nodejs:{path}:{filename} form does not fit a repo whose source tree and bundle boundaries differ.
  • Enforcement - @typescript-eslint/no-unused-vars is configured at "error" in eslint.config.mjs, with ^_ ignore patterns for arguments, variables and caught errors, matching the shape LOGGING.md prescribes. An unused const debug therefore fails lint like any other unused binding, rather than needing to be spotted by eye.

Scope - primarily the webview. This pattern (the npm debug library + nodejs: namespace) is the convention for the webview / notethink-views bundles. The extension host (client/extension/**) logs through writeToLog / writeToErrorLog (winston, in lib/errorops.ts) instead, so reaching for Debug there generally means reaching for the wrong stack.

One live exception: client/extension/src/lib/pathops.ts:1 does import Debug from "debug". Either it should move to writeToLog, or this rule is narrower than "webview only" - decide rather than leaving the doc asserting a clean state the tree contradicts. (Found 2026-08-06; the rule previously stated the extension host does not use debug, as an absolute.) Pure type-definition modules (files containing only interface / type declarations, e.g. types/NoteProps.ts) carry no debug either: they have no statements to instrument and a runtime import would defeat their erasability.

Extension Points

See workspace ../AGENTS.md > Code conventions > Extension points (keep single-case switch/if dispatches and trivial-bodied helpers that mark meaningful extension points, rather than collapsing or deleting them).

React Patterns

Component Structure

import Debug from 'debug';
import React, { useState, useEffect } from 'react';
import type { ComponentProps } from './types';
import styles from './Component.module.scss';

const debug = Debug("nodejs:notethink:ComponentName");

interface Props {
    id: string;
    title: string;
    onAction?: () => void;
}

export default function ComponentName(props: Props) {
    // hooks first
    const [is_loading, setIsLoading] = useState(false);
    const computed_value = useMemo(() => expensive(props.id), [props.id]);

    // effects
    useEffect(() => {
        debug('component mounted');
        return () => debug('component unmounted');
    }, []);

    // handlers
    const handleClick = useCallback(() => {
        setIsLoading(true);
        props.onAction?.();
    }, [props.onAction]);

    // early returns
    if (is_loading) {
        return <div>Loading...</div>;
    }

    // render
    return (
        <div className={styles.container}>
            <h1>{props.title}</h1>
            <button onClick={handleClick}>Action</button>
        </div>
    );
}

Hook Return Values

Hook return values follow snake_case:

// correct
const user_data = useUserData();
const [is_open, setIsOpen] = useState(false);
const note_state = useNoteState(props.id);

// incorrect
const userData = useUserData();  // should be snake_case

Event Handler Props

Event handler props use camelCase with on prefix:

// correct
interface Props {
    onClick: () => void;
    onSubmit: (data: FormData) => void;
    onNoteChange: (note: Note) => void;
}

// in usage
<Button onClick={handleClick} onHover={handleHover} />

View interaction state: latest-click-wins with the editor as tiebreaker

Per-view UI interaction state has two layers, and the editor wins ties. The view writes its own click/drag/hover state directly to display_options (or a peer slot in view-managed state) so a gesture lands immediately - no waiting for an editor-selection round-trip. But the editor-derived match (caret-in-note via the per-doc + source-position matcher) is the source of truth whenever it produces a result: almost all real editing happens in the editor, and the view is a real-time visualisation that should reflect editor activity instantly. The view-driven layer is the immediate-feedback bridge for the brief window between a view click and the editor's selectionChanged round-trip, and the fallback when the editor has no opinion (active editor on a doc that isn't in the aggregated set, or caret outside every matched note).

Two non-negotiable consequences for any view→editor gesture (click, keyboard nav, clear):

  • Editor takes DOM focus. revealRange / selectRange posted by the view must route through vscode.window.showTextDocument(..., { preserveFocus: false }) so the editor - not the webview - receives subsequent key events. The webview never captures keystrokes the user expects to land in the editor.
  • The view follows the editor, not the other way round. If the editor moves to a different note (the user clicked in the editor, an external file edit moved the caret, anything), the view's focused/selected state updates to match on the next derivation - even if the user previously clicked a different note in the view.

Why this matters: the view aggregates from N source files (folder mode); the editor only ever has one active doc. Driving view-interaction state through the editor selection alone silently breaks the moment the rendered tree contains anything outside that single doc - the round-trip never confirms and the view never updates. Pinning view focus on the most-recent view click instead silently breaks the moment the user starts editing in the editor - the visualisation goes stale. The fix is structural: write view state directly from the user's gesture for immediate feedback, then let the editor-derived match override as soon as it has an answer. See client/webview/src/notethink-views/src/components/views/generic/useViewHandlers.ts (click dispatcher writes view_focused_seqs / view_selected_seqs directly via setViewManagedState) and useViewContext.ts (resolveFocusedNote prefers editor-derived; view-driven fills in when the editor has no opinion) for the canonical pattern.

The same principle applies in reverse for editor-driven decoration: when the editor caret should highlight a note, the matcher must work across however many docs the view is aggregating from. Don't write a matcher that assumes a single coherent coordinate space - use per-doc origin metadata (e.g. origin.doc_path + origin.source_position) so the unified algorithm works in both current_file (trivial: all notes share one doc) and folder (N-doc merge with synthetic offsets) modes without an integration_mode branch.

Focused-note scroll framing

When a note becomes focused/selected and useScrollToCaret (viewhooks.ts) scrolls it into the viewer, the framing rule is:

  • The focus/selection highlight ring must be visible all the way around the note - never cropped on any edge. The ring is an outline with offset + width that getBoundingClientRect excludes, so the scroll must reserve space for it (measure against the actual scroll-container client size, not window.innerWidth/innerHeight).
  • When the note fits the viewer (not wider AND not taller than the viewer's client area): show the whole note, ring included.
  • When the note is wider OR taller than the viewer: anchor to the top and the left (show top-left), since the whole thing can't fit.
  • Within-note caret reveal (scrolling the clipped body to the caret line) is owned by useMarkdownNoteBodyScroll, not this hook - this hook only positions the whole story in the viewer plus its horizontal scroll container.

The recurring regression is the ring being cropped on the left edge when a card sits flush against the scroll container's edge; verify by focusing a card in the leftmost column - the left ring must show.

File Organization

Directory Structure

notethink has no root src/. It has three source roots, and knowing which one you are in is the first thing to establish:

RootWhat it is
client/extension/src/the extension host - winston logging, VS Code API, no DOM
client/webview/src/the webview app - React, debug logging, no fs
client/webview/src/notethink-views/src/a nested package with its own package.json, rollup.config.js, tsconfig and node_modules

Correction, verified 2026-08-11. The lib/ listing below carried parseops.ts, which is not in this tree - it is at client/extension/src/lib/parseops.ts, the other source root. That is the same root-confusion this block's own note says was already fixed once, so it is now marked explicitly as a sample rather than an inventory: ls the directory, which currently holds axisops, convertMdastToNoteHierarchy, frontmatterops, globMatch, groupbyops, linetagops and more besides the four named.

The tree below is client/webview/src/notethink-views/src/ specifically - components/views/DocumentView.tsx, components/notes/GenericNote.tsx, types/NoteProps.ts and types/ViewProps.ts exist there and nowhere else. (An earlier version headed this block a bare src/, which matches no directory in the repo.)

client/webview/src/notethink-views/src/
├── components/
│   ├── views/           # view-level components
│   │   ├── DocumentView.tsx
│   │   └── DocumentView.test.tsx
│   └── notes/           # note-level components
│       ├── GenericNote.tsx
│       └── GenericNote.test.tsx
├── lib/                 # utility functions, grouped by domain (*ops.ts)
│   ├── noteops.ts       # note traversal, position, classification
│   ├── originops.ts     # origin / project identification + colour
│   ├── pathops.ts       # path segmentation, workspace-root derivation
│   └── viewstateops.ts  # view-managed-state operations
│                        # (this listing is a sample, not an inventory - run `ls` on the directory)
├── types/               # shared types
│   ├── NoteProps.ts
│   └── ViewProps.ts
└── styles/              # global styles
    └── variables.scss

File Naming

  • Components: PascalCase.tsx (e.g., DocumentView.tsx)
  • Domain lib files: <noun>ops.ts - see Library organisation below.
  • Single-export utility modules: camelCase.ts when the filename mirrors the primary exported function - convertMdastToNoteHierarchy.ts, mergeAggregateRoot.ts, globMatch.ts. This is the accepted alternative to the *ops.ts pattern when a file's only purpose is one named operation. Pick one style per module and keep it stable.
  • Component-local helper files: keep them next to the component they serve (e.g. components/views/kanban/kanbanDragEndPayload.ts), not in lib/. The path itself signals "not a general op". Filename follows the single-export camelCase rule above.
  • Types: PascalCase.ts (e.g., NoteProps.ts)
  • Tests: *.test.tsx or *.test.ts next to source file
  • Styles: Component.module.scss (CSS modules)

Library organisation

Files under lib/ group pure utility functions by domain, named <noun>ops.ts (single concatenated word, no hyphen, no separator). The convention is project-agnostic - apply it whenever you reach for a lib/ file in any JS/TS project.

The pattern:

  • noteops.ts - operations on Note shapes (traversal, position checks, classification, chain construction; also generic array helpers when their only consumers are note-adjacent)
  • originops.ts - operations on Origin / project metadata (label derivation, hue, identification, folder derivation)
  • pathops.ts - path string operations (segmentation, containment, workspace-root derivation)
  • viewstateops.ts - operations on view-managed state (canonical-key resolution, state-update builders, mode detection)
  • docops.ts - operations on Doc shapes (picking, merging, abbreviating)
  • editops.ts - operations on text edits (change validation, audit logging)
  • cryptoops.ts - hashing, nonces, identifiers
  • vscodeops.ts - VS Code API wrappers + persisted state shape (project-specific example)

The rules:

  1. Group by the noun the operations act on, not by where the code is called from. A function that walks notes goes in noteops.ts regardless of which component imports it. A function that builds a view-state payload goes in viewstateops.ts even if only one site calls it today.
  2. Pure functions only. No React hooks, no JSX, no useState/useEffect/useRef. Closures over component state belong in the component or a custom hook.
  3. Prefer extending an existing *ops.ts over creating a new one. A new helper that operates on Note extends noteops.ts; create notetreeops.ts only when the existing file's domain genuinely splits.
  4. Aim for ≥ 4 exports per *ops.ts file. Rationalise periodically. A standalone file with one or two exports is usually over-engineered - fold it into a closely-related *ops.ts that shares the same noun cluster or consumes/produces the same type. Worked examples from this codebase: the generic arraysEqual<T> helper lives in noteops.ts because both its consumers are note-adjacent, and its docstring records that it lifts back out the moment a non-note caller appears; VSCodeState + migrateSavedState live in vscodeops.ts because the wrappers there produce and consume the type. When you finish a refactor that leaves any *ops.ts with fewer than 4 exports, do the merge pass in the same session - small files accumulate fast and the rationalisation cost grows with every new contributor who imports them. The target is guidance, not a hard floor: a 1-export file with a genuinely distinct noun and plausible future growth (e.g. docops.ts holding only pickMostRecentlySentDoc) can stay if you can name the second function that's coming.
  5. One domain per file. A "junk drawer" utils.ts is the anti-pattern this convention exists to prevent - when you find one, split it by the noun each export operates on (crypto.ts + getNonce from utils.ts → one cryptoops.ts; abbrevDoc from utils.tsdocops.ts).
  6. Mirrored constants across bundle boundaries are the documented exception. If two bundles (e.g. extension host + webview) cannot share a module graph, a small set of constants may be byte-identical-duplicated in two constants.ts files. Don't *ops.ts-ify either side; the duplication is the wire contract. See Avoid Duplication below.
  7. Component-local helpers stay under components/, not under lib/. If a pure helper is genuinely only ever used by one component family and lifting to lib/ would imply general reusability that doesn't exist, leave it co-located with the component. The path is the signal.
  8. Tests colocated next to the lib file (noteops.test.ts next to noteops.ts).

Why this matters: file-name-as-domain-label makes it obvious where to look for a function and where to add a new one. The alternative - utils.ts / helpers.ts / scattered parse-X.ts / mixed-purpose modules - defeats grepability and leads to duplication (two files independently growing helpers for the same noun because neither author found the other). <noun>ops.ts is the cheap convention that prevents both. The ≥ 4-exports target keeps the opposite failure mode - proliferation of tiny single-helper files - in check.

When to keep a non-*ops.ts name: a single-function file whose name reads as a sentence (convertMdastToNoteHierarchy.ts, mergeAggregateRoot.ts, globMatch.ts) - the filename mirrors the primary export and lifting to a *ops.ts would muddle the "one file = one named operation" signal. These are explicitly permitted and the ≥ 4-exports target does not apply to them; do not rename them.

Code Quality

Avoid Duplication

  • Extract repeated code into shared utility functions

  • Use shared constants instead of hardcoding values

  • Create shared components for repeated UI patterns

  • Exception - mirrored constants across the extension/webview bundle boundary. A small set of folder-view defaults is intentionally duplicated in client/extension/src/constants.ts and client/webview/src/constants.ts. The two run as separate webpack bundles with no shared module graph, so there is no import path to a single source; the duplication is the wire contract.

    The contract is that the shared subset agrees, not that the files are identical. Shared and required to match: the include/exclude glob defaults and DEFAULT_COLUMN_ORDER. Deliberately one-sided: MAX_AGGREGATE_FILES, INTEGRATION_MODE_CURRENT_FILE / _FOLDER and NOTETHINK_VIEW_TYPE are extension-only; DEFAULT_MAX_NOTES_PER_FILE is webview-only and its own comment says it is "not round-tripped to the extension". Cross-reference the shared ones in a comment on both sides.

Correction, 2026-08-06. This previously said "keep the two copies byte-identical" and listed max-notes-per-file among the mirrored constants. diff shows the files are not close to identical, and max-notes-per-file exists only in the webview copy. Following the old instruction literally would have pushed four constants across a boundary each was deliberately kept on one side of.

Remove Unused Code

  • Remove unused imports
  • Remove unused variables (check compiler warnings)
  • Remove commented-out code that's no longer needed
  • Exception - a leading underscore marks a binding that is deliberately unused. @typescript-eslint/no-unused-vars is configured with ^_ ignore patterns for arguments, variables and caught errors, so _name opts out of the rule. Use it only where the binding is evidence of something - an unwired format, a captured value whose missing assertion is a tracked bug, a parameter kept as an extension point - and always with a comment saying what it is evidence of and which story tracks it. Everything else still gets deleted.

Error Handling

Use Error Utilities

Extension host logs through writeToLog / writeToErrorLog (winston, client/extension/src/lib/errorops.ts). Webview has no winston/output-channel access, so it logs through the debug library instance (const debug = Debug("nodejs:...")) and, for render failures that should reach the host, posts a renderError message to the extension. Either way, console.* is not the error utility - see "No console.log in committed code" in the workspace AGENTS.md. A caught error that is intentionally non-fatal must still be logged (debug('… %O', err) in the webview, writeToErrorLog(...) in the extension), never silently swallowed.

// extension host
import {writeToErrorLog} from '@/lib/errorops';

try {
    await riskyOperation();
} catch (error) {
    writeToErrorLog('pathops', 'riskyOperation failed', error);
}
// webview - no winston here, so log through the debug instance
const debug = Debug("nodejs:notethink-views:DocumentView");

try {
    await riskyOperation();
} catch (error) {
    debug('riskyOperation failed %O', error);
}

The real exports of client/extension/src/lib/errorops.ts are initLogDir, isRedirect, fatalError, nonFatalErrorReport, nonFatalErrorInternally, writeToLogAtLevel, writeToLog, writeToErrorLog and debug.

Correction, 2026-08-06. The sample here previously imported createError and handleError from @/lib/errorops. Neither function exists, so the snippet in the section titled "Use Error Utilities" did not compile - while the prose directly above it correctly named writeToLog / writeToErrorLog. A copy-paste-ready sample that does not compile is the most directly harmful kind of doc error; found by adversarial verification.

Reading VS Code logs

The primary, CLI-friendly log is notethink-extension.log, written to the extension's standard VS Code log directory - vscode.ExtensionContext.logUri, the canonical per-extension log location. Never the user's open workspace folder (a shipped extension must not litter the user's project with log files). Every writeToLog / writeToErrorLog call (and logEditTextChanges) is mirrored there by the file logger in errorops.ts (initLogDir(context.logUri) in activate()).

logUri resolves under the rotating per-session logs dir. NoteThink is a web-worker extension (publisher NoteThink.notethink), so on Linux it is:

~/.config/Code/logs/<session-timestamp>/window<N>/exthost/webWorker/NoteThink.notethink/notethink-extension.log

(macOS: ~/Library/Application Support/Code/logs/...; the OS-standard logs root differs but the …/exthost/webWorker/NoteThink.notethink/ tail is the same.) Reading these needs dangerouslyDisableSandbox: true (under ~/.config/~/Library). Find and tail the live one:

# resolve the latest session + window that actually has our log, then tail it
LOG=$(ls -t ~/.config/Code/logs/*/window*/exthost/webWorker/NoteThink.notethink/notethink-extension.log 2>/dev/null | head -1)
echo "$LOG"; tail -f "$LOG"
grep -E "editText|caret-probe" "$LOG"
  • Dev-only, off by default. Gated by the NOTETHINK_DEV webpack define, which is process.env.SELFINSPECT_ENV === 'dev' (the workspace-standard env marker - never NODE_ENV). The build / watch scripts export SELFINSPECT_ENV=dev to opt in; every other build (the marketplace package build, and any hosted/web build) leaves it unset, so the file logger is stripped (if (true) { return; } → dead-code-eliminated). Off-by-default is deliberate: a shipped extension must never silently fill a user's disk with logs. The Output-panel channel below still works in production.
  • Rolling buffer. Last 500 lines, flushed ~1s after a write, written wholesale (overwritten each flush, not appended).
  • If missing/empty: production build (no NOTETHINK_DEV), the window wasn't reloaded after a rebuild (so activate/initLogDir didn't re-run), or nothing has logged yet. A new session dir is created each time VS Code starts, so always re-resolve the latest with the ls -t one-liner - don't cache the path.
  • Spurious in-repo copies. **/notethink-extension.log is gitignored ("ignore spurious logs"). The logUri path above is the only authoritative runtime log - if you find a notethink-extension.log in the repo root or a parent dir, it's stale leftover litter, not the live log; don't read it, delete it.
  • Desktop-only path. The file logger writes via vscode.workspace.fs.writeFile(logUri, …), so the tailable on-disk notethink-extension.log exists only in desktop VS Code. In a web host (any vscode-web build serving the extension) workspace.fs writes to browser/virtual storage with no terminal-readable file - there, read runtime/webview logs from the browser devtools console instead.

Two log streams, two homes - don't conflate them. The notethink-extension.log above is the runtime / behaviour log (what the running extension did). It is distinct from the build / watch log (webpack compile output: did the bundle compile, with what errors):

StreamHomeHow to read
Runtime / behaviourlogUri (the …/NoteThink.notethink/notethink-extension.log above)ls -t one-liner, dangerouslyDisableSandbox
Build / watch (webpack)test-results/dev.log in the repo (gitignored)tail -f test-results/dev.log

test-results/dev.log is where /open-dev redirects the pnpm run watch (webpack --watch) output, matching the cross-project test-results/dev.log convention. Use it to confirm a clean compile after edits; use the logUri log for runtime behaviour. Never route build output to /tmp - it's ephemeral and breaks the "one documented place per stream" narrative.

Interactive alternative - the Output panel. writeToLog also feeds the "NoteThink" LogOutputChannel (winston via LogOutputChannelTransport), visible under View → Output → NoteThink. VS Code persists that channel to NoteThink.log in the same logUri directory as above (subject to the channel's selected log level, so it is sometimes empty - prefer notethink-extension.log for CLI reads).

The debug library (webview only - const debug = Debug("nodejs:...")) is not captured to any file. Enable it via localStorage.debug = 'nodejs:*' in the webview Developer Tools console. Webview console.error() / console.warn() is also not file-captured - only visible in "Developer: Open Webview Developer Tools".

Avoid Silent Failures

// correct
function parseConfig(json: string): Config | null {
    try {
        return JSON.parse(json);
    } catch (error) {
        debug('failed to parse config: %O', error);
        return null;
    }
}

// incorrect
function parseConfig(json: string): Config | null {
    try {
        return JSON.parse(json);
    } catch {
        return null;  // silent failure, no logging
    }
}

Security

notethink has no server-side component, no database and no user accounts, so the auth, server-action and RLS sections of SECURITY.md do not apply. Its environment-variable rules do, for the build and publish scripts: never put a secret in argv or in a committed file. Anything the extension writes to a user's settings.json is public by construction (see Naming Conventions > config keys).

Testing Standards

Test File Location

Place Jest tests next to source files:

components/
├── DocumentView.tsx
├── DocumentView.test.tsx
└── DocumentView.module.scss

Exception - VS Code extension-host Mocha suite. Tests that exercise the live VS Code API (client/extension/src/test/suite/**) run under the @vscode/test-electron Mocha runner, not Jest, and are deliberately kept in that central suite rather than colocated. The runner discovers them by directory, and they need the real extension-host environment a colocated Jest test can't provide. Colocate everything else (pure logic, webview components) next to source as above.

Test Naming

describe('DocumentView', () => {
    it('renders document container with correct id', () => { });
    it('displays loading state when data is fetching', () => { });
    it('calls onClick handler when note is clicked', () => { });
});

Test Structure

import { render, screen, fireEvent } from '@testing-library/react';
import DocumentView from './DocumentView';

describe('DocumentView', () => {
    const default_props = {
        id: 'test-doc',
        title: 'Test Document',
    };

    it('renders with required props', () => {
        render(<DocumentView {...default_props} />);
        expect(screen.getByTestId('document-test-doc-inner')).toBeInTheDocument();
    });

    it('handles click events', () => {
        const handle_click = jest.fn();
        render(<DocumentView {...default_props} onClick={handle_click} />);

        fireEvent.click(screen.getByRole('button'));
        expect(handle_click).toHaveBeenCalledTimes(1);
    });
});

E2E Test Scope - no reloads as workarounds

See workspace ../AGENTS.md > Testing conventions > No page reloads as test workarounds. (Reloads are legitimate only for an explicit refresh-resilience test; if the UI doesn't update after an action, the bug is in the UI, not the test.)

Disabling a spec indefinitely - comment it out, don't test.skip

See workspace ../AGENTS.md > Testing conventions > Disable a spec by commenting it out, not test.skip. (test.skip is for runtime-conditional skips; an indefinite disable comments out the whole file body with a leading reason/re-enable comment and a trailing export {};. Same applies to a single test(...) inside a larger test.describe.)

Working Style

Present, don't force a decision

When the user asks to "have a look", "let me see them", "just present them", or "show me the options", present the artifacts/options with a brief honest assessment and stop. Do not follow up with an AskUserQuestion (or any other prompt) that forces an immediate choice. The user wants to evaluate on their own time and will volunteer the choice. Reserve AskUserQuestion for genuinely blocking ambiguity - not "which do you prefer?" when the user has explicitly asked to look first.

No speculative specs in todo.md

When working through a story's planned phases, do not invent new feature specs, phases, or tasks beyond what was originally scoped. Phases and tasks come from the user, not extrapolated by Claude. Speculative additions clutter the backlog and waste time on work the user didn't ask for. If a follow-up genuinely seems worth doing, mention it in the wrap-up so the user can choose to add it - do not write it into todo.md unilaterally.


Pre-Push Verification

All tests must pass locally before pushing. After any session where files are updated, run:

pnpm run check

This runs, in order:

  1. Lint - eslint + tsc --noEmit across all three tsconfigs
  2. Build - webpack (extension + webview bundles)
  3. Rollup - notethink-views component library build
  4. Jest - all unit/component tests across extension, webview, and notethink-views

CI only runs lint and build (no tests). Tests are the developer's responsibility before push.

No web dev server

notethink is a VS Code extension - there is no pnpm run dev and no HTTP server to start. The webview/extension bundles are produced by webpack (pnpm run build or pnpm run watch) and previewed inside the VS Code Extension Development Host. Per the /open-dev skill's special cases, this project is exempt from the workspace dev-server start pattern (see workspace AGENTS.md, ## Dev servers).

The "no HTTP server" rule is about a long-running dev server. The Playwright test harness (playwright.config.ts auto-starts playwright/harness/serve.mjs on port 9123 for the run, then tears it down) is throwaway test infrastructure, not a dev server, and is exempt.

After every code change

Always rebuild the extension after each code change so the developer can preview it in the VS Code dev host. Run pnpm run build (or pnpm run check which includes the build) before considering a change complete.

For webview/React changes specifically, a source edit alone does NOT change what VS Code shows - the extension serves the prebuilt client/webview/dist/index.js. After editing anything under client/webview/src/:

  1. Identify the live code path first when the component has multiple branches. Example: BreadcrumbTrail.tsx has two independent code paths - a single-file splitPathSegments branch (used when the toolbar shows "Current file") and a directory-aggregate integration_path branch. Determine which one is live from the current screenshot/state before editing, otherwise you'll edit the wrong branch and the visible behaviour will not change.
  2. Run pnpm run build (webpack compiles notethink-views from src/). build-and-rollup also refreshes notethink-views/dist/esm.
  3. Confirm the edit landed in the bundle: grep client/webview/dist/index.js for a token from your edit.
  4. The user must reload the VS Code window for an already-open webview to pick up the new bundle.
  5. Never report a UI change as done from a source edit alone - verify the bundle and ask the user to reload.

Webview bundle caching (dev): webview resources are cached by URL, so a rebuilt index.js served from an unchanged URL comes back stale across reloads and the fix appears not to take effect. getHtmlForWebview appends a per-load ?v=<timestamp> cache-buster when NOTETHINK_DEV, so a dev reload always fetches the fresh bundle (production keeps the cacheable URL). If a webview change still seems not to apply, confirm the running build via the file log and that the window was actually reloaded - don't assume the bundle is fresh.

Individual commands

WhatCommand
Lint onlypnpm run lint
Build onlypnpm run build
Rollup onlypnpm -C client/webview/src/notethink-views run rollup
Jest onlypnpm run test-jest
Playwright E2Epnpm run test-playwright
Everythingpnpm run check

Release & Publishing

sh/git/merge-main.sh (staging -> main) is the production release: it fast-forwards main, pushes, then runs vsce publish to the VS Code Marketplace. The push to main also triggers CI to publish @zoombuzz/notethink to GitHub Packages and cut a GitHub Release with the .vsix. Three invariants must hold or the release breaks, and all break after the merge has already shipped to main.

@types/vscode must not exceed engines.vscode

vsce package / vsce publish reject the build when devDependencies.@types/vscode declares a higher version than engines.vscode (ERROR @types/vscode ^A greater than engines.vscode ^B). The two are a matched pair: engines.vscode is the minimum VS Code the extension supports, and you must not type-check against an API surface newer than that floor.

  • bump them in lockstep. Whenever a dependency update raises @types/vscode, in the same commit either raise engines.vscode to the same minor or pin @types/vscode back down.
  • this fails only at publish time, which in this repo is after merge-main.sh has fast-forwarded main. /prod-ready does not catch it (lint, build, jest, and playwright never invoke vsce), so a mismatch surfaces mid-deploy with main already advanced. Treat the pairing as a pre-merge checklist item on any @types/vscode bump.

A dependency-specifier change must regenerate the lockfile

CI runs pnpm install with CI=true, which pnpm treats as --frozen-lockfile: it aborts (ERR_PNPM_OUTDATED_LOCKFILE) when a package.json dependency specifier does not match the specifier recorded in the matching pnpm-lock.yaml. The root postinstall cascades a pnpm install into client/extension, client/webview, and client/webview/src/notethink-views, so all four package/lockfile pairs must stay in sync - a mismatch in any one fails every workflow at the "Install dependencies" step.

  • when you change a dependency specifier (e.g. pinning @hello-pangea/dnd from ^18.0.1 to 18.0.1), regenerate and commit the matching pnpm-lock.yaml in the SAME commit. pnpm install updates it locally; when the resolved version is unchanged the only diff is the specifier: line.
  • match CI's pnpm major - the workflows pin pnpm/action-setup to v9. regenerating with a newer local pnpm can add format-only fields (e.g. libc:) that the CI pnpm may reject; when only the specifier moved, a one-line hand-edit of the specifier: line is the minimal, version-safe fix.
  • like the @types/vscode pairing this fails only at CI install time, after merge-main.sh has shipped to main; /prod-ready runs a NON-frozen local install and never catches it. Treat a specifier change as a pre-merge checklist item.

CI skips Playwright browser downloads

release.yml and publish.yml set PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD=1. CI only lints, builds, packages, and publishes - it never runs Playwright - but @playwright/browser-chromium (pulled in transitively by @vscode/test-web) otherwise runs its install script during pnpm install, downloads ~167MB of Chrome, then hangs until the 6h job timeout cancels the run. Do not remove the env var; if a CI job ever needs browsers, run playwright install as an explicit step instead.