[peco-engineer-bot]

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-11-05: 5 new learning(s) since 2026-11-04T17:25:08Z.

[peco-review-bot]

✅ No issues identified by the review bot.

[peco-review-bot]

✅ No issues identified by the review bot.

[peco-review-bot]

✅ No issues identified by the review bot.

[peco-review-bot]

✅ No issues identified by the review bot.

[github-actions]

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

[peco-review-bot]

✅ No issues identified by the review bot.

[github-actions]

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

[peco-review-bot]

✅ No issues identified by the review bot.

[github-actions]

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

[peco-review-bot]

✅ No issues identified by the review bot.

[github-actions]

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

[peco-review-bot]

✅ No issues identified by the review bot.

[github-actions]

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

[peco-review-bot]

✅ No issues identified by the review bot.

[github-actions]

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

[peco-review-bot]

Verdict: 1 Low

Documentation-only PR: it appends dated retrospective learning sections to .claude/knowledge/learning-log.md, with no executable code to review. One low-severity doc-consistency issue — the appended ## Entries section leaves the stale "No learnings have been recorded yet" preamble in place, now contradicted by the content below it (also missing a blank line before the heading). Otherwise looks good.

[peco-review-bot]

🔵 Low — This PR appends the first ## Entries section directly beneath the preamble paragraph that still reads "No learnings have been recorded yet — the first retrospective PR will append the initial dated section below." Since this is that first retrospective PR and it now adds many dated sections, the sentence immediately above ## Entries is false the moment this merges — a reader sees "no learnings recorded yet" followed by 20+ learning sections.

When the retrospective flow writes the initial section, it should also replace (or drop) that placeholder sentence so the preamble no longer contradicts the content below it.

Minor secondary nit on the same line: ## Entries is appended with no blank line separating it from the preceding paragraph. CommonMark lets an ATX heading interrupt a paragraph so it still renders, but a blank line before the heading is the house style elsewhere in this file and avoids renderer-specific surprises.

[github-actions]

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

[peco-review-bot]

Verdict: 1 Low

Looks good. This PR only adds learning-log entries, plus one low-severity fix: the file's preamble is out of date. I checked the new 2026-10-31 KEQ_SEQ claim, and it matches lib/thrift-backend/primaryKeysSchema.ts:5-10.

[peco-review-bot]

🔵 Low — The preamble right above the new ## Entries heading still says "No learnings have been recorded yet". This PR adds 17 dated sections, so that sentence is now wrong. The author flow reads this file back as knowledge, so it will see a statement that contradicts the rest of the file. Also, ## Entries comes straight after the paragraph with no blank line in between. Most Markdown renderers still treat it as a heading, but markdownlint (MD022) flags it, and it doesn't match the spacing used elsewhere in the file. Replace the stale sentence (for example, "Dated sections are appended below.") and add a blank line before ## Entries.

[github-actions]

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

[peco-review-bot]

Verdict: 1 Medium · 1 Low · 1 Nit

The learning log is mostly fine as a docs-only update, with one medium issue: the 2026-09-24 entry contradicts the later 2026-10-06 entry on how to handle empty-string catalog arguments. Separately, the lockfile resolved-URL rule now appears three times (2026-09-01, 2026-09-09, 2026-10-07); merging those into one entry would keep the log tidier (nit).

Other findings

• ⚪ Nit — This rule tells bot workflows to set queue: max on the concurrency group. I couldn't verify queue against GitHub's documented concurrency keys (group, cancel-in-progress), and nothing in this diff shows where it comes from. If the key isn't supported, a workflow written from this rule could fail validation or quietly ignore it. Either confirm the key exists and cite the workflow that uses it, or remove it from the rule.

[peco-review-bot]

🟡 Medium — This rule from 2026-09-24 says to normalize ''→undefined on the kernel metadata path, and its context says getPrimaryKeys "explicitly rejects ''". The 2026-10-06 entry (line 129) says the opposite: PR #537 removed that guard, and the rule there is to forward empty strings unchanged and never coerce them in just one backend. The author flow reads this log back as rules, so the bot gets two contradictory instructions for the same code path. Mark the older entry as superseded, or reword it so it matches the later decision. For example: note that empty-catalog semantics differ, and say to document that rather than normalize it.

[peco-review-bot]

🔵 Low — The preamble still says "No learnings have been recorded yet — the first retrospective PR will append the initial dated section below". Directly after it, this PR adds ~170 lines of learnings. There is also no blank line before ## Entries, so the heading comes right after a paragraph. Remove or reword the stale sentence and add a blank line before the heading.

(Anchored to the nearest changed line — see the description for the exact location.)