Skip to content

Add pattern #26 for replies that re-explain shared context (#269) - #271

Open
Kutomore wants to merge 1 commit into
blader:mainfrom
Kutomore:Kutomore/issue-269/over-explaining-conversational-replies
Open

Add pattern #26 for replies that re-explain shared context (#269)#271
Kutomore wants to merge 1 commit into
blader:mainfrom
Kutomore:Kutomore/issue-269/over-explaining-conversational-replies

Conversation

@Kutomore

@Kutomore Kutomore commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Fixes #269.

Problem

Humanizer can remove every sentence-level tell from a conversational reply and still leave it sounding like a generated memo. The example in #269 opens by restating the problem, works through the diagnosis, adds a query and QA counts to show a backfill is feasible, and only then gives the decision. Every sentence is fine on its own, so no current pattern catches it. The reply the issue author actually posted was less than half as long: the decision, plus the one reason it was out of scope.

The core rule, "add something the reader did not already have," only looked at the text being rewritten. In a reply, most of what the model rebuilds is already shared by the other person in the thread.

Change

  • The core rule now counts information from the surrounding conversation, not only from earlier in the text. The "why" list gets a matching bullet.
  • New section F, pattern feat(skill): Add SOTA prompting improvements #26, "Re-explaining what the reader knows." Lead with the decision. Keep only the reasoning that would change whether the reader agrees. The diagnosis and the proof that a plan will work belong in the ticket or document that follows, not in the reply. A reviewer raising a topic is not a request for the full write-up.
  • Scope guard: the pattern acts on a reply when the thread is visible, or when the text is plainly a reply. It leaves standalone writing alone.
  • Synced the README table, the section title, the "how it works" line, and the version history. Changed the section count in AGENTS.md. Version is now 3.1.0.

I added a pattern instead of folding this into an existing one because every other pattern reads the text in isolation and this one needs the thread. Putting #26 in a new short section also keeps 1 through 25 unrenumbered.

Validation

Two blind runs of the updated skill. Each run got only the input and the rule file, not the target rewrite.

Run 1: a reply, thread visible

Context given to the skill: this is a reply in a code review thread. The reviewer has already agreed the change is a workaround and has suggested fixing the underlying bug and backfilling the old data.

Input

Yeah, you're right. This is working around the issue rather than fixing it.

I think the actual fix is in MergeService#process!: when we move a child under a new parent, we should update pipeline_id along with parent_id. The model already expects those to match (parent_pipeline_matches), we're just not keeping that true during a merge.

We should be able to backfill this pretty easily from the audit log too. MergeService writes these changes with source: "merge", so we can find the affected records with:

Change.where(field: "pipeline_id", source: "merge")

I checked QA and found 123 historical merge events, with only 6 records currently having the wrong pipeline, so it looks like a pretty small cleanup.

Since the fix is in the shared MergeService model and isn't specific to this account, I'd rather put that in a separate ticket instead of expanding the scope of this PR. The fallback here is still fine to keep; it'll just stop being necessary once we fix the merge behavior and backfill the existing bad data.

If that split sounds good, I'll create a ticket for the MergeService change plus backfill.

The skill marked #26 as the main tell, with #1 (not-X-but-Y) and #9 (stacked qualifiers) alongside it.

Output

Agreed, this is a workaround. The real fix is in MergeService#process!, where a merge should update pipeline_id alongside parent_id. That code is shared and not specific to this account, so fixing it in this PR would mean going through the merge flow for every account and backfilling the rows that are already wrong.

I'd rather keep this PR account specific and open a separate ticket for the MergeService fix and the backfill. The backfill should be small: the merge events are tagged source: "merge", and on QA only 6 records currently have the wrong pipeline.

The fallback here is fine to keep until that ticket lands. Let me know if the split works and I'll write it up.

The decision comes first, the Change.where(...) query and the "123 merge events" count are gone, and the fix location is down to one sentence. The one fact the reviewer did not have, the 6-record count, stays. The result is about 90 words against 180 in the input, close to the reply the issue author wrote by hand.

Run 2: the same content as a standalone note

Input

Merge and layout assignment

When a record is merged under a new parent, MergeService updates parent_id but never pipeline_id. The layout check in parent_pipeline_matches never fires for a merged record, so it keeps its old layout.

The real fix is in MergeService#process!: when a child moves under a new parent, it should update pipeline_id along with parent_id. This is shared code used by every account, so changing it means checking the merge flow for all of them.

Existing bad data can be backfilled from the audit log. MergeService writes these changes with source: "merge", so the affected rows are Change.where(field: "pipeline_id", source: "merge"). On the QA database that is 123 past merges and 6 rows currently wrong.

Until the shared fix lands, a per-account handler picks up a record that was just re-parented and assigns the correct layout. This is a workaround and can be removed once the merge behavior is fixed and the data is backfilled.

The skill did not fire #26. Its note: "this is a standalone engineering note, not a reply, so the full explanation is appropriate." It kept the diagnosis, the query, and the counts, and changed only sentence-level tells (#3 "The real fix is in", #11 passive voice).

Package checks

python3 scripts/validate-package.py passes. SKILL.md is 392 lines.

@goransh-walia

Copy link
Copy Markdown

Clean and minimal — appreciate that it stays scoped to the actual fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Humanizer can still over-explain conversational replies

2 participants