Skip to content

fix: guard all .get(key, 0) patterns that can return None (7 files, 17 occurrences)#822

Merged
mvanhorn merged 2 commits into
mvanhorn:mainfrom
23241a6749:fix/dict-get-none-safeguard-all
Jul 15, 2026
Merged

fix: guard all .get(key, 0) patterns that can return None (7 files, 17 occurrences)#822
mvanhorn merged 2 commits into
mvanhorn:mainfrom
23241a6749:fix/dict-get-none-safeguard-all

Conversation

@23241a6749

Copy link
Copy Markdown
Contributor

Problem

dict.get(key, default) only substitutes default when the key is absent. A key that exists with value None passes straight through, so .get("views", 0) returns None and operations crash with TypeError.

Fix

Replace all 17 occurrences of dict.get(key, 0) with dict.get(key) or 0 where the dict value can be None — same root cause and fix pattern as PR #796.

Files changed (8 files, +17/−17)

File Occurrences Context
lib/github.py 4 item.get("comments", 0) in reactions + comments arithmetic
briefing.py 5 f.get("engagement_score", 0) in max() / sum()
lib/youtube_yt.py 1 .get("views", 0) in list.sort(key=...)
lib/instagram.py 1 Same sort-key pattern
lib/tiktok.py 1 Same sort-key pattern
lib/hackernews.py 1 .get("points", 0) in list.sort(key=...)
lib/signals.py 2 .get("views", 0) in engagement floor comparison
store.py 2 store_sightings boundary: engagement_score and relevance_score

Verification

All tests pass: store, signals, briefing, youtube, tiktok, hackernews, instagram, cli, internals, render.

@greptile-apps

greptile-apps Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR makes nullable engagement and score fields safer to read. The main changes are:

  • Defaults explicit None values to zero in score and engagement calculations.
  • Updates sorting and threshold checks across source adapters.
  • Stores sighting scores without collapsing every falsy value to zero.
  • Encodes URL path, query, and fragment components without quoting the hostname.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
skills/last30days/scripts/lib/http.py Adds component-level URL quoting for request URLs.
skills/last30days/scripts/store.py Defaults only missing score values to zero at the sighting boundary.
skills/last30days/scripts/briefing.py Handles missing engagement scores in daily and weekly aggregates.
skills/last30days/scripts/lib/github.py Handles missing GitHub comment counts in parsed issue data.
skills/last30days/scripts/lib/signals.py Handles missing YouTube view counts in relevance and floor checks.
skills/last30days/scripts/lib/hackernews.py Handles missing Hacker News point counts during enrichment ordering.
skills/last30days/scripts/lib/instagram.py Handles missing Instagram view counts during result ordering.
skills/last30days/scripts/lib/tiktok.py Handles missing TikTok view counts during result ordering.
skills/last30days/scripts/lib/youtube_yt.py Handles missing YouTube view counts during result ordering.

Reviews (2): Last reviewed commit: "fix: guard all .get(key, 0) patterns tha..." | Re-trigger Greptile

Comment thread skills/last30days/scripts/lib/http.py Outdated
Comment thread skills/last30days/scripts/store.py Outdated
… TypeError on arithmetic/sort/sum/max

dict.get(key, default) only substitutes the default when the key is
absent. A key that exists with value None passes through, producing
None instead of the intended default. This causes TypeError in:
- arithmetic (reactions + comments)
- list.sort() with key function
- max() comparisons
- sum() over generator expressions

Fixes 17 occurrences across 8 files:
- github.py: item.get("comments", 0) used in reactions + comments
- briefing.py: f.get("engagement_score", 0) used in max()/sum()
- youtube_yt.py: .get("views", 0) inside sort key
- instagram.py: same sort-key pattern
- tiktok.py: same sort-key pattern
- hackernews.py: .get("points", 0) in sort key
- signals.py: .get("views", 0) in engagement floor check
- store.py: .get("engagement_score", 0) / .get("relevance_score", 0)
  in store_sightings (same root cause as PR mvanhorn#796)
@23241a6749 23241a6749 force-pushed the fix/dict-get-none-safeguard-all branch from 35a3a1b to 055f475 Compare July 14, 2026 14:20
@mvanhorn mvanhorn merged commit cb44afe into mvanhorn:main Jul 15, 2026
10 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.

2 participants