docs: replace remaining hardcoded Generated-by trailers with self-identifying form - #1032
Conversation
potiuk
left a comment
There was a problem hiding this comment.
LGTM — and the part that makes this an easy approval is that you matched an existing in-repo convention instead of inventing one. Checked each thing I would normally push back on:
- The form is not new.
Generated-by: <agent> (<model>)is already the established shape intools/spec-loop/AGENTS.md, and your explanatory sentence — "where<agent>and<model>are the actual agent and model you are running as … do not hardcode either" — is that file's wording, examples included. So this propagates a convention rather than adding a competing one. That is exactly the right instinct givenAGENTS.md§ Placeholder convention says "Do not invent new placeholders." OpenCode (Big Pickle)is the project's own example, not something you made up — it comes from that same file. I checked, because an unfamiliar agent/model pair in a diff is normally worth querying.- Your scoping call is correct, and it is the subtle part of this change. You placeholder-ised specification and instruction text (the three
SKILL.mdfiles andstep-5-draft-commit/fixtures/output-spec.md) while leaving concrete model names in fixture sample data (.../fixtures/case-*/report.md). That is the right split: a fixture is a record of one concrete run, so a real model name belongs there. It also happens to be safe —step-5g-pr-body/fixtures/assertions.jsongrades structural booleans (has_genai_disclosure_block), not exact strings, so nothing desyncs. AGENTS.md:470's hardcodedGenerated-by: Claude Code (Opus 4.7)is correctly left alone: it is explicitly labelled "Concrete example for Claude Code", the same illustrative style the placeholder table itself uses.
Smaller observations
- Purely for the record, since the PR title says "remaining": hardcoded trailers do still exist under
tools/skill-evals/evals/pre-first-pr-check/.../case-*/report.mdandsecurity-issue-fix/step-5g-pr-body/fixtures/case-1-clean-body/report.md. Per the point above those are sample data and should stay concrete — flagging it only so nobody later reads "remaining" as an invitation to placeholder-ise the fixtures too.
Thanks for this one, and sorry about the collision on your other PR — that was luck of the draw on an identical edit, not a judgement on the work. This change is the more valuable of the two anyway.
This review was drafted by an AI-assisted tool and
confirmed by an Apache Magpie maintainer. The maintainer
approving this PR has read the findings and signed off. If
something feels off, please reply on the PR and a maintainer
will follow up.More on how Apache Magpie handles maintainer review:
CONTRIBUTING.md § Opening a pull request.
Fixes #1000.
Summary
Replaces the four remaining hardcoded
Generated-by:trailers with the self-identifying<agent> (<model>)form, mirroring the wording PR #734 landed intools/spec-loop/PROMPT_build.md(same examples, same "do not hardcode either" instruction):skills/setup-override-upstream/SKILL.md— Step 5 commit guidance now usesGenerated-by: <agent> (<model>)with the two worked examples, keeping the no-Co-Authored-Byrule adjacent and intactskills/setup-shared-config-sync/SKILL.md— both the golden-rule bullet and the walk-through step 5 commit instruction updatedskills/security-issue-fix/SKILL.md— the Gen-AI disclosure block template now uses<agent> (<model>)placeholders, with an added fill-in instructiontools/skill-evals/evals/setup-shared-config-sync/step-5-draft-commit/fixtures/output-spec.md— the eval no longer expects a specific vendor's product name; it now expects the trailer filled with the actual agent/model and explicitly rejects hardcoded vendor valuesScope notes (per the issue)
report.mdfixture was touched (simulated inbound commits — correct as-is)Co-Authored-Byprohibitions adjacent to edited lines are preservedPROMPT_build.md's shape, including theClaude (Opus 4.8)/OpenCode (Big Pickle)examplesTest plan
prek run --all-files— I don't have a local checkout with the toolchain in this environment, so I was unable to run it; the change is docs-only (markdown text substitutions, no structural/frontmatter changes), but happy to fix anything it flags.