Skip to content

Commit d47c517

Browse files
abhizipstackclaude
andcommitted
test: add 256 unit tests for activity log events and ADR-001
Add comprehensive test coverage for all 20 user-facing activity log events (U001-U020) introduced in PR #66. Tests verify the UserLevel contract (audience, code, message, title, subtitle, event_status, level_tag) per event class, including edge cases like truncation and conditional branches. Also adds ADR-001 documenting the architectural decision behind the UserLevel base class approach. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent bd06a62 commit d47c517

3 files changed

Lines changed: 756 additions & 0 deletions

File tree

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
# ADR-001: User-Facing Activity Logs
2+
3+
## Status
4+
5+
Accepted — implemented in PR #66
6+
7+
## Date
8+
9+
2026-04-17
10+
11+
## Context
12+
13+
The bottom Logs panel showed raw developer-internal messages like
14+
`Executing Model Node: Database: globe_testing Database Type: postgres ...`.
15+
Users had no way to tell what happened after triggering an action. We needed
16+
plain-language activity messages ("Model mdoela built successfully in 0.42s")
17+
without breaking the existing developer log pipeline.
18+
19+
### Constraints
20+
21+
- The event pipeline (`fire_event` -> `EventManager` -> `LogHelper` -> Celery -> Socket.IO) is shared across 100+ developer events. Any change must be additive.
22+
- WebSocket rooms are keyed by session ID. We cannot add a second channel without frontend proxy changes (React dev server doesn't proxy WS by default).
23+
- Events are defined as dataclasses backed by protobuf message types. Adding new events is cheap; modifying existing ones risks breaking the proto contract.
24+
25+
## Options Considered
26+
27+
### A. Add an `audience` flag to existing events
28+
29+
Reuse existing event classes. Add `audience="user"` to a subset of them.
30+
31+
- **Pro:** No new classes. Smallest diff.
32+
- **Con:** Developer events have different message formats, verbosity, and lifecycle semantics. Retrofitting "user-friendly" messages onto events designed for `DEBUG`/`INFO` log dumps is fragile. A developer event like `MaterializationTypeTable` fires mid-execution with internal context — rewriting its `message()` to be user-friendly breaks developer debugging.
33+
34+
**Rejected** — mixes concerns, high coupling between developer and user message formats.
35+
36+
### B. Separate `UserLevel` base class (chosen)
37+
38+
New `UserLevel(BaseEvent)` dataclass that overrides `audience()` to return `"user"`. 19 new event classes (U001-U019) in 5 priority tiers. Socket payload gains an `audience` field. Frontend filters by audience in the log-level dropdown.
39+
40+
- **Pro:** Clean separation. Existing events untouched. Adding a new user event = one class + one `fire_event()` call. Rich UI metadata (`title()`, `subtitle()`, `event_status()`) lives on the event class, not in the transport layer.
41+
- **Con:** 19 new classes. More code than option A.
42+
43+
**Accepted** — separation of concerns outweighs class count.
44+
45+
### C. Separate WebSocket channel for user logs
46+
47+
Dedicated socket event (`user_activity`) instead of reusing the `logs:{session_id}` channel.
48+
49+
- **Pro:** Complete decoupling. Frontend subscribes to two events independently.
50+
- **Con:** Duplicates the LogHelper -> Celery -> socket pipeline. Frontend must manage two subscriptions. Mixed "All logs" view requires merging two streams client-side. React dev server proxy needs `ws: true` configuration for each channel.
51+
52+
**Rejected** — too much infrastructure duplication for a filtering problem.
53+
54+
## Decision
55+
56+
**Option B.** Single inheritance (`UserLevel` extends `BaseEvent`) keeps the event pipeline unified while providing a clean audience separation point.
57+
58+
### Pipeline flow
59+
60+
```
61+
fire_event(UserLevelEvent)
62+
-> EventManager.fire_event()
63+
-> Logger.write_line() # checks audience()
64+
if "user": build rich payload {title, subtitle, status, code, timestamp}
65+
if "developer": build plain payload {level, message}
66+
-> LogHelper.publish_log() # Celery queue
67+
-> logs_consumer task
68+
-> sio.emit("logs:{session_id}")
69+
-> Frontend filters by audience
70+
```
71+
72+
### Key design choices
73+
74+
1. **Audience is a class-level method, not a message field.** `UserLevel.audience()` returns `"user"` — the event *is* user-facing by type, not by configuration. This prevents accidentally marking a developer event as user-facing.
75+
76+
2. **Rich metadata on the event class.** `title()`, `subtitle()`, `event_status()` are methods on `UserLevel`, not fields on the socket payload. The transport layer reads them; the event class owns the rendering logic.
77+
78+
3. **Frontend "User activity" as default view.** The dropdown defaults to showing only `audience === "user"` entries. Developer logs are one click away ("All logs") but not the default noise.
79+
80+
4. **Events fire after success, not inside try.** Each `fire_event()` call is placed after the operation succeeds to prevent false activity entries. If the event itself fails, the existing `write_line` try/except logs it without interrupting the operation.
81+
82+
## Consequences
83+
84+
- Every new user-facing action requires a new `UserLevel` subclass + proto type + `fire_event()` call at the correct point in the operation.
85+
- The `audience` field is now part of the WebSocket payload contract — removing it would break frontend filtering.
86+
- Developer logs are completely unaffected — zero changes to existing ~100 event classes.
87+
- The 5-tier priority structure (P1 core ops, P2 scheduler, P3 CRUD, P4 connections, P5 environments) provides a framework for deciding which future actions deserve user-facing events.
88+
89+
## References
90+
91+
- PR #66: feat: user-facing activity logs with 19 curated events
92+
- PR #59: log-level filter infrastructure (prerequisite)
93+
- Event definitions: `backend/visitran/events/types.py` (lines 829-1153)
94+
- UserLevel base class: `backend/visitran/events/base_types.py` (lines 94-114)
95+
- Audience routing: `backend/visitran/events/eventmgr.py` (lines 126-151)

tests/unit/conftest.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import os
2+
import sys
3+
4+
# Ensure backend/ is on sys.path so both `visitran.*` and `backend.*` are importable
5+
backend_dir = os.path.join(os.path.dirname(__file__), "..", "..", "backend")
6+
backend_dir = os.path.abspath(backend_dir)
7+
if backend_dir not in sys.path:
8+
sys.path.insert(0, backend_dir)
9+
10+
# Configure Django before any Django-dependent import
11+
os.environ.setdefault("DJANGO_SETTINGS_MODULE", "backend.server.settings.dev")
12+
13+
import django # noqa: E402
14+
15+
django.setup()

0 commit comments

Comments
 (0)