fix(songwriting): adjudicate phrasing, song-forms, bridge, and exercises against Essential Guide to Lyric Form and Structure chapters 1, 2, and 6 - #2155
Conversation
…ses against Essential Guide to Lyric Form and Structure chapters 1, 2, and 6 Opens Book 1. Chapters 1, 2, and 6 read in full with every figure, plus Chapter 5's bridge and song-system span. Settles the two claims the 0.8.2 pass had to leave standing, both of which turned out to be wrong. "Four times is a lot" is 2009's warning alone; 1991 Chapter 6 never counts verses. "Southern Comfort" has eight verse phrases, not seven with an eighth withheld -- the eighth arrives and refuses the expected close, which is what makes it a Deceptive Closure. That also dissolves the form.md standoff: both books read eight, differing only in vocabulary. song-forms.md had recorded Chapter 6 as having no linked images. It has 37, and the chapter's entire scansion argument lives in them -- which is how the seven-phrase error survived a previous pass. Also: the three bridge functions are Chapter 6's, not Chapter 5's (Book 1 carries two different bridge lists); "different-size system" belongs to the song system, not the bridge; phrasing.md's appended block restated Chapter 1's spotlight use and got it wrong where the earlier section had it right; the even/odd balance rule shipped without either of Pattison's own overrides; and Chapter 2's three exercises were missing from a file claiming to preserve Chapters 1-7. Paraphrase only. No chapter prose, example lyrics, or exercise answers. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Claude finished @kyle-sexton's task in 4m 7s —— View job Review completeScope: the 9 changed files (
Note on the fresh-docs mandate: this PR only edits prose bodies of skill-context research files plus a version bump — no plugin manifest schema, hook contract, or skill/subagent frontmatter field changed — so the mandate's contract-surface scope doesn't apply here. Finding —
|
|
Claude finished @kyle-sexton's task in 1m 13s —— View job Security review complete
No security issues found. All 9 changed files are non-executable content/metadata:
There are no GitHub Actions workflow changes, no scripts, no |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d19e2ed44
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Claude has reviewed this PR 5 times. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
…aded consumer Addresses three review findings on #2155. "Different-size song system" said the final system gets shorter. That holds for verse/chorus, where systems 1 and 2 are verse-to-chorus and a short bridge shrinks system 3. It is backwards for verse/refrain and AABA, where the bridge groups with the last verse and makes that system LONGER than the verse-only ones before it. Pat's word is "different"; only the direction is form-specific, and what arrives early there is the final verse, not the system's end. The Chapter 5 -> Chapter 6 attribution and the phrase-count reading of function 2 had survived in form.md and bridge-writing-prompt.md, which /songwriting:song-form bridge loads alongside bridge.md. A single invocation would have supplied contradictory sourcing and diagnostics. Also corrected in audit-checklist.md and audit-checklist-prompt.md, found by sweeping for the same attribution rather than only the files the review named. Exercise 5 asserted that any honest four-section sample contains a mixed-speed section. The writer picks the sections, so nothing guarantees it, and the assertion pressures inventing a change that is not there. Multiple labels are now permitted rather than required. Two more caught by the same sweep: form.md attributed "A bridge isn't a verse" to 1991 Chapter 5, but the phrase is absent from Book 1's text layer and song-forms.md and bridge-writing-prompt.md both source it to 2009 Chapter 23. And response-filter.md said V/V/Ch/V/V/Ch runs the chorus four times; it runs the verse four times and the chorus twice. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 55s —— View job Security review complete
No security issues found. All 13 changed files at HEAD
No GitHub Actions workflow changes, no scripts, no |
|
Claude finished @kyle-sexton's task in 2m 51s —— View job Code review completeScope: all 13 changed files across both commits (
Prior findings — now resolved by
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Picks up #2154's tightened changelog-parity gate, which is what CI was running against this branch.
|
Claude finished @kyle-sexton's task in 47s —— View job Security review complete
No security issues found. This synchronize event's diff (
No GitHub Actions workflow changes, no scripts, no This PR remains out of scope for this security lane by content, not by omission — consistent with the three prior security-review passes on this branch. |
|
Claude finished @kyle-sexton's task in 1m 21s —— View job Code review completeScope: re-review triggered by a
Status of the earlier findingThe finding from the prior pass — Row 2 of the "Bridge — three functions" table still reads "Insert a non-matching unit between matching units," which is the bridge's-own-phrase-count framing this PR rejects. The corrective note two lines below (L538-540) and Everything else re-checked against current HEAD
|
Blocked by the new changelog-parity gate, not by this PR's content31 of 33 checks pass. The two failures are
Nothing about the changelog heading changed between those runs. The failureBut What I checked
Probable causeThe rewritten gate routes the heading check through a new helper the old one has_heading() { rendered_lines - | awk -v h="$heading" 'index($0,h)==1{...}' ; }
What I have deliberately NOT done
Happy to open a separate PR against |
…pe two overreaching claims form.md recorded 1991 Chapter 5 as having no linked images. It has 32 (image_rsrc31Z-32Y), and like Chapter 6's 37 they carry the scansion and rhyme-scheme analysis while the prose stops at dangling colons. This is the same false-inventory defect this branch already fixed in song-forms.md for Chapter 6 -- and the same one that let the seven-phrase "Southern Comfort" error survive an earlier pass. The entry now also records that Chapter 5 has not been read in full, so claims sourced to it are not mistaken for verified. form.md's header still attributed only Chapter 5 while its body now cites Chapter 6 for the three bridge functions; book-references.md requires the header to name its sources. bridge.md said that in verse/refrain and AABA the final verse "arrives early." Chapter 6 uses the early-arrival language only in the verse/chorus passage; the verse/refrain passage claims the size change alone. Scoped to what the source says. Ex 5 asked for four sections "you did not write" and then offered the writer's own back catalogue. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 2m 5s —— View job Code review completeScope: re-review triggered by the latest push (
Latest commit (
|
Narrowing the gate failure to a single code pathFollow-up to my comment above — the CI log already contains the discriminator, In the same job, on the same file, in the same process, with the same awk:
Both read That isolates the defect to one line: has_heading() { rendered_lines - | awk -v h="$heading" 'index($0,h)==1{...}' ; }
The likely fix is to have Still not patching it from this PR — but the offer stands if a maintainer would |
|
Claude finished @kyle-sexton's task in 1m 0s —— View job Security review complete
No security issues found. All 13 changed files at HEAD
Checked specifically for this lane's concerns:
This PR is out of scope for this security lane by content, not by omission — consistent with the four prior security-review passes on this branch. |
…y gate (#2159) Closes #2158 ## Problem `changelog-parity-gate` failed PR #2130 twice with `UNDOCUMENTED BUMP: markdown-format went 0.11.3 -> 0.11.4 ...` even though `plugins/markdown-format/CHANGELOG.md` carries `## [0.11.4]` at line 6, column one — a required merge gate confidently asserting the opposite of the truth, while the same command passed locally. PR #2135 then failed the same gate on **every one of its sixteen bumped plugins** (smallest flagged changelog: 15 KB). The regression landed on `main` at 15:19:19Z in #2154 and blocks **every PR that bumps a plugin whose changelog exceeds roughly one stdio buffer (~4 KB)** — which newest-first ordering makes essentially all of them. ## Blast radius — precisely Confined to the `--check-bump` path: `has_heading` is defined inside that branch and called in exactly two places (the head-side check and the base-side `git show "$base:$changelog" | has_heading`). `--check` and `--check-order` read changelogs through `changelog_versions`, whose `grep -oE` stages drain stdin with no early exit and cannot take SIGPIPE. So the failure class is exactly "PRs that bump a manifest version"; both failing call sites go through the one function this PR fixes. ## Root cause `has_heading` runs a pipeline under `set -o pipefail` whose reader `exit`s on first match: ```bash rendered_lines - | awk -v h="$heading" 'index($0, h) == 1 { found = 1; exit } END { exit !found }' ``` The newest heading sits near the top, so the reader exits while `rendered_lines` is still writing; the writer dies of SIGPIPE (141) and pipefail reports the pipeline — the FOUND heading — as a failure. Reproduced deterministically in an `ubuntu:24.04` container at the exact CI merge commit `ba4b72fb`: `PIPESTATUS=141 0` and the byte-identical CI error under **gawk** (what the `ubuntu-24.04` runner resolves `/usr/bin/awk` to — gawk outranks mawk in the alternatives system, and only the gawk mechanism explains CI failing 15 KB files). mawk survives the closed pipe and passes at every size tested, and Windows/MSYS process timing lets the writer finish first — which is why the failure existed only in CI. The suite's 55 fixtures all fit in one buffer — hence `PASS=55` in the very job that then failed on the real file. ## Fix The reader consumes to EOF; `END { exit !found }` decides. Correct **by construction**: no reader exits early, so no writer can ever take SIGPIPE, under any awk — the failure is impossible, not rarer. Chosen over restoring the pre-#2154 single-awk form because it preserves the one-tracker-three-modes property (`rendered_lines` shared by all modes, so they cannot drift). Both `has_heading` call sites are covered (same function); the script has no other early-exiting reader downstream of a pipe (`changelog_versions` greps drain stdin; the `grep -m1` format probe reads a file directly, not a pipe). ## Regression fixture — with its fails-against-unfixed proof, per engine New `--check-bump` case: ~260 KB changelog, new `## [1.1.0]` entry near the top, expected pass. 260 KB deliberately exceeds the 64 KB pipe **capacity**, so against the unfixed script the writer blocks mid-write and the SIGPIPE is deterministic, not a winnable race. | script | gawk | mawk | |---|---|---| | unfixed | **FAIL=1** (exact #2130 error text) | PASS (mawk survives the closed pipe) | | fixed | 56/56 | 56/56 | The fixture's guard is therefore **engine-conditional — it discriminates only where `awk` resolves to gawk**, which is what the runner resolves; this is recorded in the fixture comment. The fix itself is engine-independent. Windows local: 56/56 fixed. ## Related - #2158 — the defect issue this closes - #2154 — shipped the early-exit reader; this is its first contact with a production-size changelog - #2130, #2135, #2155 — blocked by this regression; once this lands, their recomputed merge refs carry the fixed gate and need no branch-side changes - Known adjacent gap, tracked separately and deliberately NOT fixed here: the gate cannot see a **deleted** predecessor heading — a change set that renames the top heading in place (rather than adding a new one above it) destroys a shipped release section while `--check` and `--check-order` both pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…missing --check-bump reported "UNDOCUMENTED BUMP: songwriting went 0.8.2 -> 0.8.3 but plugins/songwriting/CHANGELOG.md has no '## [0.8.3]' entry at head" while that heading was the file's first, at line 6, outside any fence. Root cause, reproduced: has_heading runs `rendered_lines | awk` under `set -o pipefail` (line 63), and the matcher exited on its first hit. That closes the pipe while rendered_lines is still writing, rendered_lines takes SIGPIPE and exits 141, and pipefail promotes 141 to the status of the whole pipeline -- so a FOUND heading reports as absent. It only bites once the unread remainder outgrows the pipe buffer, which is why it passed on short changelogs and on the author's machine and failed in CI on the repo's longest changelog. Against a 79KB fixture the old shape returns 141 and the new one returns 0. Fixed by reading to EOF instead of exiting early. At these sizes that costs nothing. Also hardened the call form it depended on. has_heading passed `-` to mean stdin, but a `-` file operand is a convention rather than something every awk guarantees, and where it is taken literally the helper emits nothing and every heading looks absent. rendered_lines now forwards "$@", so a zero-argument call reaches awk as a zero-operand call -- stdin in every awk. Both defects produce the identical symptom; both are closed. The existing --check-bump pass test used a three-line changelog, which is why this shipped: with the heading on line 3 there is nothing left to write after the matcher stops, so neither defect can fire. Added a >64KiB fixture with the matched heading first, a cross-check that --check (which reaches rendered_lines by path, not stdin) agrees on the same file, and a direct probe that rendered_lines with no argument reads stdin. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Takes main's scripts/check-changelog-parity.sh and its test suite verbatim. #2159 landed the same root-cause fix this branch had staged -- dropping has_heading's early exit so the matcher cannot SIGPIPE rendered_lines under pipefail -- so the local change is redundant and main's version wins. This branch goes back to being a pure songwriting content pass. The one piece not in #2159 is hardening the `-` stdin operand into a zero-argument call. That was defensive rather than demonstrated: the SIGPIPE race is the confirmed cause and is now fixed, and nothing has shown a `-` operand failing anywhere. Left out rather than carried on speculation; it can be raised on its own if evidence for it ever appears.
|
Claude finished @kyle-sexton's task in 1m 8s —— View job Code review completeScope: re-review triggered by a
What changed in this sync
Worth flagging for the thread: Outstanding finding (posted inline, unchanged since the last two passes)
Everything else re-checked at current HEAD
|
|
Claude finished @kyle-sexton's task in 1m 21s —— View job Security review complete
No security issues found. This All 13 PR-scope files remain non-executable content/config:
Checked specifically for this lane's concerns:
This PR remains out of scope for this security lane by content, not by omission — consistent with all five prior security-review passes on this branch. |
…ote under it Review raised this three times and I kept answering it in prose beneath the table. form.md's function-2 "Effect" cell still read "Insert a non-matching unit between matching units" -- the bridge's-own-phrase-count framing this branch exists to reject -- with the correction sitting three lines below as an addendum. A table is the format readers scan, so the row was still handing out the wrong diagnosis to anyone who did not read past it. The row now carries the corrected claim: the SYSTEM changes size, shorter in verse/chorus and longer in verse/refrain and AABA. The paragraph below keeps only the explanation of why the direction differs. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 35s —— View job Security review complete
No security issues found. This The full PR diff (
Checked specifically for this lane's concerns:
This PR remains out of scope for this security lane by content, not by omission — consistent with all six prior security-review passes on this branch. |
1 participant
Opens Book 1. Essential Guide to Lyric Form and Structure (1991)
Chapters 1, 2, and 6 read in full with every figure, plus Chapter 5's
bridge and song-system span read to adjudicate two attributions.
Book 2 is complete (0.8.2). This is the first pass over Book 1, and it settles
both claims the previous pass had to leave standing — both turned out to be
wrong.
The two parked claims
"Four times is a lot" was credited to 1991 Chapter 6 as a shared warning.
It is 2009's alone. Chapter 6 never discusses
V/V/Ch/V/V/Ch, never countsverses, and never names four. Its related-but-different claim is about
pattern-size monotony — running one structural pattern too long "gets boring
fast," remedy a bridge — which he makes twice, once for AABA and once for
verse/chorus.
song-forms.mdnow scopes the warning to Writing BetterLyrics (2009), Chapter 22 and states what 1991 actually says instead.
"Southern Comfort" was read as seven phrases with the eighth withheld. The
verse has eight, rhyming
x a x a x a b b, and the eighth arrives. Itrefuses the three-stress common-meter close and the rhyme resolution the first
seven set up — that refusal is the Deceptive Closure. Nothing is withheld in
phrase count.
That also dissolves the standoff 0.8.2 recorded in
form.md. 2009 Chapter20's "two complete common-meter systems" and 1991 Chapter 6 both read eight;
they differ only in vocabulary (extra stress in the final phrase vs. deceptive
closure). The instruction to cite one book and not merge the counts is removed
rather than re-hedged.
Why the error survived
song-forms.mdrecorded 1991 Chapter 6 as having no linked images. It has37, and they carry the argument — the chapter's entire scansion and
rhyme-column analysis lives in figures while the prose trails off at dangling
colons ("…the seventh phrase only continues the pattern:"). A text-only audit
of this chapter will miscount phrases. The image inventory now says so.
The same falsehood was sitting in
form.mdfor Chapter 5, which has 32.Both are fixed.
form.md's entry also now records that Chapter 5 has not beenread in full, so claims sourced to it are not mistaken for verified.
Book 1 runs ~6× Book 2's figure density. Chapter 1 alone had three
dangling-colon sites whose content exists only in images.
Other fixes
Book 1 carries two different bridge lists and the plugin had merged their
labels. Chapter 5 gives a five-point characterization of what a bridge is;
Chapter 6 gives three purposes that adding one accomplishes.
bridge.mdnow records both and says which to cite for what.
bridge. The claim is about the song system's size, and Pat's word is
different, not shorter: in verse/chorus a short bridge shrinks the last
system, but in verse/refrain and AABA the bridge-plus-final-verse system is
longer than the verse-only ones.
bridge.mdandsong-forms-examples.mdhad it as the bridge's own phrase count — which contradicted
form.md, whereit was already correct.
phrasing.mdstated Chapter 1's spotlight use twice and got it wrong thesecond time. The early section is right (the balancing position is the last
phrase of an even section; stopping is what spotlights). The appended block
said the balance shift is the spotlight — that is Chapter 1's third use,
not its first. Fourth file found with this correct-early/wrong-late shape,
after
song-forms.mdandco-writing.md.overrides. Nesting rescues an odd count (the five-phrase "Fathers and Sons"
verse seems balanced because two short phrases add up to one long one);
closure behavior unbalances an even one ("Southern Comfort" at eight). Applied
mechanically the bare rule misdiagnoses both of his examples.
Heart" speeds up, returns to pace, then slows; Pattison says explicitly that
more than one blank gets filled.
says the surprise phrase spotlights both lines, especially the last.
exercises.mdclaims topreserve Chapters 1-7 but ran 1, 2, 3, 4 then jumped to 8. The gap is exactly
Chapter 2's three. Added Ex 5-7, generalized in the style of Ex 1-4.
(Corroboration:
five-compositional-elements.mdstates the book has 44exercises.)
Three small additions: odd sections can balance each other in pairs; the
reversal test (swap a verse pair, see if the push survives); and the
Common Meter / Ballad Stanza half of Chapter 6's paired form mapping.
What was considered and NOT changed
bridge.md's "must serve at least one, often all three" is left alone.Pat's "It would accomplish three things" describes what a bridge does for
that lyric, not a requirement on every bridge. Tightening it to "all three"
would be generalizing past the evidence in the opposite direction.
destination file end to end — the bridge-sourcing method is already at
song-forms.md:304,434, and the limerick/ABAB material is already atsong-forms.md:170-180,375-386,492-502.phrasing.md's duplicated taxonomy blocks are not collapsed here. Samerestructuring-vs-content-fix split the fix(songwriting): adjudicate form, song-forms, process, and co-writing against Writing Better Lyrics chapters 20-24 + appendix #2114 reviewer scoped as a follow-up;
it joins that item.
bridge.md:39attaches the four-times risk toV/Ch/V/Ch(two verses),contradicting
song-forms.md. Real, but 2009-sourced — not a Book 1 finding,so it is carried as a follow-up rather than smuggled in here.
Verification
typos✅ ·markdownlint✅ 0 errors / 103 files ·lychee✅ 0 errors (exitchecked, not tail-piped) ·
check-changelog-parity --check-bump origin/main✅·
validate-plugins.sh✅. No SKILL.md body changed, socheck-skill.shis notrequired.
changelog-parity-gatefails in CI and I believe the gate is at fault, notthis changelog — see the two comments on this PR. Short version: #2154 landed
a rewritten gate two minutes after this branch's first run passed; the failing
--check-bumpreaches the heading check throughrendered_lines -(stdin)while
--check, which passed in the same job on the same file, reaches itthrough
rendered_lines "$1"(file path). That one line is the wholedifference, and it affects every plugin bump, not just this one. The same
command passes locally on the identical tree. Not worked around and not patched
from here.
check-changelog-parity.shalso hangs locally under Git Bash (--check-orderbefore #2154,
--checkas well after it) — on cleanmaintoo, so it is alocal MSYS issue. Order and uniqueness verified by hand: 0.8.3 → 0.8.2 → 0.8.1
→ 0.8.0 → 0.7.4 …, no repeated version.
The
typosgate again rejected hyphenatedmis-prefixes (mis-count,mis-diagnoses), as it did onmis-transcribedpreviously — closed forms used.Scope
Book 1 Chapters 3, 4, 7 and the bulk of Chapter 5 remain unread; they hit
meter.md,prosody.md, the rhyme files, andhook.md— a disjoint file set,so they belong in a separate pass.
exercises.mdalso skips Ex 32-33(Chapter 5); left alone until Chapter 5 is read in full.
Figure counts for the rest of Book 1, measured while fixing the two false
inventories: Chapter 3 has 59, Chapter 4 has 40, Chapter 7 has 27.
Chapter 3 is the meter chapter, so the next pass is figure-heavy by definition.
Related
No linked issue