Skip to content

Commit fddcf13

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

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
@@ -58,4 +58,14 @@ until a human merges it. The engineer author phase reads this log (see
5858
- **Context:** PR #919 moved Azure endpoint/scope/app-id resolution out of the connector bridge and into the kernel, but left behind a `hostname` parameter on `kernel_auth_kwargs` (threaded from `client.py` and every new test), an unused `get_effective_azure_login_app_id` import, and a module docstring still describing the old bridge-computes-the-bundle design — reviewers flagged all three repeatedly as dead/misleading surface.
5959
**Rule:** When you delegate a computation to another layer, delete the now-inert plumbing it leaves behind (parameters, call-site args, imports) and update docstrings/PR descriptions that still describe the pre-delegation behavior — stale dead surface misleads future readers into assuming it still affects resolution.
6060

61+
### 2026-08-25: learnings since 2026-08-24T17:35:31Z
62+
- **Context:** PR #923 fixed lost SEA `ExecuteStatementAsync` telemetry in the kernel backend, where `get_query_state`/`get_execution_result` had always re-attached to the async statement by id.
63+
**Rule:** On the kernel backend, `attach_async_statement` (attach-by-id) returns a fresh handle wired to no-op telemetry — the real `ExecuteStatementAsync` telemetry row is only finalized on the original submitting ("owning") handle. Use the retained owning handle for the first in-process status poll and result stream; fall back to attach-by-id only for re-fetch, cross-process, or restarted-process resume.
64+
- **Context:** PR #923 replaced per-call attach-by-id with a per-connection retained owning handle shared across cursors, and reviewers flagged that `handle.status()`/`handle.await_result()` run outside `_async_handles_lock`.
65+
**Rule:** Refactoring from a fresh-handle-per-call pattern to a shared/retained handle silently introduces concurrent-method-call races the old path avoided (the bookkeeping lock guards only the dict, not the handle methods that run outside it). Reserve the shared handle with an in-flight set and route concurrent callers back to the fresh attach-by-id path, or document that concurrent in-process polling of one id is unsupported.
66+
- **Context:** Run #32772322538 diagnosed a kernel logging bug where raising the log level and opening a new session delivered no records.
67+
**Rule:** The kernel Rust→Python log bridge (pyo3-log) lazily caches each logger target's *effective* level on first use and never re-derives it; installing it once at module import freezes an "off" level for the whole process if `databricks.sql.kernel` logging was disabled at import time. Re-invalidate/reset the pyo3-log cache (e.g. on session open) so later level changes take effect — a manual `reset_logging()` workaround already existed in tests.
68+
- **Context:** In run #32772322538 the author phase called `StructuredOutput` five times in a row (turns 42-50), each resubmission trimming the `root_cause` field (+3252 → +1749 chars).
69+
**Rule:** Structured-output fields have length limits that reject over-long submissions and force silent retries; front-load a concise root_cause/summary the first time rather than emitting a maximal draft and shrinking it across repeated failed `StructuredOutput` calls.
70+
6171
--- *Add new entries above this line (oldest→newest); newest sections sort to the bottom.* ---

0 commit comments

Comments
 (0)