feat(ui): add rpg class assigner metric (fixes #38)#282
Conversation
git-miku
Bot
added
the
needs-triage
git-miku
Bot
added
size/S
|
Tip 👋 Hey @Diwakar-odds — Miku's on it. Here are the most useful commands for this PR:
Note 🎓 This repo is part of ECSOC26. Run Note ⭐ Star this repo to unlock all commands. Say 📖 All commands🤖 AI-powered — 14 commands
🔧 Issue & PR management — 18 commands
🎉 Community & utility — 9 commands
|
PR Analysis & Label RequestThis PR introduces the RPG Class Assigner (Issue #38) to the TUI dashboards. It calculates the user's primary command usage and dynamically assigns them an RPG developer archetype (e.g. "Docker Demolitionist"). ECSoC26 Label Justification:
Please review when you have a moment. Thank you! |
|
| Filename | Overview |
|---|---|
| termstory/insights.py | Adds a separate string-returning daily RPG class helper while keeping the existing aggregate class helper contract. |
| termstory/tui.py | Displays the daily RPG class string in the TUI and keeps commit totals based on session commits. |
| tests/test_insights.py | Adds focused coverage for the new daily RPG class assignment helper. |
Reviews (4): Last reviewed commit: "Merge remote-tracking branch 'upstream/m..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
bitflicker64
left a comment
bitflicker64
left a comment
There was a problem hiding this comment.
@/tmp/pr282-review-body.md
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Superseded by review with inline comments
bitflicker64
left a comment
bitflicker64
left a comment
There was a problem hiding this comment.
Hey @Diwakar-odds, thanks for picking up #38. The idea is fun and the helper logic is mostly sound, but I hit a real P1 while reading and CI is red on all four Python versions, so this needs another pass before merge.
What's blocking
-
Duplicate
assign_rpg_classintermstory/insights.py. Your new string-returning version lives at line 209. There's already a dict-returningassign_rpg_classat line 548 that the codebase depends on. In Python the lastdefwins, so everyfrom termstory.insights import assign_rpg_classresolves to the dict version. That means your new helper is dead code on import, and the three header call sites intui.pyare actually receiving a dict, not a string. -
That's why CI is red.
MarkupError: auto closing tag ('[/]') has nothing to closeintest_tui_landing_page_after_onboardingcomes from the newDAILY CLASSheader interpolating a dict repr ({'class_name': 'Regex Sorcerer', ...}) into a Rich markup string. The unescaped[from the dict breaks the parser. Same pattern at all three header sites. -
Wrapped view loses the commit count. In
_render_wrapped_view_uiyou replaced[bold cyan]COMMITS:[/] [dim]{total_commits}[/]with the new class line, buttotal_commitsis still computed above. Monthly wrapped summaries no longer surface commit volume.
Suggested fix shape
- Rename your new helper (e.g.
assign_daily_rpg_class) and remove the dead duplicate at line 209, or delete the old dict-returning one and migrate its two callers (tui.py:183-195andtests/test_insights.py:176-201). Renaming is the lower-risk path. - Restore the
COMMITS:row in_render_wrapped_view_uiinstead of replacing it. You have header lines to spare. - Add a test for the new helper (empty sessions, no commands, single command, top-cmd fallback to
Scripting Shaman (foo)).
Smaller notes
level = min(100, max(1, total_cmds // 50))means 50 commands = Level 1, 100 = Level 2. The issue's example is "Level 12 Regex Sorcerer", which is 600 commands. Either drop the threshold or normalize against the dominant-class share rather than raw totals, or the level will feel arbitrary.cmd.split()[0]won't match a top command if shell history stores it as a path (e.g.~/.local/bin/python). Strip the basename.class_mapdoesn't includepython3,pip,pytest,npx,pnpm,yarn,gh,make,awk,sed. The old dict-returning version covers more ground vialower_cmd.split()substring match. Worth keeping that coverage in the new map.- Trailing whitespace on a few blank lines inside the new function (PEP-8 nit, not blocking).
Happy to approve once the duplicate is resolved and CI is green.
| break | ||
| return streak | ||
|
|
||
| def assign_rpg_class(sessions: List[Session]) -> str: |
There was a problem hiding this comment.
This is the duplicate function. There's already an assign_rpg_class(sessions) -> Dict[str, Any] further down in this same file (around line 548 on the PR head). In Python the last def at module scope wins, so from termstory.insights import assign_rpg_class resolves to the dict version, not this new string version. Rename this one (e.g. assign_daily_rpg_class) and update the three call sites in termstory/tui.py to use the new name, or alternatively delete the old dict version and migrate its callers (tui.py:183-195 and tests/test_insights.py:176-201).
| header_lines.append(f"[bold cyan]{avatar_lines[7]}[/] [bold cyan]FOCUS SCORE:[/] [bold green]{fs:.1f}/10.0[/]") | ||
| header_lines.append(f"[bold cyan]{avatar_lines[8]}[/] [bold cyan]PEAK VELOCITY:[/] [dim]{peak_velocity}[/]") | ||
| header_lines.append(f"[bold cyan]{avatar_lines[9]}[/] [bold cyan]PROJECTS:[/] [dim]{active_projects_count}[/]") | ||
| header_lines.append(f"[bold cyan]{avatar_lines[9]}[/] [bold cyan]DAILY CLASS:[/] [dim]{rpg_class}[/dim][/]") |
There was a problem hiding this comment.
Root cause of the CI failure (MarkupError: auto closing tag ('[/]') has nothing to close). Because of the duplicate assign_rpg_class at line 548, rpg_class here is actually a dict, not a string. The f-string interpolates the dict repr ({'class_name': 'Regex Sorcerer', ...}) into a Rich markup string, and the [ in the repr breaks the parser. Fix the duplicate upstream and this resolves.
| header_lines.append(f"[bold cyan]{avatar_lines[7]}[/] [bold cyan]FOCUS SCORE:[/] [bold green]{fs:.1f}/10.0[/]") | ||
| header_lines.append(f"[bold cyan]{avatar_lines[8]}[/] [bold cyan]PEAK VELOCITY:[/] [dim]{peak_velocity}[/]") | ||
| header_lines.append(f"[bold cyan]{avatar_lines[9]}[/] [bold cyan]COMMITS:[/] [dim]{total_commits}[/]") | ||
| header_lines.append(f"[bold cyan]{avatar_lines[9]}[/] [bold cyan]DAILY CLASS:[/] [dim]{rpg_class}[/dim][/]") |
There was a problem hiding this comment.
Same MarkupError root cause as the timeframe header. Once the duplicate assign_rpg_class is resolved, this line is fine.
| header_lines.append(f"[bold cyan]{avatar_lines[7]}[/] [bold cyan]FOCUS SCORE:[/] [bold green]{fs:.1f}/10.0[/]") | ||
| header_lines.append(f"[bold cyan]{avatar_lines[8]}[/] [bold cyan]PEAK TIME:[/] [dim]{peak_velocity}[/]") | ||
| header_lines.append(f"[bold cyan]{avatar_lines[9]}[/] [bold cyan]PROJECTS:[/] [dim]{len(projects)}[/]") | ||
| header_lines.append(f"[bold cyan]{avatar_lines[9]}[/] [bold cyan]DAILY CLASS:[/] [dim]{rpg_class}[/dim][/]") |
There was a problem hiding this comment.
Same MarkupError root cause. Fix the duplicate upstream.
| header_lines.append(f"[bold cyan]{avatar_lines[7]}[/] [bold cyan]FOCUS SCORE:[/] [bold green]{fs:.1f}/10.0[/]") | ||
| header_lines.append(f"[bold cyan]{avatar_lines[8]}[/] [bold cyan]PEAK VELOCITY:[/] [dim]{peak_velocity}[/]") | ||
| header_lines.append(f"[bold cyan]{avatar_lines[9]}[/] [bold cyan]COMMITS:[/] [dim]{total_commits}[/]") | ||
| header_lines.append(f"[bold cyan]{avatar_lines[9]}[/] [bold cyan]DAILY CLASS:[/] [dim]{rpg_class}[/dim][/]") |
There was a problem hiding this comment.
Wrapping the COMMITS: row instead of replacing it keeps the commit-volume metric visible in monthly wrapped summaries. total_commits is still computed above, just no longer rendered.
|
Quick housekeeping note: there are 4 stray "test" review stubs above from when I was probing the API to confirm which diff paths GitHub accepts for inline comments. I couldn't delete them after the fact (GitHub only allows deleting pending reviews). Sorry for the noise. The real review with inline comments is the CHANGES_REQUESTED one directly above. — bitflicker |
This fixes issues where tests were failing due to un-awaited background threads (like get_github_avatar_ascii) and dynamic ID delays during view mounting.
git-miku
Bot
added
size/M
|
I've resolved the duplicate �ssign_rpg_class function (renamed the new one to �ssign_daily_rpg_class), restored the COMMITS line in the wrapped view, added tests for the new helper, fixed the thresholds, included the extra commands in the class map, and resolved the race conditions that were causing CI to fail. CI should be fully green now! Please review. |
|
:miku /label good-backend |
|
miku 🏷️ Added label |
|
Get this merch, and you'll get the maximum points you can from me. |
|
Can you confirm if these labels are stackable? If yes, I can stack them too. |
|
/triage |
git-miku
Bot
added
triage/triaged
|
miku ✅ Marked as triaged ( |
|
:miku /check-star |
|
miku @Diwakar-odds still hasn't starred this repo. Applied Star this repo first, then run |
git-miku
Bot
added
the
do-not-merge/needs-star
…ug/traceback; restore commits source and legacy headers; remove broken DAILY CLASS insert in Timeframe; restore daily chronicle ai/status fields
|
👋 Thanks for the PR, @Diwakar-odds! To get this PR reviewed, please ⭐ star this repo — it's free and helps others discover it. Once starred, run :miku /check-star on this PR and I'll start helping out. A |
# Conflicts: # tests/test_tui.py
|
👋 Thanks for the PR, @Diwakar-odds! To get this PR reviewed, please ⭐ star this repo — it's free and helps others discover it. Once starred, run :miku /check-star on this PR and I'll start helping out. A |
|
/ecsoc |
|
miku 🏷️ Added |
|
/lgtm /approve |
![git-miku[bot]](https://avatars.githubusercontent.com/in/4140752?s=60&v=4){kind=small}
|
miku ✅ LGTM! Label added by @bitflicker64. Use miku ✅ Approved! 🎉 |
|
/rewrite |
|
miku ✅ PR description rewritten! Check the PR body above. PreviewSummaryAdds a daily RPG class metric that analyzes command frequency to assign developer archetypes (e.g., "Level 12 Docker Demolitionist") for TUI dashboard gamification. This replaces the previous dictionary-based Changes
Architecture / Flowflowchart LR
A[Session Commands] --> B[assign_daily_rpg_class]
B --> C[Command Frequency Count]
C --> D[Class Map Lookup]
D --> E[Level Calculation]
E --> F[Formatted String Output]
F --> G[TUI Headers]
F --> H[analyze_all]
Fixes
TestingRun NotesThe new |
|
👋 Thanks for the PR, @Diwakar-odds! To get this PR reviewed, please ⭐ star this repo — it's free and helps others discover it. Once starred, run :miku /check-star on this PR and I'll start helping out. A |
bitflicker64
removed
the
do-not-merge/needs-star
2 participants
Summary
Adds a daily RPG class metric that analyzes command frequency to assign developer archetypes (e.g., "Level 12 Docker Demolitionist") for TUI dashboard gamification. This replaces the previous dictionary-based
assign_rpg_classwith a simpler string-returningassign_daily_rpg_classfunction and integrates the metric into Wrapped and Daily Chronicle view headers.Changes
termstory/insights.py):assign_daily_rpg_class()function that returns a formatted string (e.g., "Level 12 Docker Demolitionist") instead of a dictionaryanalyze_all()to call the new function and extract the class string directlytermstory/tui.py):calculate_dashboard_stats()to useassign_daily_rpg_class_render_wrapped_view_ui()render_daily_chronicle_view()refresh_details_canvas()for debuggingtests/test_insights.py):test_assign_daily_rpg_class()covering empty sessions, known commands (kubectl), and fallback for unknown commandsArchitecture / Flow
flowchart LR A[Session Commands] --> B[assign_daily_rpg_class] B --> C[Command Frequency Count] C --> D[Class Map Lookup] D --> E[Level Calculation] E --> F[Formatted String Output] F --> G[TUI Headers] F --> H[analyze_all]Fixes
Testing
Run
pytest tests/test_insights.py::test_assign_daily_rpg_classto verify the new function handles empty sessions, known commands, and fallback scenarios. To test the TUI integration, runtermstory guiand navigate to any Wrapped or Daily Chronicle view to confirm the "DAILY CLASS" metric renders correctly in the header.Notes
The new
assign_daily_rpg_class()function returns a string directly, unlike the previousassign_rpg_class()which returned a dictionary with a "class_name" key. This is an internal API change but maintains backward compatibility in theanalyze_all()return value. The function includes special handling for piped commands (defaults to "grep" for Regex Sorcerer logic) and strips basename from command paths usingos.path.basename().