Skip to content

fix(categories): don't let persisted install defaults beat a shipped preset - #974

Merged
ErikBjare merged 8 commits into
ActivityWatch:masterfrom
TimeToBuildBob:fix/preset-first-run-poison
Sep 9, 2026
Merged

fix(categories): don't let persisted install defaults beat a shipped preset#974
ErikBjare merged 8 commits into
ActivityWatch:masterfrom
TimeToBuildBob:fix/preset-first-run-poison

Conversation

@TimeToBuildBob

Copy link
Copy Markdown
Contributor

Problem

A build that ships a preset category set (Research Edition:
research-study via AW_PRESET_CATEGORY_SETS) is supposed to activate that
preset on a fresh install. settings.save() writes every key, so the
first theme/view save persisted classes=<install default> and
active_set_ids=['default'].

loadCategories() treated that as a user taxonomy, wrapped a competing
default set, and left the shipped preset inactive. On
v0.14.0b5-research the Activity view then used default and the study
taxonomy never showed unless the user deleted that set.

Tracked in ActivityWatch/activitywatch#1439.

What this does

Preset sets still activate only when the user has no stored categorization
of their own. Persisted install defaults no longer count:

  • stock defaultCategories (including after a first-run save)
  • a copy of a shipped preset, including a colorless copy of a later
    colored preset (so a palette recut does not look like user data)

Real edits are kept: custom names, edited regexes/flags/select_keys/priority,
a recolored category, a saved empty class list, an Uncategorized-rule edit.
A stored category_sets entry still wins outright.

Tests

npx jest --selectProjects node --testPathPattern=presetCategories.test.node

45 passed, including the #1439 reproduction (first-run save of stock
defaults, and of the preset class list) and the edit-preservation cases
above.

Notes

Does not close ActivityWatch/activitywatch#1439 on its own. Colors are
ActivityWatch/activitywatch#1441. Both need a research recut after the
aw-server-rust webui pin moves.

…preset

settings.save() writes every key, so a first-run theme/view save persisted
classes=defaultCategories and active_set_ids=['default']. loadCategories
treated that as a user taxonomy, wrapped a competing default set, and left
the shipped preset inactive (ActivityWatch/activitywatch#1439).

Treat stored classes that still match the stock defaults or a shipped
preset as unconfigured, and activate the first preset instead.

Git-Session-Id: d16d
Name-only matching would treat a user who edited a regex (or flags,
select_keys, priority) as unconfigured and replace their classes with
the shipped preset. Include rule fields in the signature; keep ignoring
data.color so palette updates do not look like user edits.

Git-Session-Id: d16d
Filtering that leaf out made an Uncategorized-rule edit look like an
untouched install, so a shipped preset would replace it. Compare every
category, and treat rule type null and none as the same.

Git-Session-Id: d16d
An empty `classes` array is a deliberate clear, not an install default.
Replacing it with the shipped preset would undo that save.

Git-Session-Id: d16d
A different data.color is a real edit and must not be replaced by the
shipped preset. A missing color still counts as unconfigured so a later
palette on the preset does not look like user data.

Git-Session-Id: d16d
@TimeToBuildBob

TimeToBuildBob commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

🤖 AI code review

Safe to merge — no P0/P1 findings

Confidence 5/5

No thread-worthy findings. Advisory notes follow; they are retained without opening review threads.

1 advisory finding (summary-only, not scored)

These P2 guard, heuristic, trade-off, or documentation claims are retained for judgment without opening review threads.

⚠️ P2 mediumsrc/util/classes.ts:346

matchesInstallDefault treats a stored category with a missing/empty data.color as matching the reference even when the reference has an explicit color. This means if a user deliberately clears a color that the preset defined, that customization is indistinguishable from an install default. After reload, classesLookUnconfigured returns true, so loadCategories activates the preset set and discards the user's saved classes, restoring the preset's color. The user's intent to remove the color is silently overwritten. This is a real but edge-case loss of a user-visible setting.

if (new Set(stored.map(categoryNameKey)).size !== reference.length) return false;

How this was verified: Traced the logic: for a stored category with null color and a reference (preset) with a color, the condition evaluates false, so the comparison continues. Reached by reading matchesInstallDefault (lines 337-348) and confirming categoryColor returns null for empty/undefined. Test a colorless copy of a colored preset is still an install default (lines 395-410) confirms this path is exercised.

Consensus: 3/3 passes agreed
Distinct keys: 1 (general)

Reviewed d5b2de6d4555 · openrouter/deepseek/deepseek-v4-flash-0731 · llm (in-band) engine · 208s · about this reviewer

Maintainer commands

@TimeToBuildBob review (own line) — fresh review · @TimeToBuildBob fix — a worker acts on the findings. Once per comment; 👀 = received.

@greptile-apps

greptile-apps Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

The PR appears safe to merge; the previously outstanding legacy-score issue is fully addressed without introducing a new actionable defect.

Summary

  • Compares legacy classes with stock and shipped defaults while preserving detectable user edits.
  • Gives explicitly stored category sets precedence over presets.
  • Adds regression coverage for defaults, legacy scores, edited taxonomies, colors, duplicate names, and empty category lists.

Comment thread src/util/classes.ts
Comment thread src/util/classes.ts
@codecov

codecov Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 52.82%. Comparing base (3e7fb7c) to head (58426fe).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #974      +/-   ##
==========================================
+ Coverage   52.25%   52.82%   +0.56%     
==========================================
  Files          48       48              
  Lines        2943     2978      +35     
  Branches      692      712      +20     
==========================================
+ Hits         1538     1573      +35     
+ Misses       1385     1322      -63     
- Partials       20       83      +63     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

CI-green and mergeable — waiting only on a maintainer click.

This PR is ready to merge, but the bot has pull-only access to this repo and can't self-merge — surfacing it here so it isn't lost. The monitoring loop will stop re-flagging it now that this note is posted.

…nstallDefault

Greptile P1 fixes:
- Duplicate stored category names could bypass the one-to-one name match, letting
  a taxonomy with a deleted+duplicated entry be treated as an install default and
  be replaced by a shipped preset. Add a uniqueness check for stored names
  mirroring the existing check on the reference side.
- A score-only edit (data.score) was ignored in the install-default comparison,
  so a user's scored taxonomy was silently displaced by the preset. Treat a
  non-null stored score that differs from the reference as a user customization.

Two regression tests added:
- 'a score-only edit is kept as a custom taxonomy (greptile P1)'
- 'a taxonomy with duplicate stored names is kept as a custom taxonomy (greptile P1)'

All 47 tests pass.
@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

@greptileai review

Comment thread src/util/classes.ts Outdated
… P1)

