forked from eva/focus-flow
docs: add logging handoff rules
This commit is contained in:
parent
4183cb95ee
commit
1d0e2b01d7
1 changed files with 355 additions and 0 deletions
355
LOGGING_RULES.md
Normal file
355
LOGGING_RULES.md
Normal file
|
|
@ -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.
|
||||||
Loading…
Reference in a new issue