Fix children_count_worker crash on empty DataFrame groupby - #167
Merged
Conversation
A production job run with 'Child Tags': [] crashed with
KeyError: 'parentId': an empty tag set matches no annotations, so the
filtered connection list was empty and pd.DataFrame([]) — which has no
columns — raised inside groupby('parentId').
- sendError with a clear message when no child tag is selected, instead
of crashing (an empty required tag selection is a misconfiguration)
- Skip the pandas groupby when zero connections match: upload counts of
0 for every parent and sendWarning explaining why (zero children is
valid data, e.g. before a connection tool has been run)
- sendWarning when the parent or child tags match no annotations
- Defensive .get() for 'Child Tags Exclusive'
The old tests mocked pandas.DataFrame entirely, so this path was never
exercised; the rewritten empty-tags test and the three new tests run the
real code (test_empty_tag_filters had asserted a count-all-children
behavior that never existed). All four new tests reproduce the crash on
the previous code and pass on the fix.
Also documents the failure mode in CHILDREN_COUNT.md and adds it to the
nimbus-worker-hardening catalog (sweep found no other worker feeding a
possibly-empty list into groupby; connect_* workers pre-declare columns).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Mbm8CVwCNx2kdcPLadmsLA
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3393ce352
ℹ️ 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".
The new failure-mode entry was added only to the Claude copy of the nimbus-worker-hardening skill; Codex loads .agents/skills/, which still ended at catalog entry 6. The new section has no Claude-specific references, so it mirrors verbatim. Addresses PR #167 review comment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mbm8CVwCNx2kdcPLadmsLA
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
Fixes a production crash in
children_count_workerwhere an empty'Child Tags'selection causedpd.DataFrame([]).groupby('parentId')to raiseKeyError: 'parentId'. The worker now validates required inputs early and handles zero-match scenarios gracefully with appropriate error/warning messages.Changes
entrypoint.py:
child_tagswith a clearsendErrormessage asking the user to select at least one child tagworkerInterface['Child Tags Exclusive']→.get('Child Tags Exclusive', 'No')groupbyentirely whenfiltered_connectionsis empty (legitimate outcome: no connections exist)sendWarninginstead of crashingtest_children_count.py:
test_empty_tag_filterstest (which mocked pandas and missed the crash) with three focused regression tests:test_empty_child_tags_sends_error: Verifies empty tag list triggerssendErrorand prevents uploadtest_missing_child_tags_field_sends_error: Verifies missing'Child Tags'key also triggers errortest_no_connections_uploads_zero_counts: Real pandas path with zero connections → warns and uploads zerostest_no_matching_child_annotations_warns_and_uploads_zeros: Valid tags but zero matches → warns and uploads zerosCHILDREN_COUNT.md:
Implementation Details
The fix follows the pattern from the nimbus-worker-hardening skill (catalog #6 and #7):
pd.DataFrame([])has no columns, sogroupbyfails; check for empty input before calling itsendErrorfor missing required input,sendWarningfor legitimate zero resultsThe old tests mocked
pandas.DataFrameentirely, which is why the crash survived code review. New tests use real pandas with empty inputs to catch this class of bug.https://claude.ai/code/session_01Mbm8CVwCNx2kdcPLadmsLA