simplify: remove disk re-reads, fix dead param, return set from load_combined#3
Merged
jordankrueger merged 1 commit intomainfrom Apr 28, 2026
Merged
simplify: remove disk re-reads, fix dead param, return set from load_combined#3jordankrueger merged 1 commit intomainfrom
jordankrueger merged 1 commit intomainfrom
Conversation
…et from load_combined - build.py: allowlist self-test now checks in-memory sets instead of re-reading all four output files from disk (same guarantee, no 4× 66k-file parse round-trip) - import_to_actionkit.py: drop dead `base_url` param from fetch_existing (it was never used — URL was built from `instance` directly); inline the base URL at the one POST call site - import_to_actionkit.py: load_combined() now returns set[str] directly, eliminating the redundant set() wrap in main() - import_to_actionkit.py: convert stray .format() to f-string for consistency Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
AI Code Review (gpt-5.1-codex-mini)
🟡 IMPORTANT scripts/import_to_actionkit.py:74 — fetch_existing now drops the base_url parameter while the body still uses base_url to build GET calls; the name is undefined and the import script will raise a NameError before making any requests, so existing domains can’t be read or compared.
Summary: Removing base_url from fetch_existing broke the function: it now references an undefined name and will crash before syncing with ActionKit, so the import script cannot run.
Result: FAIL — blocking findings detected
1 finding(s) at or above 'important' threshold block this PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
base_urlparameter fromfetch_existing()(it was never referenced inside the function; the URL was always built frominstancedirectly). Inlined the base URL at the one POST call site.load_combined()now returnsset[str]directly, eliminating the redundantset(domains)wrap inmain()..format()call to f-string for consistency with the rest of the file.Test plan
normalize()works,load_combined()returns asetof 66,169 domains withgmail.comabsentpython3 scripts/build.py --no-fetchrun (CI / manual)🤖 Generated with Claude Code