Skip to content

engineering-guidelines.md

Latest commit

 

History

History
228 lines (156 loc) · 19.5 KB

engineering-guidelines.md

File metadata and controls

228 lines (156 loc) · 19.5 KB
Raw

Tasklog Engineering Guidelines

This document describes how the codebase is currently structured and why. It is not a rulebook - it is context. When something deviates from these patterns, that is worth a conversation, not necessarily a blocker.

Read docs/architecture.md first to understand the system structure.

Core principle

Right-sized, not over-engineered.

Add abstraction when it removes real duplication or makes something genuinely clearer. The codebase should be understandable by reading it top to bottom. When in doubt, prefer the simpler approach and evolve later.

Backend (ASP.NET Core Web API)

Current patterns

  • Controllers handle HTTP concerns: routing, input validation, response codes.
  • Controllers call DbContext directly - no repository or service layer yet.
  • Request bodies use C# record types (immutable, concise).
  • All database operations are async/await.
  • Route constraints enforce types at the URL level (e.g. {id:int}).
  • Timestamps are stored in UTC (DateTime.UtcNow).

Current patterns - additions from v2.4

  • EF Core eager loading with .Include() - used in TasksController to load Labels alongside tasks. All task queries use .Include(t => t.Labels) so callers always receive a populated labels array. Use the same pattern for any future navigation property that is always needed with the parent entity.
  • Many-to-many via implicit join table - TaskModel and Label use EF Core's HasMany/WithMany to configure a join table (LabelTaskModel) without an explicit join entity. Cascade deletes on the join table are handled automatically by the database.

Current patterns - additions from v2.10.2

  • JsonElement partial PATCH (present-key detection) - TasksController.Update (PATCH /api/tasks/{id}) binds the body to [FromBody] JsonElement and inspects each field with TryGetProperty, rather than binding to a typed record. This is the only way to distinguish "field omitted" (keep the current value) from "field set to null" (clear it). A typed record with nullable properties collapses both cases to null, so it cannot express a clear-vs-keep difference. Reach for this whenever a PATCH needs nullable-clear semantics. Unlike query-string array binding, the present-key logic is directly unit-testable: build a JsonElement from a JSON string with JsonDocument.Parse(json).RootElement.Clone() and pass it to the action, which exercises the same branching the HTTP body would.

Current patterns - additions from v2.12.0

  • Midnight = "date-only" sentinel for deadlines - Deadline is a single DateTime, but a value at exactly 00:00:00 is treated as a date-only deadline (no specific time), while any non-midnight time is a real moment. ComputeDueStatus branches on deadline.TimeOfDay != TimeSpan.Zero for the "overdue" rule (timed = past the instant; date-only = past the calendar day). This avoids a separate HasTime column. Trade-off: a literal midnight deadline behaves as date-only - acceptable since "due at exactly 00:00" is rare and "end of day" is the sensible reading. The frontend mirrors this with hasTimeComponent(iso) (checks the HH:mm substring, timezone-safe).

Current patterns - additions from v2.10.5

  • Non-zero column defaults go in OnModelCreating, not just the property initializer - TaskModel.Priority defaults to 4 (P4 = none). EF Core derives a migration column's SQL default from the CLR default of the type (0 for int), NOT from a C# property initializer (= 4). Setting only the initializer means existing rows migrate to 0 (an invalid out-of-range value here). Configure the DB default explicitly: modelBuilder.Entity<TaskModel>().Property(t => t.Priority).HasDefaultValue(4), then generate the migration - it emits defaultValue: 4 and existing rows migrate correctly. Always verify the generated migration's defaultValue for any non-zero default. (See learnings/orm-migration-default-values.md.)

Current patterns - additions from v2.10.3

  • Computed response field via [NotMapped] getter - TaskModel.DueStatus is a read-only property ([NotMapped] public string DueStatus => ComputeDueStatus(Deadline, DateTime.Today);) rather than a stored column or a DTO field. [NotMapped] (from System.ComponentModel.DataAnnotations.Schema) tells EF Core to ignore it for mapping/migrations, while System.Text.Json still serializes the getter. The effect: the field appears on every response that returns the model, with zero per-action wiring and no schema change, and it is always computed fresh (so a value relative to "now" never goes stale). Use this for derived, response-only fields that have no business being persisted. Pair it with a pure static helper that takes its time-dependency as a parameter (ComputeDueStatus(deadline, today)) so the logic is unit-testable without freezing the clock - the instance getter just wires in DateTime.Today. Keeps the project's no-DTO convention intact.

