Constrain --agent to the detector's own vocabulary - #749
Conversation
The flag was required but unvalidated, so every value that was not exactly `claude` took the bounded-tail branch. A capture reduced with `--agent Claude` lost the rows above the tail without saying so, which is the data loss the flag was added to prevent, and a trimmed screen is a plausible screen so nothing downstream could notice. Two of the runtime names are not the case name — `cursor-agent` and `qodercli` — so a wrong value is not only a typo risk. `choices` turns it into an argparse error. The list is a hand copy of `DetectedAgent`, so a test parses the enum and fails when the two drift, and an end-to-end test pins the rejection.
onevtail
left a comment
onevtail
left a comment
There was a problem hiding this comment.
The argparse choices correctly fix the reported failure mode: unknown agent names such as Claude now exit with status 2 instead of silently producing a truncated fixture.
There is one blocking mismatch for the newly accepted pi value. Production keeps 32 non-empty lines for Pi, while this script still applies the default 24-line limit. For a 40-line capture, --agent pi currently emits row 16 through row 39; the production-equivalent result is row 8 through row 39. The corpus check cannot detect this after truncation because applying a 32-line tail to an already truncated 24-line fixture leaves it unchanged.
Please align the Python reduction logic with the production rules (Claude/full, Pi/32, default/24), add coverage for those three categories, and update the help text and fixture README accordingly.
The enum synchronization test also stops parsing at var id:. Swift permits cases after a computed property, so a later case could drift without failing this test. Please either parse the complete enum body or explicitly assert that no cases occur after that point.
onevtail - an assistant to @onevcat
2 participants
Follow-up to the review comment on #702.
--agentis required but unvalidated, so every value that is not exactlyclaudetakes the bounded-tail branch:--agent Claudetherefore reduces a Claude capture to 24 lines and says nothing. That is the loss the flag was added to prevent, and a trimmed screen is a plausible screen, so no later step can catch it. Two of the runtime names are also not the case name,cursor-agentandqodercli, so a wrong value is not only a typo risk.choices=DETECTED_AGENTSturns it into an argparse error and puts the accepted names in--help.The list is a hand copy of
DetectedAgent, which is the part that rots. A test parses the enum out ofDetectedAgent.swiftand compares, so the copy fails the build rather than drifting, in the same shape asAgentScreenRuleCoverageTests. A second test pins the rejection end to end.make checkpasses, 78 script tests.