Implement aligned trend findings and deduplicate matrix tags#218
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Risk Assessment
Risk level: Medium
Code review is required, so I am not approving this PR.
Evidence from the diff:
- Changes production backend analytics behavior in
backend/app/services/insight_engine.py,backend/app/services/insight_service.py, andbackend/app/services/stats_service.py, including tag alias canonicalization, latest-insight de-duplication, and yearly timeseries granularity. - Changes broad user-facing frontend analytics flows across insights, trends, matrix rendering, symptom heatmaps, and chart axis alignment.
- Adds custom tag creation UI inside
TagPicker.svelte, affecting an entry creation workflow and a mutation path. - Diff spans 29 files with cross-layer backend/frontend behavior changes, though there are no migrations, auth/security model changes, or infra changes.
Reviewer assignment: no reviewers were requested because the repo exposes only Sturmi77 as a collaborator/historical domain owner, and Sturmi77 is also the PR author. There is no CODEOWNERS file, so CODEOWNERS review is not required.
CI status observed: the API/web/security checks on the current head were successful; the automation check was still in progress while this assessment was posted.
Sent by Cursor Automation: Assign PR reviewers
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71b8723282
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if insight.subject_label: | ||
| return ("tag_label", insight.subject_label.casefold()) |
There was a problem hiding this comment.
When a user has two different custom tags with the same display name and legacy insight rows generated before payload.tag_slug existed, /insights/latest now keys both rows as ("tag_label", subject_label.casefold()), so the newer insight suppresses the other even though their subject_id/slug are different. Tag names are not unique, so the no-slug fallback should preserve distinct subjects rather than collapsing every same-label tag.
Useful? React with 👍 / 👎.
| insight.subject_type === 'tag' | ||
| ? tagSlug || insight.subject_label?.toLocaleLowerCase() || insight.subject_id || '' |
There was a problem hiding this comment.
If the matrix receives legacy tag insights that do not include payload.tag_slug, two different tags with the same display label are deduped before the table is rendered because the fallback uses the lowercased label before subject_id. Since users can create separate tags with identical names, this hides one valid row; prefer subject_id when no canonical slug is present, or only label-collapse cases that are known aliases.
Useful? React with 👍 / 👎.
| export const compareDailyAxisLayout: DailyAxisLayout = { | ||
| labelWidth: 160, | ||
| dayWidth: 18, | ||
| dayGap: 4, |
There was a problem hiding this comment.
With the shared layout, dayWidth: 18 is now also the rendered size of every ComparisonHeatmap button, and the previous coarse-pointer CSS that expanded cells to 2.75rem was removed. On phones/tablets the date cells become 18×18px tap targets, making the documented tap-to-open-history interaction very hard to use; the shared axis needs a touch layout that keeps both the line chart and heatmap aligned while preserving usable hit targets.
Useful? React with 👍 / 👎.
| $: axisStart = | ||
| tagHeatmap?.start_date ?? symptomHeatmap?.start_date ?? points[0]?.period_start ?? ''; | ||
| $: axisEnd = | ||
| tagHeatmap?.end_date ?? | ||
| symptomHeatmap?.end_date ?? | ||
| points[points.length - 1]?.period_end ?? |
There was a problem hiding this comment.
In time zones where the browser's local day differs from the backend's UTC as_of day (for example U.S. evening hours), the heatmap range comes from the local dateWindow() while fetchTimeseries(range) still uses the server's current day. Since the shared axis now prefers tagHeatmap dates and MetricTimeseries only plots points whose period_start is in that axis, the newest timeseries day can be dropped and the line no longer represents the API response; build both datasets from the same as_of/date range.
Useful? React with 👍 / 👎.
2 participants
Summary
Documentation
docs/FRONTEND.md: entry TagPicker create flow, Insights symptom toggle, Trends shared daily axis/layer persistence, component map.docs/frontend/SYMPTOM_VISUALIZATION.md: status changed to partially implemented, implemented subset and acceptance criteria updated.docs/features/symptom-analytics.md: implementation note for the frontend-only descriptive slice.docs/ARCHITECTURE.mdanddocs/DESIGN_DOCUMENT.md: clarify shipped UI context vs. pending inferential analytics.Testing
pnpm --dir apps/web test -- src/lib/utils/charts.test.ts src/lib/components/trends/MetricTimeseries.test.ts src/lib/components/insights/InsightFeed.test.ts src/lib/components/entries/TagPicker.test.tspnpm --dir apps/web test -- src/lib/components/insights/InsightMatrix.test.tspnpm --dir apps/web lintbackend/.venv/bin/python -m pytest tests/test_stats_service.py --no-covbackend/.venv/bin/python -m pytest tests/test_insight_engine.py tests/test_insights.py --no-covbackend/.venv/bin/python -m ruff check app/services/stats_service.py tests/test_stats_service.pybackend/.venv/bin/python -m ruff check app/services/insight_engine.py app/services/insight_service.py tests/test_insight_engine.py tests/test_insights.pypnpm --dir apps/web exec prettier --write ../../docs/FRONTEND.md ../../docs/frontend/SYMPTOM_VISUALIZATION.md ../../docs/features/symptom-analytics.md ../../docs/ARCHITECTURE.md ../../docs/DESIGN_DOCUMENT.mdandgit diff --checkNote:
backend/.venv/bin/python -m pytest tests/test_stats_service.pypasses the selected tests but exits non-zero because the repository-wide coverage gate is enforced on a single-file run.