A user selecting 'Inherit parent score' stores undefined for the score
field while the install default (e.g. Work) has score: 10. The previous
check  skipped null/
undefined stored scores, so the taxonomy was misclassified as unconfigured
and could be displaced by the shipped preset.

Fix: compare scores with  normalization so any divergence —
including a cleared score — is treated as a user edit.

Add test: 'a cleared score (inherit parent) is kept as a custom taxonomy'
(48 tests pass).
@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

@greptileai review

@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

Addressed Greptile P1 finding 3 (cleared-score edge case) in the latest push:

Bug: if (storedScore != null && storedScore !== refScore) skipped undefined stored scores. A user selecting "Inherit parent score" stores undefined while the install default (Work) has score: 10 — the taxonomy was misclassified as unconfigured and could be displaced by the shipped preset.

Fix: normalized both sides with ?? null so a cleared score is detected as a user edit:

const storedScore = cat.data?.score ?? null;
const refScore   = ref.data?.score ?? null;
if (storedScore !== refScore) return false;

Added test: "a cleared score (inherit parent) is kept as a custom taxonomy" — 48 tests pass.

Greptile re-review triggered. Findings 1 & 2 from the earlier review (score-only change and duplicate stored names) were already addressed in the prior commit; those tests were in the previous 47-test run.

Comment thread src/util/classes.ts Outdated
… edit

A taxonomy persisted before category scores were introduced has no stored
score field. The previous ?? null normalization compared null (absent) against
the reference score (10), treating the legacy entry as a user customization
and blocking preset activation — the opposite of the intended behavior.

A missing stored score is indistinguishable from a 'Inherit parent score'
edit at the storage level, so we accept the trade-off: only an explicitly
set stored score that differs from the reference counts as a user edit.
This correctly handles legacy upgrade paths while still detecting explicit
score changes (e.g. score: 0.9).

- Updated matchesInstallDefault comment to document the trade-off
- Changed 'cleared score = custom' test to reflect new semantics
- Added regression test for the legacy-no-score case (Greptile P1 thread
  3969768942)
@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

@greptileai review

@ErikBjare
ErikBjare merged commit 2618faa into ActivityWatch:master Sep 9, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Research Edition: category preset conflicts with default set (no selection UI, missing colors)

2 participants