Current patterns - additions from v2.14.0

  • Pure domain helper in Services/ (not a DI service layer) - recurrence logic lives in Services/RecurrenceRule.cs: a static class that parses/validates an RRULE-shaped string and computes the next deadline (NextDeadline(current)). It is the first file under Services/, but it is deliberately NOT the "service layer" pattern below - no DI, no injected DbContext, no instance state. It is the same shape as TaskModel.ComputeDueStatus: a pure, parameterized function the controller wires in (Complete calls RecurrenceRule.TryParse(...).NextDeadline(deadline)). Because it is clock-free (it advances from the passed-in deadline, never DateTime.Now), it sidesteps the DateTime.Now/UtcNow deviation (#18) and is trivially unit-testable. Reach for this - a static helper under Services/ - when logic is non-trivial and non-HTTP but still stateless and pure; reserve a true injected service (below) for when it needs the DbContext or other dependencies.

Current patterns - additions from v2.20.0 (#78)

  • Optional-Include on a list endpoint driven by a query flag - GET /api/tasks?includeSubtasks=true conditionally .Include(t => t.Subtasks.OrderBy(...)) so the web gets the full rows for its inline checklist, while the default (MCP) path stays lean and only stitches on two grouped-count fields (subtaskCount/completedSubtaskCount). The counts are [NotMapped] settable properties the controller fills (from the loaded collection when included, from a grouped query otherwise) - the same "computed response field" idea as DueStatus, but populated by the controller rather than a pure getter because the source isn't always loaded. Reach for a flag-gated Include when one caller needs a heavy nav and another does not, rather than always loading it or splitting the endpoint.
  • One shared inline component across three layouts - SubtaskChecklist renders the same clubbed, tickable list under a task on the mobile card, the desktop table (as a full-width sub-row via a keyed Fragment + colSpan), and the board card. Keeping it one component (with a max + "+N more" prop) meant the "tick a step" gesture reads identically everywhere and there is one place to change. When the same affordance must appear in structurally different containers (a div card vs a <td>), lift it into a layout-agnostic component and let each host wrap it.
  • @dnd-kit for drag-reorder (first drag-and-drop dependency) - SubtaskSection uses @dnd-kit/core + /sortable + /utilities for touch-friendly subtask reordering (a PointerSensor with a small activation distance + a KeyboardSensor). Admitted under the same "clear-benefit dependency" rule that allowed chrono-node: reorder-on-touch is fiddly and error-prone hand-rolled, and the phone is a first-class client. Reorder is optimistic - arrayMove locally, POST the new id order, revert to the pre-drag snapshot on failure.
  • DateTime.Now (not UtcNow) followed deliberately - subtask timestamps/comparisons use DateTime.Now to match the surrounding controllers (the open #18 deviation), so a subtask's dates behave identically to its parent's rather than introducing a mixed-clock inconsistency inside one feature.

Current patterns - additions from v3.0 (#79)

  • JSON-as-TEXT columns - JournalTemplate.SectionsJson, JournalEntry.ContentJson, and MoodCheckin.WordsJson store JSON as plain TEXT, serialized with System.Text.Json (NOT EF Core's ToJson owned-entity mapping). Deliberate: the content is opaque to SQL by design, and this is the simplest understandable form for the codebase's first JSON columns. The rule that keeps it honest: anything you need to QUERY (energy, MoC level, dates) gets a real column; the JSON blob is for shape-flexible content only.
  • Paired cross-language contract - the journal's content shapes exist twice on purpose: frontend/src/lib/journal.ts (the client contract) and Services/JournalMarkdown.cs (the renderer mirror). Plan bucket keys and evening field keys are duplicated verbatim in both. When touching either side, change both together - the files reference each other in comments.
  • Startup upsert for code-defined rows - journal templates are constants in Services/JournalTemplates.cs, upserted into their table by Key in Program.cs after Database.Migrate(). Editing a definition updates the row on next run with no migration. Reach for this when data is code-owned but tables/FKs still want real rows.
  • Page-scoped theme tokens - the journal ships its own identity as --color-j-* variables beside the app tokens in globals.css (light + html.dark overrides). Components use bg-j-card / text-j-ink etc. This scopes a distinct look to one section without forking the theming mechanism; promoting it app-wide later is a token swap, not a rewrite.
  • react-markdown for display-only markdown - admitted under the clear-benefit dependency rule. Markdown is GENERATED backend-side (Services/JournalMarkdown.cs, a pure static helper - the RecurrenceRule shape); the frontend never parses or round-trips it. No Markdig: we only write markdown, never read it.

Patterns not yet in use - and when to consider them

Repository pattern - not used currently. DbContext is injected directly into controllers. Worth considering if the data access layer needs to be swapped out or tested in isolation.

Service layer - not used currently. Business logic (minimal right now) lives in the controller. Worth introducing when controller actions contain logic that is not HTTP-specific and is shared across more than one action.

AutoMapper / object mapping - not used. Models are simple enough to map by hand. Worth considering if the gap between API response shapes and database models grows significantly.

Global exception handling middleware - not used. Controllers return explicit error responses. Worth adding when error handling becomes repetitive across many endpoints.

Response codes

Situation Code
Success with data 200 OK
Created 201 Created (with CreatedAtAction)
Success, nothing to return 204 No Content
Invalid input 400 Bad Request
Not found 404 Not Found

Frontend (Next.js App Router)

Server Components vs Client Components

Default to Server Components. Add "use client" when the component needs:

  • useState or useEffect
  • Browser event handlers (onClick, onChange, onSubmit)
  • Browser-only APIs

Current component breakdown:

Component Type Reason
layout.tsx Server Static shell, no interaction
page.tsx Server Renders client component, no state
tasks/[id]/page.tsx Server Fetches task and projects, no interaction
ProjectLayout.tsx Client Owns activeView and projects state, handles project CRUD
ProjectSidebar.tsx Client Project nav with create/rename/delete interactions
TasksClient.tsx Client Owns task list state, handles mutations, filters by activeView
AddTaskForm.tsx Client Controlled inputs, form submission, project dropdown
AssignProjectButton.tsx Client Select element, PATCH on change, router.refresh()
DeleteTaskButton.tsx Client Click handler, redirect
CompleteTaskButton.tsx Client Click handler, router.refresh() (no redirect)

API calls

Fetch calls currently live in src/lib/api.ts. This keeps the API contract in one place and makes it easy to see what the frontend depends on. If a new pattern is needed (e.g. React Query, SWR), that is a good conversation to have before adding ad-hoc fetch calls elsewhere.

Component responsibilities

  • One component, one job as a starting point.
  • Components that manage data should not also own complex layout - but use judgement.
  • Error and loading states should always be handled. A blank screen is never intentional.

Styling

  • Tailwind utility classes are the current approach.
  • The default Tailwind colour scale maps to the UI spec - see UI-SPEC.md.
  • Arbitrary values (e.g. w-[137px]) are a signal to check if a scale value would work instead.
  • Responsive: mobile-first. sm: prefix for desktop variants.

Custom hooks

Reusable hooks live in src/hooks/. Current hooks:

Hook Purpose
usePolling(fetchFn, intervalMs, enabled) Background data refresh on a timer. Pauses when the tab is hidden (Page Visibility API) and when enabled is false (e.g. during in-flight operations). Fires an immediate fetch when the tab becomes visible again. Used in TasksClient, ProjectLayout, and LabelsClient.

When adding a new hook, place it in src/hooks/ and follow the same pattern: accept a callback, an interval or config, and an enabled flag for conditional execution.

Shared utilities

When the same logic appears in more than one place, extracting it to src/lib/ is worth considering. Current candidates: formatDate, deadlineColorClass (issue #5).

Additions from v2.15.0

  • First frontend runtime dependency: chrono-node (MIT) for natural-language date parsing in quick-add (lib/quickAdd.ts). The project's "avoid unnecessary frameworks" rule explicitly allows a clear-benefit dependency, and free-form date NL ("next friday", "jan 27", "in 3 days") is the textbook don't-reinvent case - chrono also returns the matched span, which the highlight overlay and title-stripping need. Recurrence and the #/@/pN tokens are still hand-rolled (bounded grammar we control), so the dependency is scoped to dates only. quickAdd.ts itself is a pure function, mirroring the format.ts precedent.
  • Backdrop-overlay highlighting over a normal <input> (QuickAddInput.tsx) - a transparent-text-aware backdrop div renders tint rectangles behind recognized token spans while the real <input> sits on top (scroll synced). This gives inline highlighting without a contenteditable rewrite (which true padded inline pills would require). "Unlink a wrong token" is offered as a removable-chips row below the field rather than in-place click, for the same reason. Reach for this overlay pattern when you need to decorate input text without giving up the native input/caret/a11y.

MCP server (Node/TypeScript, v2.10+)

The MCP server is the first Node.js service in the repository. Different patterns from the .NET backend and Next.js frontend; documented here so the precedent is explicit.

Current patterns

  • Hono with @hono/node-server as the HTTP framework. Chosen over Express for modern TypeScript-native types, smaller dep tree, equivalent capability for this scope.
  • TypeScript strict mode + NodeNext module resolution. All imports use explicit .js extensions (NodeNext requires this, even when importing from TypeScript source).
  • Hand-rolled OAuth 2.1 rather than a library. Visibility into every spec requirement was prioritized over ergonomics, given this was the project's first OAuth implementation.
  • Opaque tokens (not JWT). Random 32-byte hex strings stored in SQLite with metadata. Simpler than JWT (no key management) and trivially revocable.
  • Zod schemas inlined per tool file (not centralized in a schemas.ts). With 16 small tools across 3 files, the indirection of a separate schemas file would not pay back.
  • node:test for unit tests (not Jest or Vitest). Built into Node 20+, no extra dependency. Test files are *.test.ts next to the code they test; excluded from production build via tsconfig.
  • better-sqlite3 for the auth DB. Synchronous API matches the rest of the request-handler shape; performance is microsecond-scale for the kinds of lookups we do (single token, single client).

Patterns not yet in use - and when to consider them

Refactor store.ts to accept an injected DB path / in-memory mode - currently it opens the configured file at module load. Worth doing if we ever want to add DB-backed unit tests (currently we rely on end-to-end smoke testing for rotation, audience, expiry).

ESLint / Prettier - skipped in the initial scaffold. TypeScript strict mode catches most issues; editor-side formatting handles consistency. Worth adding if collaborators join, or if drift becomes a problem.

A library-based OAuth server (e.g. mcp-auth) - hand-rolled was correct for learning, but if requirements grow (refresh token chains, multiple upstream IdPs, scoped consent), a library may be worth migrating to.

Response codes (MCP server specific)

Situation Code
Missing or bad Bearer token on /mcp 401 + RFC 9728 WWW-Authenticate
Wrong Origin on /mcp 403
Unsupported MCP-Protocol-Version 400
Bad OAuth request (missing params, bad PKCE, etc.) 400 with RFC 6749 error code in JSON body
GET /mcp (no server-initiated push) 405 with Allow: POST

Known deviations from these patterns

These are open issues - areas where the current code does not yet match the patterns above. They are tracked rather than hidden so they can be addressed deliberately.

Issue What's not yet in place
#1 CORS not applied outside dev; server-side fetch uses localhost
#2 Feedback timer not cleared; optimistic delete before API confirms
#3 Database path is relative; API URL has no startup validation
#4 Contrast and focus indicators below WCAG AA in places
#5 Utility functions duplicated; DateTime.Now instead of UtcNow
#6 CORS policy too broad; AllowedHosts is wildcard
#17 ProjectsController CreatedAtAction points to wrong route
#18 Inconsistent DateTime.Now vs UtcNow across controllers
#19 Assigning task to non-existent project returns 500 not 400

When adding a feature

A useful checklist - not a gate:

  1. Read docs/architecture.md to understand where the change fits.
  2. Glance at docs/product-design.md - if the feature shifts the product scope, that is worth noting before building.
  3. Add an API endpoint if new data operations are needed.
  4. Add a typed function in src/lib/api.ts for any new endpoint.
  5. Build UI in the right component type (Server if read-only, Client if interactive).
  6. If logic is shared across components, extract it to src/lib/ rather than copying.