From 1d0e2b01d71f0e934fc53ac6f1abab127b32cb94 Mon Sep 17 00:00:00 2001 From: Ashley Venn Date: Thu, 2 Jul 2026 19:16:34 -0700 Subject: [PATCH] docs: add logging handoff rules --- LOGGING_RULES.md | 355 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 355 insertions(+) create mode 100644 LOGGING_RULES.md diff --git a/LOGGING_RULES.md b/LOGGING_RULES.md new file mode 100644 index 0000000..37ce57e --- /dev/null +++ b/LOGGING_RULES.md @@ -0,0 +1,355 @@ +# FocusFlow Logging Rules + +These instructions are for adding logging calls to the FocusFlow codebase. +The goal is to add useful diagnostics without moving logging policy into +individual services, controllers, repositories, or widgets. + +## Hard Requirements + +1. Use the shared logger only. + Do not create service-specific logger classes, log enums, log entries, helper + sinks, caller-capture code, stack-frame parsing, or message-formatting code in + feature files. + +2. Call logging directly from feature code. + Correct examples: + + ```dart + logger.debug(() => 'Applying flexible quick action. taskId=${task.id}'); + logger.warn(() => 'Duplicate operation ignored. operationId=$operationId'); + logger.error(() => 'SQLite bootstrap failed. code=${failure.code.name}'); + ``` + + Incorrect examples: + + ```dart + if (logger.wouldWrite(FocusFlowLogLevel.debug)) { + logger.debug('Applying action. taskId=${task.id}'); + } + ``` + + ```dart + final message = 'Expensive state dump. tasks=${tasks.map(...).join(',')}'; + logger.finest(message); + ``` + +3. Lazy messages are required for anything with interpolation, collection + formatting, object dumps, date formatting, joins, maps, or stack-related + values. Pass a closure: `logger.debug(() => '...')`. The logger will not + evaluate the closure unless that level is enabled. + +4. Do not manually capture caller file, method, line, or stack traces for normal + log calls. The logging layer automatically captures caller information when + the configured level requires it: + - `finest` configuration captures caller information for every written log. + - `fine`, `finer`, or `finest` configuration captures caller information for + `warn` and `error` logs. + - `debug`, `info`, `warn`, or `error` configuration does not capture caller + information for normal lower-detail logs. + +5. If a caught exception already provides an error or stack trace, pass it to + the logger. Do not create `StackTrace.current` just to satisfy logging. + + ```dart + } on Object catch (error, stackTrace) { + logger.error( + () => 'Startup failed.', + error: error, + stackTrace: stackTrace, + ); + } + ``` + +6. Do not change runtime behavior to add logging. Logging must not affect return + values, operation IDs, scheduling results, persistence results, widget state, + retry behavior, or exception behavior. + +7. Preserve existing SPDX headers, library docs, Dartdoc, imports, and part-file + structure. New Dart files need SPDX metadata and Dartdoc, but this pass should + not need new Dart files. + +8. Use ASCII only unless the edited file already requires non-ASCII. + +## Import Rules + +Use the shared scheduler logger from: + +```dart +import 'package:scheduler_core/scheduler_core.dart' show logger; +``` + +If a file already has a local `logger` name or a conflict, use an alias: + +```dart +import 'package:scheduler_core/scheduler_core.dart' as scheduler_core; + +scheduler_core.logger.info(() => '...'); +``` + +Inside `packages/scheduler_core/lib/src/...`, prefer existing local library +patterns. Some scheduler core files are `part of` files. A `part of` file cannot +add imports. For those files, add the import to the parent library file instead. +Example: `flexible_task_action_service.dart` is a part of +`task_actions.dart`, so `task_actions.dart` imports the logging layer and the +part file can call `logger.debug(...)`. + +Do not import the Flutter app file logger into scheduler core or package code. +The app file logger is only the configured sink. Feature code should use the +shared `logger`. + +## Message Shape + +Prefer concise, structured text: + +```dart +logger.debug(() => 'Scheduling backlog item. taskId=$taskId ' + 'durationMinutes=$durationMinutes windowStart=${window.start.toIso8601String()}'); +``` + +Use: +- A short action phrase first. +- Stable key names in `key=value` form. +- IDs, enum names, dates, counts, status, outcome codes, and operation IDs. +- `toIso8601String()` for dates when logging dates. + +Avoid: +- Full user-authored task titles, descriptions, notes, or free-form content at + `info`, `debug`, `fine`, `warn`, or `error`. +- Large object dumps outside `finest`. +- Vague messages like `Something went wrong`. +- Messages that blame the user. +- Duplicate logs for the same event at multiple levels. + +## Level Rules + +### `finest` + +Use for extremely granular diagnostics only. + +Good uses: +- Full input or output dumps after level gating. +- Raw decoded payloads or document maps when debugging mapping/migration. +- Complete task lists, scheduling inputs, scheduling results, or repository + state snapshots. +- Per-candidate loop details only when needed to reconstruct a scheduling + decision. + +Do not use for: +- Normal app flow. +- High-level operation starts or finishes. +- Any message that would be useful in normal debugging at `debug`. + +Notes: +- `finest` configuration automatically prepends caller information for every + written log. +- Always use a lazy closure. + +### `finer` + +Use for intra-method minor actions and small state changes. + +Good uses: +- Branches selected inside a larger operation. +- Resolved operation IDs, generated IDs, or normalized inputs. +- A candidate accepted/rejected inside a scheduling method. +- A repository deciding which query path to use. +- A controller clearing or preserving local selection as a secondary state + change. + +Do not use for: +- Method entry/exit in every method. +- Normal high-level actions that belong at `debug`. +- Large data dumps that belong at `finest`. + +### `fine` + +Use for secondary detail and "possible issue" logging. + +Good uses: +- Extra detail that explains a `warn` or `error` path. +- Detailed result summaries, notice lists, change lists, or stack traces when + already caught. +- A handled unexpected or non-typical value that could indicate a real issue, + but the code safely handled it. + +Possible issue rule: +- A "possible issue" is an unexpected or non-typical value that was handled but + may indicate bad upstream state or a missed assumption. +- Only use this when there is a real reason to suspect an issue. +- Start the message with `Possible issue:` when using this concept. + +Good possible issue example: + +```dart +logger.fine(() => 'Possible issue: completion transition did not apply. ' + 'taskId=${task.id} outcome=${transition.outcomeCode.name}'); +``` + +Do not use `fine` for: +- Normal control flow. +- Expected empty states. +- User choices such as canceling, closing a modal, or choosing no date. +- Anything that should clearly be a handled issue at `warn`. + +### `debug` + +Use for high-level debug flow and useful state values. + +Good uses: +- A command/action starts with important input IDs and state. +- A scheduling operation finishes with outcome, change count, notice count. +- A task is pushed and the earliest candidate time or destination used. +- A controller submits an action and refreshes data. +- A repository save/load operation begins or finishes with IDs and counts. + +Do not use for: +- Healthy app lifecycle milestones that belong at `info`. +- Handled failures that belong at `warn`. +- Extremely detailed dumps that belong at `finest`. +- Per-frame or every-build Flutter widget logs. + +### `info` + +Use for very high-level healthy lifecycle checkpoints. + +Good uses: +- App startup started/completed. +- Persistent runtime opened/closed. +- SQLite connected or persistence backend opened. +- Backup/export started/completed. +- A major recovery routine completed with a compact summary. + +Do not use for: +- Per-task operations. +- UI taps. +- Scheduler internal decisions. +- Warnings, handled issues, or noisy repeated operations. + +### `warn` + +Use when something did go wrong, but the app handled it and kept going. + +Good uses: +- Duplicate operation ignored. +- Task not found and a typed no-op/not-found result is returned. +- Invalid task state handled with a typed result. +- Config file unreadable and defaults are used. +- Recovery performed a fallback and continued. +- Repository compare-and-set conflict handled without corrupting state. + +Do not use for: +- Expected user choices. +- Empty query results. +- Normal no-op behavior that is part of the happy path. +- "Possible issues" that are only unusual and safely handled; those are `fine`. + +### `error` + +Use when state may be corrupted, startup cannot continue, persistence may be in a +bad state, data may require review, or an invariant is being violated. + +Good uses: +- Throwing because a service received an impossible or invalid task type. +- Startup/bootstrap failure. +- Repository commit failure. +- Migration or mapping failure that prevents safe loading. +- Backup encryption/decryption failure. +- Unhandled command failure requiring review. + +Do not use for: +- Handled typed no-op results. +- Validation failures that are expected user input paths. +- Missing optional configuration. +- Anything recovered cleanly without review; use `warn`. + +## Where Logging Belongs + +Add logs to imperative boundaries and domain operations: +- `main.dart` and app composition/open/close paths. +- Controllers that submit commands, refresh data, or handle failures. +- Application use cases. +- Scheduling services and engines. +- Repository and persistence adapters. +- Backup/export/import operations. +- Recovery, migration, mapping, and command orchestration. + +Avoid logs in: +- Pure data classes, enums, constants, token files, and simple value objects. +- `copyWith`, equality, validation constructors, and simple formatting helpers + unless they throw or handle a real issue. +- Flutter `build` methods, painters, layout helpers, and frequently called + visual functions unless there is a rare handled error. Build methods can run + often; logging there can make logs unusable. + +For loops: +- Prefer one `debug` summary before/after the loop. +- Use `finer` for important branch decisions inside the loop only when useful. +- Use `finest` for per-item dumps only when the details are essential. + +For caught exceptions: +- `warn` if recovered and state is safe. +- `error` if state may be bad, operation failed, or review is needed. +- Pass `error:` and `stackTrace:` if already available. + +## Performance Rules + +The logger already gates work by level. Call sites should rely on that. + +Good: + +```dart +logger.finest(() => 'Scheduling input dump. input=$input tasks=${input.tasks}'); +``` + +Bad: + +```dart +final dump = 'Scheduling input dump. input=$input tasks=${input.tasks}'; +logger.finest(dump); +``` + +Do not add `logger.wouldWrite(...)` checks around normal calls. If message +creation is expensive, put that expensive work inside the closure. Only consider +`wouldWrite` for rare cases where a large temporary structure must be built +before a logger call and cannot reasonably be built inside the closure. + +## Validation Rules + +After adding logs: + +1. Run formatter on edited Dart files. +2. Run package analysis and tests for touched packages. +3. At minimum for scheduler core changes: + + ```bash + cd packages/scheduler_core + dart analyze + dart test + ``` + +4. At minimum for Flutter app changes: + + ```bash + cd apps/focus_flow_flutter + dart analyze + flutter test + ``` + +5. Do not introduce new analyzer warnings. Existing unrelated info-level lints + may remain, but do not add new ones. +6. Logging changes must not change functional test expectations except tests + that directly assert logging behavior. + +## Zip Replacement Instructions + +This zip is intended to be edited externally and then extracted over the repo +root. It contains this rules file at the top level and source files under their +repo-relative paths, such as `apps/...` and `packages/...`. + +When returning edited files: +- Keep the same paths. +- Do not rename files. +- Do not remove files from the archive unless they are intentionally deleted in + the repository. +- Do not add generated files, build output, `.dart_tool`, or dependency caches. +- Preserve the shared logging layer design.