diff --git a/.claude/knowledge/learning-log.md b/.claude/knowledge/learning-log.md index abadcb97b..5e2bf5e85 100644 --- a/.claude/knowledge/learning-log.md +++ b/.claude/knowledge/learning-log.md @@ -178,4 +178,16 @@ until a human merges it. The engineer author phase reads this log (see - **Context:** PR #1004 bumped OSV-Scanner from v2.3.8 to v2.7.0 by find-and-replacing the version string. That also changed comments claiming "vX doesn't support per-package CVE scoping" without checking whether the claim still holds, and the reviewer flagged it. A stale claim can leave suppressions broader than they need to be. **Rule:** When bumping a tool or dependency version, never find-and-replace the version inside capability claims ("vX does not support Y"). Re-check each claim against the new version's docs, then update the claim or the config it justifies. +### 2026-11-10: learnings since 2026-11-09T17:26:03Z +- **Context:** PR #1013 (#629) made inline params render a sequence directly after `IN` as `(1,2,3)` instead of `ARRAY(1,2,3)`. The reviewer noticed that `_renders_as_array` treats any non-`str` `Sequence` as list-able, and that includes `bytes`/`bytearray`. Before the change, `x IN %s` with bytes rendered as `IN ARRAY(97,98,99)`, which Spark rejected with a loud `PARSE_SYNTAX_ERROR`. After it, the query became valid SQL and quietly matched the wrong rows. The fix left bytes out of the IN-list path. + **Rule:** When you widen which input types take a new rendering or conversion path, list the types that used to fail loudly (bytes, str, sets, empty containers) and check that none of them now produce valid but wrong output. Exclude those types on purpose and pin each one with a unit test, because a loud error that turns into a silent wrong result is a regression. +- **Context:** In PR #1013, `test_probe_shaped_string_value_is_not_a_probe` was meant to guard the nonce check in `_markers_following_in`. It could never fail: an earlier step swaps every escaped string value for `''` during the probe pass, so the probe-shaped text never reached the code the nonce check protects. The test passed whether or not the check existed. + **Rule:** When a test is meant to guard one specific check, remove or break that check and confirm the test fails (a manual mutation check). Make sure another mechanism earlier in the pipeline doesn't neutralize the input first. If one does, feed the input in where it actually reaches the guarded code, or patch the dependency the check relies on (for example `secrets.token_hex`). + +### 2026-11-11: learnings since 2026-11-10T17:25:56Z +- **Context:** PR #1017 made `Connection.http_client` `Optional` (it is `None` for explicit `use_kernel=True`) and added `assert self.connection.http_client is not None` in the staging PUT/GET/REMOVE handlers in `client.py`. The reviewer pointed out that `python -O` strips `assert`. If a future kernel change made staging reachable, users would get an opaque `AttributeError: 'NoneType' object has no attribute 'request'` instead of a clear error. + **Rule:** Don't use `assert` to enforce a runtime invariant on an Optional attribute, even when it also narrows the type for mypy. `python -O` removes it. Use a small helper that raises a descriptive driver error (`NotSupportedError`/`ProgrammingError`, e.g. "staging operations are not supported with use_kernel=True"). That keeps mypy narrowing and still works under `-O`. +- **Context:** PR #1022 was titled as fixing kernel TLS certificate errors, but it only bumped `KERNEL_REV`. That file only sets which kernel commit `kernel-e2e.yml` builds from source for CI. Users installing the `[kernel]` extra still resolve the PyPI release allowed by `pyproject.toml` (`^1.4.0`), which predates the fix. The reviewer also noted there was no in-repo assertion (e.g. in `tests/e2e/test_kernel_tls.py`) to catch the behavior regressing on a later pin bump. + **Rule:** Bumping `KERNEL_REV` changes CI only and ships nothing to users. A kernel-side fix reaches users only after a kernel PyPI release that includes it plus a matching floor bump in `pyproject.toml`/`poetry.lock`. Don't call a pin bump a user-facing fix, and add an in-repo e2e assertion for the behavior so later pin bumps can't silently regress it. + --- *Add new entries above this line (oldest→newest); newest sections sort to the bottom.* ---