fix(security-tracker-stats): recognise the framework's own comment marker - #1130
Merged
Merged
Conversation
…rker The default `bot_prefixes` did not list `<!-- apache-magpie: `, the canonical machine marker every framework skill writes on its status-rollup, hand-off, and import comments. Those comments were therefore counted as *human triage activity*, pulling the time-to-triage median toward zero on any tracker the skills touch — and the more the skills are used, the more wrong the metric gets. At the same time the default listed `<!-- airflow-s status rollup v`, a marker specific to one adopter's tracker repo and meaningless to every other project. That is the template-genericity residue the upgrade flow's Step 6d audit exists to catch; apache#1123 removed the last apache-steward references but this one survived. Swaps the leaked adopter marker for the framework's own. Also corrects the README's merge documentation. It said the `milestones` and `categories` lists are replaced entirely, implying other lists merge. `deep_merge` replaces *every* list — its own docstring says "Lists are REPLACED, not concatenated" — so an overlay adding a single `bot_prefixes` entry silently drops all the defaults. That is precisely the trap that makes this bug hard to notice downstream: an adopter fixes the marker locally and unknowingly discards the rest of the list. Verified: the config still parses under the hand-rolled `_minimal_yaml_load` subset parser with the added comments, `is_bot_body` now classifies a real `<!-- apache-magpie: status-rollup v3 -->` comment as bot while leaving a human triage comment alone, and the tool's 114 tests pass. Generated-by: Claude Code (Claude Opus 5)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The default
bot_prefixesdoes not list<!-- apache-magpie:— the canonical machine marker every framework skill writes on its status-rollup, hand-off and import comments. Those comments are therefore counted as human triage activity, pulling the time-to-triage median toward zero on any tracker the skills touch. The more the skills are used, the more wrong the metric gets.Meanwhile the default does list
<!-- airflow-s status rollup v— a marker specific to one adopter's tracker repo, meaningless to every other project. This swaps the leaked adopter marker for the framework's own.Two things worth a reviewer's attention
1. Template-genericity residue. The
airflow-sprefix is exactly what the upgrade flow's Step 6d audit exists to catch. #1123 removed the lastapache-stewardreferences; this one survived because it names an adopter, not the old framework.2. The README's merge documentation was wrong, and that is what made this bug hard to notice downstream. It said the
milestonesandcategorieslists are replaced entirely — implying other lists merge.deep_mergereplaces every list; its own docstring says "Lists are REPLACED, not concatenated." So an adopter who fixes the marker locally by overlaying onebot_prefixesentry silently discards all the defaults. Corrected to say every list replaces.Test plan
_minimal_yaml_loadsubset parser with the added comments — checked directly, since that parser handles only a YAML subset and comments were a real risk.is_bot_bodynow classifies a real<!-- apache-magpie: status-rollup v3 -->comment as bot (True) while leaving a human triage comment alone (False).uv run --project . pytest.prek run --filespasses on both files.🤖 Generated with Claude Code