learn: retrospective learnings - #908
peco-engineer-bot[bot] wants to merge 16 commits into
Conversation
Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
peco-engineer-bot
Bot
added
the
engineer-bot-learning
Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
![peco-review-bot[bot]](https://avatars.githubusercontent.com/in/3801177?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: 1 Low
Doc-only change appending dated learning sections to the retrospective knowledge log; content is coherent and no executable code is touched. One low-severity concern: the new 2026-09-28 section duplicates the preceding 2026-09-24 entry (same PR #947, same since 2026-09-18T17:27:19Z window), adding a restated learning rather than a new one.
| **Rule:** A `logging.Filter` that mutates `record.args` must handle all three shapes it can take — `None` (message logged with no args), a tuple, and a dict — and tests must exercise each shape (especially the dict path) since an untested branch masked a NameError. | ||
|
|
||
| ### 2026-09-28: learnings since 2026-09-18T17:27:19Z | ||
| - **Context:** PR #947 fixed `RedactUrlQueryParamsFilter.filter` in `src/databricks/sql/__init__.py`: the non-dict branch assumed `record.args` was always iterable and crashed when it was `None` (log calls with no interpolation args), and a `record.arg[k]` typo in the dict branch had gone unnoticed because that path was never exercised by a test. |
There was a problem hiding this comment.
🔵 Low — This new 2026-09-28 section is a near-duplicate of the immediately-preceding 2026-09-24 section: both cover the same PR #947 fix to RedactUrlQueryParamsFilter.filter in src/databricks/sql/__init__.py, and both carry the identical since 2026-09-18T17:27:19Z window start. The retrospective window evidently did not advance (both say "since 2026-09-18T17:27:19Z"), so the same learning was re-emitted with only slightly reworded Context/Rule text.
The log's own convention comment says each daily run appends "one dated section of 0..N learnings" — a genuinely-new run that surfaced no new source PR should append 0 learnings rather than restate a prior one. As written, a future reader mining this log for durable lessons hits the same record.args-mutation rule twice back-to-back, and the duplicated since timestamp obscures which retrospective actually produced it. Consider dropping this section (or, if a new run genuinely occurred with nothing new, appending an empty-dated section) so the log doesn't accumulate restated duplicates.
Rolling retrospective learnings
This PR accumulates one dated section of learnings per day (from merged PRs and engineer-bot author runs) until it is merged; merging it starts a fresh one. The bot never writes the canonical log directly.
Latest update 2026-09-28: 1 new learning(s) since 2026-09-18T17:27:19Z.