Repository navigation
Look at the has_subst data after 2026-08-17 #35
Description
Why we must do this
PR #34 added the has_subst flag to the tool-usage artifact. The flag shows command
substitution in a command. We need this flag to find the cause of the denials in #33.
We have too little data now. We must wait. Then we must look at the data.
When to do this
Do this after 2026-08-17. That date gives one week of data.
Do this before 2026-08-24. The artifacts stay for 14 days only. After that date the
first artifacts are gone.
What to measure
Count the gh pr review rows in the artifacts. Count them two times:
- Count all the attempts. Group them by
has_subst. - Count only the denied attempts. Group them by
has_subst.
Then compare the two denial rates.
How to decide
The decision is simple:
- The denials are mostly
has_subst=true, and the attempts are not. Command substitution
is the cause. Then change the prompt. The reviewer must send the body without
substitution. - The two rates are approximately equal. Command substitution is not the cause. Then
look for a different cause. The--bodylength is one other possible cause.
The data we have now
One run only. This is not enough data for a decision.
The run is monopoly PR#1696 (run 31436210556). The reviewer ran gh pr review one time.
That command had has_subst=true. The command was not denied.
This first result does not agree with the theory in #33. But one run proves nothing.
The numbers to compare against
These are the values before PR #34, from 170 runs:
| Measure | Value |
|---|---|
gh pr review attempts |
241 |
| Attempts denied | 71 (29%) |
Denials that compound flagged |
1 |
| Extra cost for each affected run | +$0.46 |
| Extra time for each affected run | +73s |
How to get the numbers
The artifact for each run holds the counts. Read the commands and denied_commands fields.
gh api "repos/hotdata-dev/<repo>/actions/artifacts?per_page=100" \
--jq '.artifacts[] | select(.name|startswith("claude-tool-usage")) | [.id,.created_at] | @tsv'
gh api "repos/hotdata-dev/<repo>/actions/artifacts/<id>/zip" > a.zipDo not use created=>DATE to filter the runs API. That filter returns zero rows always. Use
a range, like created=2026-08-17..2026-08-24. Or use no filter and compare the timestamps
in your own code.
Related
gh pr reviewdenied on 29% of attempts despite being allowlisted #33 holds the bug and the evidence. Closegh pr reviewdenied on 29% of attempts despite being allowlisted #33 when this measurement gives an answer.
Activity
zfarrell commentedon Aug 11, 2026
Early data (11 runs since the merge, all healthy, flag populated on 11/11):
Two things to carry forward.
The hypothesis is looking weak, not merely unproven. 10 substitution-carrying calls produced 1 denial. If substitution were sufficient to cause a refusal, all 10 would have failed. The 10% rate also sits below the 29% pre-change baseline.
The natural experiment may never give a control group. Substitution-free calls are 2 of 12. If that ratio holds, there will not be enough of them to compare rates against, however long we wait — so the plan in the description ("compare the two denial rates") can stall indefinitely.
If the ratio has not shifted by 2026-08-17, stop waiting and test directly instead: have the reviewer post one verdict with
--body-filerather than--body "$(...)", and see whether the denials stop. That answers the question in one run instead of one week.Note also that the checking script needs a floor on n. An earlier version reported "hypothesis SUPPORTED" from a single denial, because it measured the share of denials carrying substitution (high automatically whenever substitution is common) rather than the denial rate with substitution versus without. Use the rate comparison, and require >=10 denials and >=5 substitution-free attempts first.
zfarrell commentedon Aug 11, 2026
Warning for whoever runs this: do not read the post-merge rate as an improvement.
Post-merge
gh pr reviewdenials are running at 8% (2/24) against the 29% baseline. Clustered by PR that difference holds up statistically (+21pp, 95% CI [+6.6, +33.2], 75 PRs vs 12). It is still not evidence of anything, because PR #34 has no causal path to it —has_substonly adds a field to the artifact projection, in a step that runs after the review. It touches neither the allowlist nor the prompt.A significant difference with no possible mechanism means the comparison is confounded. The likely cause is repo mix: the post-merge sample is overnight traffic concentrated in monopoly plus a few SDK repos, and pre-merge denials were already known to cluster hard (polyglot 16, dlthubworker 11, monopoly 7).
Also checked and worth recording: the baseline was not already falling before the merge. Day by day it was flat-to-rising — Aug 7-8 at 27%, Aug 9-10 at 33%. So the 29% in #33 was accurate and current, and the ticket is not overstated.
Practical consequence: a pre/post comparison across different repo populations will stay confounded however long it runs. Prefer the
--body-filetest in the comment above — same repo, same PR, one variable.(Side note on reading the older artifacts: rows before 2026-08-05 show
attempts=0with 82-100% of runs hitting some denial. Not a contradiction — command labelling shipped Aug 4-5, so earlier artifacts carry denial counts without command labels.)