Seven PRs Merged Into Six Organizations in 24 Days: What the Maintainers Taught Me
Table of Contents
- The numbers first
- 1. CERT/CC SSVC #1225: `df == None` does not mean what you think on a DataFrame
- 2. hotosm drone-tm #882: the named parameter bound the value, and `**kwargs` was empty
- 3. NIST ai-metrology-submissions #10: a Linux-only CI cannot see a Windows traceback
- 4. FINOS architecture-as-code #3009: `path.relative` returns the host separator
- 5. UK AISI inspect_k8s_sandbox #236: a Helm template printed a list without commas
- 6. casact chainladder-python #1205: I was wrong, and the maintainer checked
- 7. casact chainladder-python #1275: one character, 775 rows wrong
- Three diseases
- If you want to start
Between August 12 and September 5, 2026, 24 days, seven pull requests I opened were merged by maintainers at six organizations: the Casualty Actuarial Society (casact), Humanitarian OpenStreetMap (hotosm), the US National Institute of Standards and Technology (NIST), Carnegie Mellon's CERT/CC, the Fintech Open Source Foundation (FINOS) and the UK AI Security Institute (UK AISI), with the actuarial society taking two. Every one was merged by the other side. None was self-merged.
This is not a victory lap. I want to lay the seven side by side: which line the bug lived on, why the existing tests did not catch it, and what the reviewer corrected along the way. By the end you will see that six unrelated codebases share exactly three diseases.
The numbers first
| Org | PR | Opened | Merged | Diff | Merged by |
|---|---|---|---|---|---|
| casact | chainladder-python #1205 | 08-12 | 08-15 | +100 / -8 | henrydingliu |
| CERT/CC | SSVC #1225 | 08-13 | 08-24 | +78 / -2 | ahouseholder |
| UK AISI | inspect_k8s_sandbox #236 | 08-12 | 08-19 | +27 / -1 | art-dsit |
| FINOS | architecture-as-code #3009 | 08-20 | 08-23 | +90 / -4 | LeighFinegold |
| NIST | ai-metrology-submissions #10 | 08-23 | 08-26 | +19 / -5 | marionlb |
| hotosm | drone-tm #882 | 09-01 | 09-02 | +70 / -1 | spwoodcock |
| casact | chainladder-python #1275 | 09-03 | 09-05 | +27 / -9 | henrydingliu |
Across all seven, 441 lines changed. Fewer than 100 of them are production code (51 of those in #1205 alone). The rest is tests and configuration. That ratio is lesson one: maintainers do not merge the fix, they merge the proof that the fix is right.
1. CERT/CC SSVC #1225: `df == None` does not mean what you think on a DataFrame
SSVC is CERT/CC's vulnerability triage tool. ascii_tree(dt, df=None) raised ValueError for every non-None df, because the sentinel check used ==:
if df == None:
df = decision_table_to_longform_df(dt)
On a DataFrame, == is elementwise. It returns a same-shaped boolean frame, the if asks that frame for its truth value, and pandas raises from __bool__ by design. The default path only survived because None == None is a scalar True. Every caller in the repository and both README examples omit df, so nothing ever exercised it.
The fix is is None. But I changed one more thing that was not in the issue: the function dropped the row column from the incoming frame with inplace=True. Harmless while the parameter could not be supplied, since the frame always belonged to the function. Once the parameter works, a caller's own object gets silently mutated. I wrote "this is the part I would most like a second opinion on" into the PR, with before-and-after column lists measured on the caller's frame. Reviewer sei-vsarvepalli approved with one line: "Thanks for this patch."
Lesson: compare sentinels with is. That is not a style preference, it is the difference between working and raising in pandas. And fixing a parameter nobody could use opens every side effect behind it. Look at all of them.
2. hotosm drone-tm #882: the named parameter bound the value, and `**kwargs` was empty
HOT's drone tasking manager collects aerial imagery after disasters. The upload helper declared content_type in its signature and documented it, but the call into minio's put_object passed four positional arguments plus **kwargs. Because content_type is an explicit named parameter, Python binds the caller's value to it, leaves kwargs without it, and the value never reaches the SDK. Every aerial photo was stored as application/octet-stream. Browsers download it instead of displaying it.
metadata in the same function was fine, because it is not in the signature and genuinely travels through **kwargs. The sibling function twelve lines up, add_file_to_bucket, already forwarded content_type=content_type.
Maintainer spwoodcock asked the right question: would **kwargs not carry it? It would not. That is Python's binding rule, not this project's bug. I wrote it up on its own: A named parameter never reaches **kwargs.
The test is worth describing. I did not mock a fake put_object and assert it was called. I bound the call through inspect.signature(Minio.put_object).bind(...), the real SDK signature, so the assertion is on what the SDK would put on the wire, not what the wrapper thinks it sent.
Lesson: bind mocks to the real signature. "The function was called" and "the function received the right arguments" are different claims, and tests of the first are always green.
3. NIST ai-metrology-submissions #10: a Linux-only CI cannot see a Windows traceback
This NIST repository takes submissions of AI measurement methods, and its README asks contributors to run the validator locally before opening a PR. finish() prints the report to stdout, and every report carries an emoji status marker. On a console whose encoding is not UTF-8, which is the Windows default, encoding that marker raises UnicodeEncodeError and the traceback replaces the report. It happens on the passing path too: a contributor who did everything right sees a traceback and exit 1 instead of "Passed".
CI is ubuntu-latest only, so this is invisible upstream. I did not file "it breaks on Windows" and leave. I added a case to the test suite that runs the validator with PYTHONIOENCODING=cp1252, which reproduces the failure on any platform. Maintainer marionlb's reply, kept verbatim:
Reproducing a Windows-only failure on any platform with PYTHONIOENCODING is the part I appreciate most. It turned something we had no way to test into something we can now check on every push.
During review she pointed out that one sentence of mine about child-process encoding was backwards. My reply: you are right, and the second point I had backwards rather than merely imprecise. Fixed, rebased, merged.
Lesson: when a bug only appears on someone else's platform, your job is not to report it. It is to turn it into a test that runs on every push.
4. FINOS architecture-as-code #3009: `path.relative` returns the host separator
FINOS is the open source foundation for financial services, and architecture-as-code is its architecture documentation tooling. path.relative returns the host separator, so documents generated on Windows carry backslash paths that a POSIX reader treats as a single filename. Three sites persist a path this way: front matter, the timeline document and the bundle manifest.
Maintainer LeighFinegold replied that they had hit this before in #1357 and #1358, that CI is Linux-only so this class never shows up in builds, and asked whether I would also take timeline.ts:80. I swept and found one more: bundle.ts:184 writes a path into the manifest that resolveFilePath reads back, the same write-here-read-there shape.
Two test gotchas. Two existing bundle.spec.ts assertions already expected forward slashes and had been failing on Windows. And timeline.spec.ts computed its expected value with path.join, which pins the host separator and so stayed green on Linux-only CI. The new tests mock path to win32 so the Windows behaviour is exercised on any host. A "no backslashes" assertion would pass on Linux with or without the fix and prove nothing.
One more thing, unrelated to code. LeighFinegold mentioned that contributors had discussed reducing unnecessary AI verbosity at the last office hours and asked me to trim the commit messages and PR description. I rewrote them: both commit messages cut down, inline comments to one line each. I turned that into a standing rule: outward text cut to a third, evidence kept for myself, conclusions for the maintainer.
5. UK AISI inspect_k8s_sandbox #236: a Helm template printed a list without commas
The UK AI Security Institute's inspect evaluation framework has a Kubernetes sandbox chart. Its templates/services.yaml rendered args: {{ $service.args }}. Go's default stringification writes the list as [setarch -R /bin/echo hello]. YAML flow sequences are comma-delimited, this has no commas, so it parses back as a one-element sequence holding a single space-joined string. The container execs one argv token instead of the four the caller wrote.
It reaches users through compose/_converter.py, which maps compose's command: onto Helm's args, so any compose file using list-form command: was affected. entrypoint: was not, because it maps onto command three lines up, which already used toYaml.
The fix is toYaml. I swept the chart: every other bare interpolation is a scalar, this was the only list-valued field rendered without it. One site is the whole fix. The test asserts on the parsed list, not a substring: "setarch" in str(args) holds before and after, so a substring assertion would prove nothing. I ran the new test against the unmodified template first (fails), then with the fix (passes), in that order.
Lesson: assert on the parsed structure, not on whether a string contains a word. The latter passes for the wrong reason.
6. casact chainladder-python #1205: I was wrong, and the maintainer checked
The Casualty Actuarial Society's loss-reserving library. ParallelogramOLF crashed on two of the four origin grains: quarterly and semiannual. Two independent causes. One parsed the leap-year flag back out of an already-stringified label with a format that only knew years and months, so 2016Q1 failed to parse. The other: pandas has no to_period("S"), it reads S as seconds.
What is worth writing about is not the fix. It is that my explanation of to_period("2Q") in the PR description was wrong. I wrote that pandas drops the multiple. Maintainer henrydingliu ran it himself and showed that the multiple is honoured, a 2Q period really is six months long; the actual problem is that each period is anchored to the quarter of the observation rather than to a fixed half-year boundary, so consecutive windows overlap and two years give eight overlapping windows instead of four half years.
The first sentence of my reply: you are right and my explanation was wrong, thank you for checking it rather than taking my word. I rewrote the description with the correct account and validated the numbers with the reciprocal-mean check he had proposed himself in #524. The other maintainer, kennethshsu, formally approved, and henrydingliu, who had made the correction, merged it himself.
Lesson: when corrected, the most valuable reply is a specific admission. Not "thanks for the catch", but which sentence was wrong and what is true, written back into the PR so the next reader is not misled by me.
7. casact chainladder-python #1275: one character, 775 rows wrong
Three weeks later, same repository. CapeCod.predict() decides whether to regroup the prediction data up to the grain the model was fit at by counting the index levels sample_weight carries that apriori_ does not. The condition said "more than one", so the common case of exactly one extra level fell through to the ungrouped branch and recomputed the apriori from the prediction data instead of using the fitted one.
On the clrd sample's comauto line, the fitted apriori is 0.5689995797 and predict() returned 1.2516635774. All 775 rows differed from the value fitted for their line. The fix is > 1 to > 0. One character.
The existing test_capecod_predict2 covers this code path. Why did it miss? It uses the prism dataset, whose triangle carries five index levels more than the fitted model, so it always stays on the grouped branch. The test covered the branch, not the boundary. The new test uses clrd, which carries exactly one. The full teardown is in its own post.
This PR carried an AI disclosure under casact's policy: Claude Code reproduced the bug, ran the before-and-after comparison and drafted the regression test; the one-character fix was settled with the maintainer in the issue before any code was written. I reviewed the diff and the test myself and ran the suite locally. The maintainer merged without comment on it.
Three diseases
Stack the seven:
- Linux-only CI. NIST and FINOS both break on Windows in ways upstream can never see. The answer is not to demand a Windows job. It is to turn the platform difference into a parameter (
PYTHONIOENCODING, mockingpathtowin32) so it reproduces on Linux. - Tests that pass for the wrong reason. casact #1275's test walked the other branch, inspect_k8s would stay green on a substring assertion, FINOS computed its expected value with
path.joinand pinned the bug into the expectation. For every PR I ran the new test against the unmodified code first to see it fail, then against the fix to see it pass. - Sentinels and bindings that betray intuition. CERT/CC's
== None, hotosm's named parameter never reachingkwargs, Helm's list stringification. None of these is the project's fault. The language or the tool has a rule that disagrees with intuition, and intuition wins.
If you want to start
None of the seven is a feature. Every one is: read the issue, reproduce, find the line, prove the fix, write down what you are unsure of and ask. Merge speed is inversely proportional to PR size and proportional to how fast a maintainer can see what you proved.
Pick a library you actually use. Find an issue with reproduction steps. Make it break on your machine first. If it will not break, do not open the PR.
Every PR is listed on the Ultra Lab homepage's open-source record, each linking to its GitHub page.