Skip to content

Commit af0767b

Browse files
learn: retrospective learnings
Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
1 parent 9d0a18a commit af0767b

1 file changed

Lines changed: 10 additions & 0 deletions

File tree

.claude/knowledge/learning-log.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,4 +92,14 @@ until a human merges it. The engineer author phase reads this log (see
9292
- **Context:** PR #930 was titled "fix two incorrect Kernel claims" but the diff actually rewrote four rows (`_disable_pandas` ❌→✅ plus behavioral additions to `_use_arrow_native_decimals`/`_use_arrow_native_timestamps`); the reviewer verified the edits were correct but flagged that the extra changes weren't reflected in the PR title/description, and the engineer-bot could not fix it because updating the PR summary is out-of-band from file-edit tools.
9393
**Rule:** Keep the PR title/description in sync with the full scope of the diff — when a "fix N things" change grows to touch more, update the summary, since reviewers skimming the description otherwise miss changed rows and bots cannot amend the PR body from in-tree edits.
9494

95+
### 2026-08-28: learnings since 2026-08-27T20:45:13Z
96+
- **Context:** In kernel telemetry forwarding (PR #925) and metadata-filter tests (PR #933), unit tests mocked `databricks_sql_kernel.Session` with a `MagicMock(**kwargs)` that silently accepted any kwarg — so they only proved the connector *passed* a value, not that the real fixed-signature PyO3 `Session` would accept it (the 0.2.0 wheel actually raises `TypeError` on the phase-7 kwargs, breaking every `use_kernel=True` connect).
97+
**Rule:** A `**kwargs` MagicMock lies about argument acceptance — to lock kwarg-forwarding into a fixed-signature/compiled (PyO3) binding, assert with a fake mirroring the real constructor signature (or an e2e/real-wheel test), never a permissive mock that swallows unknown kwargs.
98+
- **Context:** PR #925 forwards phase-7 identity/telemetry kwargs to the kernel's PyO3 `Session`, which has a fixed signature with no `**kwargs` catch-all; the pinned `^0.2.0` floor predates those kwargs, so unconditional forwarding is a hard `TypeError` at construction. The fix introspects `inspect.signature(_kernel.Session)`, forwards only declared kwargs, and falls **closed** (omits) when introspection fails (a PyO3 class lacks `__text_signature__` unless built with `#[pyo3(signature=...)]`).
99+
**Rule:** When forwarding optional kwargs to a compiled/fixed-signature binding across versions, gate each kwarg on the installed constructor's introspected signature and fall closed on introspection failure; if the feature truly requires a newer wheel, bump the dependency floor so mismatches fail at install time, not connect time.
100+
- **Context:** In PR #933 the kernel adapter changed empty-string metadata-filter semantics (empty `LikePattern` = match-nothing, empty catalog `Identifier` = `InvalidArgument`, `%`/`*` = all-catalogs), but the new wording was placed on the backend-agnostic `DatabricksClient` base and `Cursor` docstrings. The Thrift path forwards these verbatim (`%` is a literal catalog name), so the docs misrepresented Thrift behavior and hid a kernel-vs-Thrift parity divergence.
101+
**Rule:** Filter/wildcard normalization that lives only in one backend adapter must be documented as backend-scoped, not stated unconditionally on backend-agnostic base-class or `Cursor` docstrings — and cross-backend parity divergences (kernel vs Thrift/SEA) should be called out explicitly in the CHANGELOG.
102+
- **Context:** PR #925 disabled kernel-path telemetry via `getattr(connection.session, "use_kernel", False) is True`, but backend routing selects the kernel with a plain truthiness check (`kwargs.get("use_kernel", False)`). An `is True` identity check diverges from truthiness for truthy-but-non-`True` values (e.g. `1`, `"true"`), so such a connection would route to the kernel yet keep Python-side telemetry on.
103+
**Rule:** When a second code path must mirror a routing/enable decision made elsewhere, reuse the *same* predicate form (truthiness vs identity) — an `is True` check silently diverges from a truthiness gate for non-bool truthy inputs.
104+
95105
--- *Add new entries above this line (oldest→newest); newest sections sort to the bottom.* ---

0 commit comments

Comments
 (0)