fix(research): classify by longest matching pattern, not map order - #138
Conversation
Study category maps list both a generic and a specific form of the same host: `google.com` -> Search & Navigation appears well before `docs.google.com` -> Work & Productivity. Because classify_title matched in insertion order, every Google Workspace URL (Docs, Drive, Calendar, Meet) was filed under Search & Navigation. Reported from a Research Edition field test: the participant's Top Applications view showed Workspace time as Search. Select the longest matching pattern instead, in both the Python filter and its Swift twin used by the macOS capture path. Ties resolve to map order, so existing single-form maps behave exactly as before. Empty pattern keys are now ignored rather than matching everything. 6 regression tests added; all fail against the previous implementation.
Greptile SummaryThe PR changes research classification to select the longest matching URL or title pattern while preserving map order for ties.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Captured browser activity] --> B{URL has a configured match?}
B -->|Yes| C[Select longest URL pattern]
B -->|No| D[Select longest title pattern]
C --> E[Resolve equal lengths by map order]
D --> E
E --> F[Store configured category]
E -->|No match| G[Store excluded]
Reviews (2): Last reviewed commit: "fix(tests): use cross-platform config di..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| pattern_lower = pattern.lower() | ||
| if pattern_lower and pattern_lower in haystack and len(pattern_lower) > best_len: | ||
| best_category = category |
There was a problem hiding this comment.
If a category map contains overlapping non-ASCII patterns, Python ranks lowercased code points while Swift ranks original extended grapheme clusters and uses different case-folding semantics, producing different research categories for the same activity across platforms.
Knowledge Base Used:
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Fixed in 4906753. replaces in so Swift matches Python's code-point length semantics. For the study maps in use today (ASCII domain names) there is no behavioural difference, but the fix prevents divergence when non-ASCII patterns are added.
| transform(window, self.CATEGORY_MAP, self.APP_CATEGORY_MAP) | ||
| self.assertEqual(window, original) | ||
|
|
||
| class TestSpecificHostBeatsGenericHost(unittest.TestCase): |
There was a problem hiding this comment.
The new longest-match regression tests are not run by make test or the pull-request workflow, so CI remains green when these classification cases fail.
Knowledge Base Used: Quality automation
There was a problem hiding this comment.
Fixed in 4906753. Added poetry run python -m pytest tests/ to the Makefile test target — the same target the CI workflow calls with make test. Both the 44 pre-existing tests and the 6 new regression cases now run on every push across ubuntu, macOS, and Windows.
Two findings from review: - Swift's String.count measures grapheme clusters while Python's len() measures code points, so the two longest-match twins could pick different winners for non-ASCII patterns of equal visual length. Compare unicodeScalars.count instead. - 'make test' only ran '--help' and mypy, so tests/ has never executed in CI — not the new regression cases, and not the 44 that predate them. Add pytest to the test target.
|
Both review findings addressed in Unicode length mismatch — Swift's Tests not run in CI — correct, and the gap is wider than this PR: Local: 50/50 green, mypy clean. |
…_HOME XDG_CONFIG_HOME is Linux-only; appdirs ignores it on macOS and Windows, so the two config tests that needed a controlled config dir were failing on those platforms (StopIteration / False is True assertions). Replace the env-var approach with a monkeypatch of aw_core.dirs.get_config_dir that redirects to tmp_path on all platforms. Apply consistently across all three XDG_CONFIG_HOME test sites.
|
@greptileai review |
|
Merged — thanks Erik. Verified the final head had green Linux/macOS/Windows tests, typecheck, CodeQL, and a 5/5 Greptile review. No follow-up is needed on this PR. |
…rue (#143) The bundle's Research Edition release (v0.14.0b5-research, run 34155142876) failed on every job because release.yml runs patch -> build -> test: scripts/patch_research_edition_config.py flips research_enabled's default to true before `make test` runs, and two guards added in #138 assert the pristine (non-research) defaults: - test_research_edition_sed_target_is_intact asserts the literal `research_enabled = false` sed target is present and unindented — false by construction once the research patch has run. - test_parse_args_defaults_research_off_without_config asserts research is off by default — the research patch turns it on. Both guards are valuable on standard/PR CI (they catch accidental drift in config.py that would silently break the release patch or ship research defaults to normal users) and must stay in place there. They just shouldn't fire on the very build they exist to protect. Skip both via AW_RESEARCH_EDITION=true, which release.yml already sets at job level for research builds and which `make test` inherits. Verified locally: plain pytest still runs and passes both guards; with AW_RESEARCH_EDITION=true they skip cleanly (2 skipped, 0 failed); and with the research patch actually applied plus the env var set (the real release scenario), the full suite is green. Git-Session-Id: cb3d
2 participants
Problem
classify_titlematched study category patterns in insertion order, firstmatch wins. Study maps routinely list both a generic and a specific form of the
same host:
google.comdocs.google.comgoogle.comis a substring ofdocs.google.com, so every Google Workspace URL(Docs, Drive, Calendar, Meet) classified as Search & Navigation.
Found during a Research Edition field test — a participant's "Top Applications"
view showed Workspace time filed as Search. The macOS
swiftstrategy capturesURLs via accessibility APIs, so macOS participants hit this on every Workspace
page.
Fix
Select the longest matching pattern instead of the first, in both twins:
aw_watcher_window/research_filter.py— new_longest_match()helper, usedfor both the URL and title passes.
aw_watcher_window/macos.swift—longestResearchMatch(), same semantics, sothe macOS capture path stays consistent with the Python filter.
Behaviour notes:
are unaffected.
"" in haystackwas alwaystrue, so a stray empty key swallowed every window.
Tests
6 regression tests in
tests/test_research_filter.py, covering specific-hostprecedence on both the URL and title paths, plain
google.com/searchstillmatching the generic entry, tie resolution, and the empty-key guard.
All 6 fail against the previous implementation; the full 50-test suite passes
with the fix.
mypy aw_watcher_window/ --ignore-missing-importsclean.