diff --git a/.changeset/review-feedback-signal.md b/.changeset/review-feedback-signal.md new file mode 100644 index 00000000..3631a5bb --- /dev/null +++ b/.changeset/review-feedback-signal.md @@ -0,0 +1,5 @@ +--- +"review": minor +--- + +Turn on the review-feedback signal: the production GitHub wiring for the thumbs sweep and the live counters. `thumbs-sweep-github.ts` is the octokit-backed `ThumbsSweepPort` (reviewer comments identified per grain; the risks/patterns marker for summary comments, the code-owned Conventional-Comment label taxonomy for inline ones; with per-reactor logins, bounded to PRs updated in the last 14 days and not closed more than 3 days ago), and `run-thumbs-sweep.ts` is the env-configured CLI the consumer repos' scheduled sweep workflows run. Live tallies use the shared positive/negative reaction sets (👍/❤️/🎉/🚀 vs 👎/😕, matching gh-aw's outcome evaluation), and resolved inline threads are counted per PR via GraphQL as their own positive column. Sweep-time reaction seeding is removed (`seedReactions`, the port's `addReactions`, `REVIEW_SWEEP_SEED_REACTIONS`): the nudge pair will instead be seeded at post time by the consumer repos' review workflow, and the sweep's self-reaction filtering keeps those seeds out of all signal. `counters-report.ts` aggregates a directory of downloaded per-run artifacts into the live-counters job summary, synthesizing the run summary from the gh-aw engine artifacts (`safeoutputs.jsonl`, `safe-output-items.jsonl`, `agent_usage.json`) when `summary.json` is absent, and tolerating the known gh-aw artifact staging-path nesting. `counters.ts` now also understands the production validator artifact shape (`{"claims": […]}` with three-state `verification`) and can attribute unjoinable decisions to a fallback source. The `review` package gains its first runtime dependency, `octokit` (pinned exactly); consumer sweep workflows install it with `npm install --omit=dev` in the checked-out `workflows/review` directory. No review semantics change. diff --git a/package.json b/package.json index 3fcc1079..78cf3f80 100644 --- a/package.json +++ b/package.json @@ -24,6 +24,7 @@ "eslint-plugin-prettier": "^4.0.0", "fast-glob": "^3.3.3", "memfs": "^4.51.0", + "octokit": "^5.0.5", "prettier": "^2.6.2", "typescript": "^5.9.3", "vitest": "^4.0.10" diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 019cacac..b943d56f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -50,6 +50,9 @@ importers: memfs: specifier: ^4.51.0 version: 4.51.0 + octokit: + specifier: ^5.0.5 + version: 5.0.5 prettier: specifier: ^2.6.2 version: 2.6.2 @@ -107,7 +110,11 @@ importers: actions/shared-node-cache: {} - workflows/review: {} + workflows/review: + dependencies: + octokit: + specifier: 5.0.5 + version: 5.0.5 packages: @@ -643,6 +650,113 @@ packages: resolution: {integrity: sha512-oGB+UxlgWcgQkgwo8GcEGwemoTFt3FIO9ababBmaGwXIoBKZ+GTy0pP185beGg7Llih/NSHSV2XAs1lnznocSg==} engines: {node: '>= 8'} + '@octokit/app@16.1.2': + resolution: {integrity: sha512-8j7sEpUYVj18dxvh0KWj6W/l6uAiVRBl1JBDVRqH1VHKAO/G5eRVl4yEoYACjakWers1DjUkcCHyJNQK47JqyQ==} + engines: {node: '>= 20'} + + '@octokit/auth-app@8.2.0': + resolution: {integrity: sha512-vVjdtQQwomrZ4V46B9LaCsxsySxGoHsyw6IYBov/TqJVROrlYdyNgw5q6tQbB7KZt53v1l1W53RiqTvpzL907g==} + engines: {node: '>= 20'} + + '@octokit/auth-oauth-app@9.0.3': + resolution: {integrity: sha512-+yoFQquaF8OxJSxTb7rnytBIC2ZLbLqA/yb71I4ZXT9+Slw4TziV9j/kyGhUFRRTF2+7WlnIWsePZCWHs+OGjg==} + engines: {node: '>= 20'} + + '@octokit/auth-oauth-device@8.0.3': + resolution: {integrity: sha512-zh2W0mKKMh/VWZhSqlaCzY7qFyrgd9oTWmTmHaXnHNeQRCZr/CXy2jCgHo4e4dJVTiuxP5dLa0YM5p5QVhJHbw==} + engines: {node: '>= 20'} + + '@octokit/auth-oauth-user@6.0.2': + resolution: {integrity: sha512-qLoPPc6E6GJoz3XeDG/pnDhJpTkODTGG4kY0/Py154i/I003O9NazkrwJwRuzgCalhzyIeWQ+6MDvkUmKXjg/A==} + engines: {node: '>= 20'} + + '@octokit/auth-token@6.0.0': + resolution: {integrity: sha512-P4YJBPdPSpWTQ1NU4XYdvHvXJJDxM6YwpS0FZHRgP7YFkdVxsWcpWGy/NVqlAA7PcPCnMacXlRm1y2PFZRWL/w==} + engines: {node: '>= 20'} + + '@octokit/auth-unauthenticated@7.0.3': + resolution: {integrity: sha512-8Jb1mtUdmBHL7lGmop9mU9ArMRUTRhg8vp0T1VtZ4yd9vEm3zcLwmjQkhNEduKawOOORie61xhtYIhTDN+ZQ3g==} + engines: {node: '>= 20'} + + '@octokit/core@7.0.6': + resolution: {integrity: sha512-DhGl4xMVFGVIyMwswXeyzdL4uXD5OGILGX5N8Y+f6W7LhC1Ze2poSNrkF/fedpVDHEEZ+PHFW0vL14I+mm8K3Q==} + engines: {node: '>= 20'} + + '@octokit/endpoint@11.0.3': + resolution: {integrity: sha512-FWFlNxghg4HrXkD3ifYbS/IdL/mDHjh9QcsNyhQjN8dplUoZbejsdpmuqdA76nxj2xoWPs7p8uX2SNr9rYu0Ag==} + engines: {node: '>= 20'} + + '@octokit/graphql@9.0.3': + resolution: {integrity: sha512-grAEuupr/C1rALFnXTv6ZQhFuL1D8G5y8CN04RgrO4FIPMrtm+mcZzFG7dcBm+nq+1ppNixu+Jd78aeJOYxlGA==} + engines: {node: '>= 20'} + + '@octokit/oauth-app@8.0.3': + resolution: {integrity: sha512-jnAjvTsPepyUaMu9e69hYBuozEPgYqP4Z3UnpmvoIzHDpf8EXDGvTY1l1jK0RsZ194oRd+k6Hm13oRU8EoDFwg==} + engines: {node: '>= 20'} + + '@octokit/oauth-authorization-url@8.0.0': + resolution: {integrity: sha512-7QoLPRh/ssEA/HuHBHdVdSgF8xNLz/Bc5m9fZkArJE5bb6NmVkDm3anKxXPmN1zh6b5WKZPRr3697xKT/yM3qQ==} + engines: {node: '>= 20'} + + '@octokit/oauth-methods@6.0.2': + resolution: {integrity: sha512-HiNOO3MqLxlt5Da5bZbLV8Zarnphi4y9XehrbaFMkcoJ+FL7sMxH/UlUsCVxpddVu4qvNDrBdaTVE2o4ITK8ng==} + engines: {node: '>= 20'} + + '@octokit/openapi-types@27.0.0': + resolution: {integrity: sha512-whrdktVs1h6gtR+09+QsNk2+FO+49j6ga1c55YZudfEG+oKJVvJLQi3zkOm5JjiUXAagWK2tI2kTGKJ2Ys7MGA==} + + '@octokit/openapi-webhooks-types@12.1.0': + resolution: {integrity: sha512-WiuzhOsiOvb7W3Pvmhf8d2C6qaLHXrWiLBP4nJ/4kydu+wpagV5Fkz9RfQwV2afYzv3PB+3xYgp4mAdNGjDprA==} + + '@octokit/plugin-paginate-graphql@6.0.0': + resolution: {integrity: sha512-crfpnIoFiBtRkvPqOyLOsw12XsveYuY2ieP6uYDosoUegBJpSVxGwut9sxUgFFcll3VTOTqpUf8yGd8x1OmAkQ==} + engines: {node: '>= 20'} + peerDependencies: + '@octokit/core': '>=6' + + '@octokit/plugin-paginate-rest@14.0.0': + resolution: {integrity: sha512-fNVRE7ufJiAA3XUrha2omTA39M6IXIc6GIZLvlbsm8QOQCYvpq/LkMNGyFlB1d8hTDzsAXa3OKtybdMAYsV/fw==} + engines: {node: '>= 20'} + peerDependencies: + '@octokit/core': '>=6' + + '@octokit/plugin-rest-endpoint-methods@17.0.0': + resolution: {integrity: sha512-B5yCyIlOJFPqUUeiD0cnBJwWJO8lkJs5d8+ze9QDP6SvfiXSz1BF+91+0MeI1d2yxgOhU/O+CvtiZ9jSkHhFAw==} + engines: {node: '>= 20'} + peerDependencies: + '@octokit/core': '>=6' + + '@octokit/plugin-retry@8.1.0': + resolution: {integrity: sha512-O1FZgXeiGb2sowEr/hYTr6YunGdSAFWnr2fyW39Ah85H8O33ELASQxcvOFF5LE6Tjekcyu2ms4qAzJVhSaJxTw==} + engines: {node: '>= 20'} + peerDependencies: + '@octokit/core': '>=7' + + '@octokit/plugin-throttling@11.0.3': + resolution: {integrity: sha512-34eE0RkFCKycLl2D2kq7W+LovheM/ex3AwZCYN8udpi6bxsyjZidb2McXs69hZhLmJlDqTSP8cH+jSRpiaijBg==} + engines: {node: '>= 20'} + peerDependencies: + '@octokit/core': ^7.0.0 + + '@octokit/request-error@7.1.0': + resolution: {integrity: sha512-KMQIfq5sOPpkQYajXHwnhjCC0slzCNScLHs9JafXc4RAJI+9f+jNDlBNaIMTvazOPLgb4BnlhGJOTbnN0wIjPw==} + engines: {node: '>= 20'} + + '@octokit/request@10.0.11': + resolution: {integrity: sha512-+s7HUxjfFqOMS9VlIwDffq0MikjSAK0gSpG73W+meAvVAvX4MBrHYTK5Bj3Uot55qFT4gzUtfzE4mGWY4Br8/Q==} + engines: {node: '>= 20'} + + '@octokit/types@16.0.0': + resolution: {integrity: sha512-sKq+9r1Mm4efXW1FCk7hFSeJo4QKreL/tTbR0rz/qx/r1Oa2VV83LTA/H/MuCOX7uCIJmQVRKBcbmWoySjAnSg==} + + '@octokit/webhooks-methods@6.0.0': + resolution: {integrity: sha512-MFlzzoDJVw/GcbfzVC1RLR36QqkTLUf79vLVO3D+xn7r0QgxnFoLZgtrzxiQErAjFUOdH6fas2KeQJ1yr/qaXQ==} + engines: {node: '>= 20'} + + '@octokit/webhooks@14.2.0': + resolution: {integrity: sha512-da6KbdNCV5sr1/txD896V+6W0iamFWrvVl8cHkBSPT+YlvmT3DwXa4jxZnQc+gnuTEqSWbBeoSZYTayXH9wXcw==} + engines: {node: '>= 20'} + '@oxc-resolver/binding-android-arm-eabi@11.19.1': resolution: {integrity: sha512-aUs47y+xyXHUKlbhqHUjBABjvycq6YSD7bpxSW7vplUmdzAlJ93yXY6ZR0c1o1x5A/QKbENCvs3+NlY8IpIVzg==} cpu: [arm] @@ -956,6 +1070,9 @@ packages: '@tybys/wasm-util@0.10.1': resolution: {integrity: sha512-9tTaPJLSiejZKx+Bmog4uSubteqTvFrVrURwkmHixBo0G4seD0zUxp98E1DzUBJxLQ3NPwXrGKDiVjwx/DpPsg==} + '@types/aws-lambda@8.10.162': + resolution: {integrity: sha512-Fn658grtLOci1oxi1391vvDWJRKNGWRSqfxRkmN/Iy3c0tQH1USMKEXcPYHLvope+ZgTFocx9FRQJx1muBL6qw==} + '@types/chai@5.2.3': resolution: {integrity: sha512-Mw558oeA9fFbv65/y4mHtXDs9bPnFMZAL/jxdPFUpOHHIXX91mcgEHbS5Lahr+pwZFR8A7GQleRWeI6cGFC2UA==} @@ -1218,6 +1335,9 @@ packages: base64-js@1.5.1: resolution: {integrity: sha512-AKpaYlHn8t4SVbOHCy+b5+KKgvR4vrsD8vbvrbiQJps7fKDTkjkDry6ji0rUJjC0kzbNePLwzxq8iypo41qeWA==} + before-after-hook@4.0.0: + resolution: {integrity: sha512-q6tR3RPqIB1pMiTRMFcZwuG5T8vwp+vUvEG0vuI6B+Rikh5BfPp2fQ82c925FOs+b0lcFQ8CFrL+KbilfZFhOQ==} + better-path-resolve@1.0.0: resolution: {integrity: sha512-pbnl5XzGBdrFU/wT4jqmJVPn2B6UHPBOhzMQkY/SPUPB6QtUXtmBHBIwCbXJol93mOpGMnQyP/+BB19q04xj7g==} engines: {node: '>=4'} @@ -1226,6 +1346,9 @@ packages: resolution: {integrity: sha512-2cGmJupaNgg+QUwVLAucDuWuoMZ6EX9iHDRswZ5lsNYEmwPaRknMPCLZz07yTzVq/83p4o/wzbDZbBrTvGGTIw==} engines: {node: '>=18'} + bottleneck@2.19.5: + resolution: {integrity: sha512-VHiNCbI1lKdl44tGrhNfU3lup0Tj/ZBMJB5/2ZbNXRCPuRCO7ed2mgcK4r17y+KB2EfuYuRaVlwNbAeaWGSpbw==} + brace-expansion@1.1.11: resolution: {integrity: sha512-iCuPHDFgrHX7H2vEI/5xpz07zSHB00TpugqhmYtVmMO6518mCuRMoOYFldEBl0g187ufozdaHgWKcYFb61qGiA==} @@ -2271,6 +2394,9 @@ packages: json-stable-stringify-without-jsonify@1.0.1: resolution: {integrity: sha512-Bdboy+l7tA3OGW6FjyFHWkP5LuByj1Tk33Ljyq0axyzdk9//JSi2u3fP1QSmd1KNwq6VOKYGlAu87CisVir6Pw==} + json-with-bigint@3.5.10: + resolution: {integrity: sha512-Vcx+JVNEBts/xfcoCS69sKrOhOk/3TVlvlT+XzUOefVKnnrbYSCKpDCm10pohsJFtsJVYnwa/cXRZ4eElzaM6w==} + json5@1.0.2: resolution: {integrity: sha512-g1MWMLBiz8FKi1e4w0UyVL3w+iJceWAFBAaBnnGKOpNa5f8TLktkbre1+s6oICydWAm+HRUGTmI+//xv2hvXYA==} hasBin: true @@ -2518,6 +2644,10 @@ packages: obug@2.1.1: resolution: {integrity: sha512-uTqF9MuPraAQ+IsnPf366RG4cP9RtUi7MLO1N3KEc+wb0a6yKpeL0lmk2IB1jY5KHPAlTc6T/JRdC/YqxHNwkQ==} + octokit@5.0.5: + resolution: {integrity: sha512-4+/OFSqOjoyULo7eN7EA97DE0Xydj/PW5aIckxqQIoFjFwqXKuFCvXUJObyJfBF9Khu4RL/jlDRI9FPaMGfPnw==} + engines: {node: '>= 20'} + on-finished@2.4.1: resolution: {integrity: sha512-oVlzkg3ENAhCk2zdv7IJwd/QUD4z2RxRwpkcGY8psCVcCYZNq4wYnVWALHM+brtuJjePWiYF/ClmuDr8Ch5+kg==} engines: {node: '>= 0.8'} @@ -2869,10 +2999,6 @@ packages: resolution: {integrity: sha512-7++dFhtcx3353uBaq8DDR4NuxBetBzC7ZQOhmTQInHEd6bSrXdiEyzCvG07Z44UYdLShWUyXt5M/yhz8ekcb1A==} engines: {node: '>=8'} - side-channel-list@1.0.0: - resolution: {integrity: sha512-FCLHtRD/gnpCiCHEiJLOwdmFP+wzCmDEkc9y7NsYxeF4u7Btsn1ZuwgwJGxImImHicJArLP4R0yX4c2KCrMrTA==} - engines: {node: '>= 0.4'} - side-channel-list@1.0.1: resolution: {integrity: sha512-mjn/0bi/oUURjc5Xl7IaWi/OJJJumuoJFQJfDDyO46+hBWsfaVM65TBHq2eoZBhzl9EchxOijpkbRC8SVBQU0w==} engines: {node: '>= 0.4'} @@ -2885,10 +3011,6 @@ packages: resolution: {integrity: sha512-WPS/HvHQTYnHisLo9McqBHOJk2FkHO/tlpvldyrnem4aeQp4hai3gythswg6p01oSoTl58rcpiFAjF2br2Ak2A==} engines: {node: '>= 0.4'} - side-channel@1.1.0: - resolution: {integrity: sha512-ZX99e6tRweoUXqR+VBrslhda51Nh5MTQwou5tnUDgbtyM0dBgmhEDtWGP/xbKn6hqfPRHujUNwz5fy/wbbhnpw==} - engines: {node: '>= 0.4'} - side-channel@1.1.1: resolution: {integrity: sha512-6x6dK6zJdpTzF4sQeNYxwtvBzf6Eg4GtlesS94HOvTudUeyK2WXAaIfmDgsyslYrRBeFIlsi54AYsFGUuhmvrQ==} engines: {node: '>= 0.4'} @@ -3059,6 +3181,10 @@ packages: resolution: {integrity: sha512-65P7iz6X5yEr1cwcgvQxbbIw7Uk3gOy5dIdtZ4rDveLqhrdJP+Li/Hx6tyK0NEb+2GCyneCMJiGqrADCSNk8sQ==} engines: {node: '>=8.0'} + toad-cache@3.7.4: + resolution: {integrity: sha512-m1TdR/rvT7kgGJZhspNtXdsdYk0fddFpJJFlG5s+UkPFo6lkLoZ3YLOaovPYjq1R75NP5JfeTlSHaOsE09peCg==} + engines: {node: '>=20'} + toidentifier@1.0.1: resolution: {integrity: sha512-o5sSPKEkg/DIQNmH43V0/uerLrpzVedkUh8tGNvaeXpfpuwjKenlSox/2O/BTlZUtEe+JG7s5YhEz608PlAHRA==} engines: {node: '>=0.6'} @@ -3147,6 +3273,12 @@ packages: undici-types@7.18.2: resolution: {integrity: sha512-AsuCzffGHJybSaRrmr5eHr81mwJU3kjw6M+uprWvCXiNeN9SOGwQ3Jn8jb8m3Z6izVgknn1R0FTCEAP2QrLY/w==} + universal-github-app-jwt@2.2.2: + resolution: {integrity: sha512-dcmbeSrOdTnsjGjUfAlqNDJrhxXizjAz94ija9Qw8YkZ1uu0d+GoZzyH+Jb9tIIqvGsadUfwg+22k5aDqqwzbw==} + + universal-user-agent@7.0.3: + resolution: {integrity: sha512-TmnEAEAsBJVZM/AADELsK76llnwcf9vMKuPz8JflO1frO8Lchitr0fNaN9d+Ap0BjKtqWqd/J17qeDnXh8CL2A==} + universalify@0.1.2: resolution: {integrity: sha512-rBJeI5CXAlmy1pV+617WB9J63U6XcazHHF2f2dbJix4XzpUF0RS3Zbj0FGIOCAva5P/d/GBOYaACQ1w+0azUkg==} engines: {node: '>= 4.0.0'} @@ -3984,6 +4116,154 @@ snapshots: '@nodelib/fs.scandir': 2.1.5 fastq: 1.13.0 + '@octokit/app@16.1.2': + dependencies: + '@octokit/auth-app': 8.2.0 + '@octokit/auth-unauthenticated': 7.0.3 + '@octokit/core': 7.0.6 + '@octokit/oauth-app': 8.0.3 + '@octokit/plugin-paginate-rest': 14.0.0(@octokit/core@7.0.6) + '@octokit/types': 16.0.0 + '@octokit/webhooks': 14.2.0 + + '@octokit/auth-app@8.2.0': + dependencies: + '@octokit/auth-oauth-app': 9.0.3 + '@octokit/auth-oauth-user': 6.0.2 + '@octokit/request': 10.0.11 + '@octokit/request-error': 7.1.0 + '@octokit/types': 16.0.0 + toad-cache: 3.7.4 + universal-github-app-jwt: 2.2.2 + universal-user-agent: 7.0.3 + + '@octokit/auth-oauth-app@9.0.3': + dependencies: + '@octokit/auth-oauth-device': 8.0.3 + '@octokit/auth-oauth-user': 6.0.2 + '@octokit/request': 10.0.11 + '@octokit/types': 16.0.0 + universal-user-agent: 7.0.3 + + '@octokit/auth-oauth-device@8.0.3': + dependencies: + '@octokit/oauth-methods': 6.0.2 + '@octokit/request': 10.0.11 + '@octokit/types': 16.0.0 + universal-user-agent: 7.0.3 + + '@octokit/auth-oauth-user@6.0.2': + dependencies: + '@octokit/auth-oauth-device': 8.0.3 + '@octokit/oauth-methods': 6.0.2 + '@octokit/request': 10.0.11 + '@octokit/types': 16.0.0 + universal-user-agent: 7.0.3 + + '@octokit/auth-token@6.0.0': {} + + '@octokit/auth-unauthenticated@7.0.3': + dependencies: + '@octokit/request-error': 7.1.0 + '@octokit/types': 16.0.0 + + '@octokit/core@7.0.6': + dependencies: + '@octokit/auth-token': 6.0.0 + '@octokit/graphql': 9.0.3 + '@octokit/request': 10.0.11 + '@octokit/request-error': 7.1.0 + '@octokit/types': 16.0.0 + before-after-hook: 4.0.0 + universal-user-agent: 7.0.3 + + '@octokit/endpoint@11.0.3': + dependencies: + '@octokit/types': 16.0.0 + universal-user-agent: 7.0.3 + + '@octokit/graphql@9.0.3': + dependencies: + '@octokit/request': 10.0.11 + '@octokit/types': 16.0.0 + universal-user-agent: 7.0.3 + + '@octokit/oauth-app@8.0.3': + dependencies: + '@octokit/auth-oauth-app': 9.0.3 + '@octokit/auth-oauth-user': 6.0.2 + '@octokit/auth-unauthenticated': 7.0.3 + '@octokit/core': 7.0.6 + '@octokit/oauth-authorization-url': 8.0.0 + '@octokit/oauth-methods': 6.0.2 + '@types/aws-lambda': 8.10.162 + universal-user-agent: 7.0.3 + + '@octokit/oauth-authorization-url@8.0.0': {} + + '@octokit/oauth-methods@6.0.2': + dependencies: + '@octokit/oauth-authorization-url': 8.0.0 + '@octokit/request': 10.0.11 + '@octokit/request-error': 7.1.0 + '@octokit/types': 16.0.0 + + '@octokit/openapi-types@27.0.0': {} + + '@octokit/openapi-webhooks-types@12.1.0': {} + + '@octokit/plugin-paginate-graphql@6.0.0(@octokit/core@7.0.6)': + dependencies: + '@octokit/core': 7.0.6 + + '@octokit/plugin-paginate-rest@14.0.0(@octokit/core@7.0.6)': + dependencies: + '@octokit/core': 7.0.6 + '@octokit/types': 16.0.0 + + '@octokit/plugin-rest-endpoint-methods@17.0.0(@octokit/core@7.0.6)': + dependencies: + '@octokit/core': 7.0.6 + '@octokit/types': 16.0.0 + + '@octokit/plugin-retry@8.1.0(@octokit/core@7.0.6)': + dependencies: + '@octokit/core': 7.0.6 + '@octokit/request-error': 7.1.0 + '@octokit/types': 16.0.0 + bottleneck: 2.19.5 + + '@octokit/plugin-throttling@11.0.3(@octokit/core@7.0.6)': + dependencies: + '@octokit/core': 7.0.6 + '@octokit/types': 16.0.0 + bottleneck: 2.19.5 + + '@octokit/request-error@7.1.0': + dependencies: + '@octokit/types': 16.0.0 + + '@octokit/request@10.0.11': + dependencies: + '@octokit/endpoint': 11.0.3 + '@octokit/request-error': 7.1.0 + '@octokit/types': 16.0.0 + content-type: 2.0.0 + json-with-bigint: 3.5.10 + universal-user-agent: 7.0.3 + + '@octokit/types@16.0.0': + dependencies: + '@octokit/openapi-types': 27.0.0 + + '@octokit/webhooks-methods@6.0.0': {} + + '@octokit/webhooks@14.2.0': + dependencies: + '@octokit/openapi-webhooks-types': 12.1.0 + '@octokit/request-error': 7.1.0 + '@octokit/webhooks-methods': 6.0.0 + '@oxc-resolver/binding-android-arm-eabi@11.19.1': optional: true @@ -4200,6 +4480,8 @@ snapshots: tslib: 2.8.1 optional: true + '@types/aws-lambda@8.10.162': {} + '@types/chai@5.2.3': dependencies: '@types/deep-eql': 4.0.2 @@ -4501,6 +4783,8 @@ snapshots: base64-js@1.5.1: {} + before-after-hook@4.0.0: {} + better-path-resolve@1.0.0: dependencies: is-windows: 1.0.2 @@ -4519,6 +4803,8 @@ snapshots: transitivePeerDependencies: - supports-color + bottleneck@2.19.5: {} + brace-expansion@1.1.11: dependencies: balanced-match: 1.0.2 @@ -5512,7 +5798,7 @@ snapshots: dependencies: es-errors: 1.3.0 hasown: 2.0.2 - side-channel: 1.1.0 + side-channel: 1.1.1 ip-address@10.2.0: {} @@ -5724,6 +6010,8 @@ snapshots: json-stable-stringify-without-jsonify@1.0.1: {} + json-with-bigint@3.5.10: {} + json5@1.0.2: dependencies: minimist: 1.2.8 @@ -5973,6 +6261,20 @@ snapshots: obug@2.1.1: {} + octokit@5.0.5: + dependencies: + '@octokit/app': 16.1.2 + '@octokit/core': 7.0.6 + '@octokit/oauth-app': 8.0.3 + '@octokit/plugin-paginate-graphql': 6.0.0(@octokit/core@7.0.6) + '@octokit/plugin-paginate-rest': 14.0.0(@octokit/core@7.0.6) + '@octokit/plugin-rest-endpoint-methods': 17.0.0(@octokit/core@7.0.6) + '@octokit/plugin-retry': 8.1.0(@octokit/core@7.0.6) + '@octokit/plugin-throttling': 11.0.3(@octokit/core@7.0.6) + '@octokit/request-error': 7.1.0 + '@octokit/types': 16.0.0 + '@octokit/webhooks': 14.2.0 + on-finished@2.4.1: dependencies: ee-first: 1.1.1 @@ -6377,11 +6679,6 @@ snapshots: shebang-regex@3.0.0: {} - side-channel-list@1.0.0: - dependencies: - es-errors: 1.3.0 - object-inspect: 1.13.4 - side-channel-list@1.0.1: dependencies: es-errors: 1.3.0 @@ -6402,14 +6699,6 @@ snapshots: object-inspect: 1.13.4 side-channel-map: 1.0.1 - side-channel@1.1.0: - dependencies: - es-errors: 1.3.0 - object-inspect: 1.13.4 - side-channel-list: 1.0.0 - side-channel-map: 1.0.1 - side-channel-weakmap: 1.0.2 - side-channel@1.1.1: dependencies: es-errors: 1.3.0 @@ -6502,7 +6791,7 @@ snapshots: internal-slot: 1.1.0 regexp.prototype.flags: 1.5.4 set-function-name: 2.0.2 - side-channel: 1.1.0 + side-channel: 1.1.1 string.prototype.repeat@1.0.0: dependencies: @@ -6596,6 +6885,8 @@ snapshots: dependencies: is-number: 7.0.0 + toad-cache@3.7.4: {} + toidentifier@1.0.1: {} tr46@0.0.3: {} @@ -6693,6 +6984,10 @@ snapshots: undici-types@7.18.2: {} + universal-github-app-jwt@2.2.2: {} + + universal-user-agent@7.0.3: {} + universalify@0.1.2: {} unixify@1.0.0: diff --git a/workflows/review/README.md b/workflows/review/README.md index a128a1f9..1ce3cbc0 100644 --- a/workflows/review/README.md +++ b/workflows/review/README.md @@ -313,6 +313,74 @@ run by default; every other row is opt-in via `ROUTING` and earns its line throu the eval suite. Per-role Fable-5 / Sonnet experiment arms are eval-suite measurements to run after the suite exists. +### Feedback signal: thumbs sweep and live counters + +Two small scheduled workflows in each consumer repo turn on the tuning loop's +production signal. Both are plain GitHub Actions YAML (not gh-aw), both check +out this repo at the pinned `review-v*` tag and run lib scripts with +`npx -y tsx`, and neither touches review semantics: + +- **Thumbs sweep** (`lib/run-thumbs-sweep.ts`, every 1-2 hours): collects + reactions on the reviewer's comments at both grains (inline review comments, + identified by the code-owned Conventional-Comment label prefixes; the + risks/patterns summary comment, identified by its hidden marker) and posts + exactly one "why?" follow-up per newly-downvoted comment, offering the closed + reason vocabulary (`incorrect` / `unimportant` / `unclear` / `duplicate`). + Reactions are tallied with the same sets gh-aw's outcome evaluation uses + (👍/❤️/🎉/🚀 positive, 👎/😕 negative; a 😕 triggers the follow-up like a 👎), + and resolved inline threads are counted as their own positive column: threads + also get resolved just to clear noise, so resolution is reported alongside + the reaction tallies rather than folded into them. Idempotent across restarts + via the hidden follow-up markers; bounded to PRs updated in the last 14 days + (`REVIEW_SWEEP_LOOKBACK_DAYS`), skipping PRs closed or merged more than 3 + days ago (`REVIEW_SWEEP_CLOSED_GRACE_DAYS`; feedback lands around merge time, + after which a landed PR stops changing). Needs only `pull-requests: write`. + The sweep run needs `npm ci --omit=dev` in the checked-out + `workflows/review/` first (the sweep's `octokit` dependency is pinned exactly + in `package.json`, with the transitive tree locked by the committed + `package-lock.json`); the other lib scripts remain dependency-free. Each run's + `SweepResult` and API-request count land in the job summary. +- **Live counters** (`lib/counters-report.ts`, weekly): the workflow downloads + the review runs' per-run artifacts (bounded window), and the script + aggregates them with `lib/counters.ts` into the job summary — verdict mix, + comments/run, validator drop rate, cost/run. Needs only `actions: read`. + +The reviewer posts as `github-actions[bot]` (gh-aw safe outputs use the +workflow's own token), so that login is both the sweep's `botLogin` filter and +the author of its follow-ups; every count in the sweep excludes that login's +own reactions, so the seeded nudge pair (below) is never live signal. + +### Relationship to the gh-aw outcome-collector + +Consumer repos also run gh-aw's outcome-collector workflow, which periodically +classifies every agentic safe output as accepted / rejected / ignored / +pending and exports the results to Sentry over OTLP. The two systems answer +different questions and neither replaces the other: + +- **Outcome-collector**: passive fleet-wide acceptance telemetry. It never + writes to GitHub, so it can observe engagement but cannot ask *why* a + comment was downvoted. Its data lives in Sentry. +- **Thumbs sweep**: active reason elicitation for the reviewer's tuning loop. + Its "why?" follow-ups produce the closed reason labels that calibrate the + eval-suite judge and feed dismissal learning. Its data lives in each run's + job summary and stdout JSON (not exported to OTel today). + +Two known interactions: + +- **Nudge seeding** is planned as a post-time step in the consumer repos' + review workflow (a custom safe-output job that reacts 👍/👎 to each posted + comment seconds after posting), not in the sweep: gh-aw cannot react to its + own safe outputs natively, and comments posted via `GITHUB_TOKEN` emit no + workflow events, so post-time is the only immediate option. +- Once seeding is live, the outcome-collector's `add_comment` metric for the + review workflow is **inflated by design**: its evaluator counts any reaction + as acceptance with no reactor identity, so every seeded summary comment + reads as `accepted`. The inflation is bounded to that one metric (inline + comments and submitted reviews are evaluated by other means), and the sweep's + identity-filtered tallies are the authoritative reviewer-comment engagement + numbers. An upstream gh-aw change to identity-aware reaction counting would + retire this caveat. + ### Required secrets / variables - `ANTHROPIC_API_KEY` — used by the `claude` engine. diff --git a/workflows/review/lib/counters-report.test.ts b/workflows/review/lib/counters-report.test.ts new file mode 100644 index 00000000..1e4a99e2 --- /dev/null +++ b/workflows/review/lib/counters-report.test.ts @@ -0,0 +1,230 @@ +import {mkdtempSync, mkdirSync, writeFileSync, rmSync} from "node:fs"; +import {tmpdir} from "node:os"; +import {join} from "node:path"; + +import {describe, it, expect, afterEach} from "vitest"; + +import { + findArtifactFile, + loadRunDir, + parseJsonl, + renderCountersMarkdown, + synthesizeSummaryFromGhAw, + UNATTRIBUTED_SOURCE, +} from "./counters-report.ts"; +import {computeRunCounters, joinValidatorDecisions} from "./counters.ts"; + +describe("parseJsonl", () => { + it("keeps valid records and drops blank/malformed lines", () => { + const text = '{"type":"add_comment"}\n\nnot json\n{"type":"x"}'; + expect(parseJsonl(text)).toEqual([{type: "add_comment"}, {type: "x"}]); + }); +}); + +describe("synthesizeSummaryFromGhAw", () => { + it("derives verdict, posted count, and cost from the engine artifacts", () => { + const summary = synthesizeSummaryFromGhAw({ + safeOutputs: [ + '{"type":"create_pull_request_review_comment","body":"a"}', + '{"type":"submit_pull_request_review","event":"REQUEST_CHANGES"}', + '{"type":"upload_artifact"}', + ].join("\n"), + postedItems: [ + '{"type":"create_pull_request_review_comment","url":"u1"}', + '{"type":"add_comment","url":"u2"}', + ].join("\n"), + agentUsage: { + input_tokens: 1000, + output_tokens: 500, + ai_credits: 430.924, + }, + }); + + expect(summary["verdict"]).toBe("REQUEST_CHANGES"); + // The handler log (what actually posted) wins over emitted intent. + expect(summary["postedCommentCount"]).toBe(2); + expect(summary["cost"]).toEqual({usd: 4.30924, tokens: 1500}); + }); + + it("keeps REQUEST_CHANGES when a malformed artifact also carries APPROVE", () => { + const summary = synthesizeSummaryFromGhAw({ + safeOutputs: [ + '{"type":"submit_pull_request_review","event":"REQUEST_CHANGES"}', + '{"type":"submit_pull_request_review","event":"APPROVE"}', + ].join("\n"), + }); + expect(summary["verdict"]).toBe("REQUEST_CHANGES"); + }); + + it("counts emitted intent when the handler log is absent", () => { + const summary = synthesizeSummaryFromGhAw({ + safeOutputs: '{"type":"add_comment","body":"guidance"}', + }); + expect(summary["postedCommentCount"]).toBe(1); + expect(summary["verdict"]).toBeUndefined(); + }); + + it("yields an empty posted count with no artifacts at all", () => { + expect(synthesizeSummaryFromGhAw({})["postedCommentCount"]).toBe(0); + }); +}); + +describe("validator vocabulary folding (counters.ts)", () => { + it("accepts the production {claims: [...]} shape with three-state verification", () => { + const decisions = joinValidatorDecisions( + undefined, + { + claims: [ + {id: "v1", verification: "refuted"}, + {id: "v2", verification: "plausible"}, + {id: "v3", verification: "confirmed"}, + {id: "v4", verification: "unsure"}, // unknown -> skipped + ], + }, + UNATTRIBUTED_SOURCE, + ); + expect(decisions).toEqual([ + {source: UNATTRIBUTED_SOURCE, decision: "drop"}, + {source: UNATTRIBUTED_SOURCE, decision: "keep"}, + {source: UNATTRIBUTED_SOURCE, decision: "keep"}, + ]); + }); + + it("still skips unattributed entries when no fallback source is given", () => { + const decisions = joinValidatorDecisions(undefined, [ + {id: "v1", verdict: "drop"}, + ]); + expect(decisions).toEqual([]); + }); + + it("prefers the claims.json source join when it matches", () => { + const decisions = joinValidatorDecisions( + [{id: "v1", source: "security-auth"}], + [{id: "v1", verdict: "keep"}], + UNATTRIBUTED_SOURCE, + ); + expect(decisions).toEqual([ + {source: "security-auth", decision: "keep"}, + ]); + }); +}); + +describe("loadRunDir", () => { + const cleanups: string[] = []; + afterEach(() => { + for (const dir of cleanups.splice(0)) { + rmSync(dir, {recursive: true, force: true}); + } + }); + + const makeRunDir = (): string => { + const dir = mkdtempSync(join(tmpdir(), "counters-report-")); + cleanups.push(dir); + return dir; + }; + + it("tolerates gh-aw staging-path nesting and synthesizes the summary", () => { + const dir = makeRunDir(); + // The known staging-path bug: artifact contents nested under the + // absolute staging path instead of the artifact root. + const nested = join(dir, "tmp", "gh-aw", "review", "out"); + mkdirSync(nested, {recursive: true}); + writeFileSync( + join(nested, "claim-validator.json"), + JSON.stringify({ + claims: [ + {id: "v1", verification: "refuted"}, + {id: "v2", verification: "confirmed"}, + ], + }), + ); + writeFileSync( + join(dir, "safeoutputs.jsonl"), + '{"type":"submit_pull_request_review","event":"APPROVE"}\n' + + '{"type":"create_pull_request_review_comment"}\n', + ); + writeFileSync( + join(dir, "agent_usage.json"), + JSON.stringify({ + input_tokens: 10, + output_tokens: 5, + ai_credits: 200, + }), + ); + + const {run, hasVerdict} = loadRunDir(dir); + expect(hasVerdict).toBe(true); + expect(run.verdict).toBe("APPROVE"); + expect(run.postedCommentCount).toBe(1); + expect(run.cost).toEqual({usd: 2, tokens: 15}); + expect(run.validatorDecisions).toEqual([ + {source: UNATTRIBUTED_SOURCE, decision: "drop"}, + {source: UNATTRIBUTED_SOURCE, decision: "keep"}, + ]); + }); + + it("flags a run with no submitted review as verdict-less", () => { + const dir = makeRunDir(); + writeFileSync( + join(dir, "safeoutputs.jsonl"), + '{"type":"add_comment","body":"guidance"}\n', + ); + const {run, hasVerdict} = loadRunDir(dir); + expect(hasVerdict).toBe(false); + expect(run.verdict).toBe("HOLD_FOR_HUMAN"); // documented safe fallback + }); + + it("prefers an explicit summary.json over synthesis", () => { + const dir = makeRunDir(); + writeFileSync( + join(dir, "summary.json"), + JSON.stringify({verdict: "REQUEST_CHANGES", postedCommentCount: 4}), + ); + writeFileSync( + join(dir, "safeoutputs.jsonl"), + '{"type":"submit_pull_request_review","event":"APPROVE"}\n', + ); + const {run} = loadRunDir(dir); + expect(run.verdict).toBe("REQUEST_CHANGES"); + expect(run.postedCommentCount).toBe(4); + }); + + it("findArtifactFile returns the shallowest match", () => { + const dir = makeRunDir(); + mkdirSync(join(dir, "deep", "deeper"), {recursive: true}); + writeFileSync(join(dir, "deep", "deeper", "x.json"), "{}"); + writeFileSync(join(dir, "x.json"), "{}"); + expect(findArtifactFile(dir, "x.json")).toBe(join(dir, "x.json")); + expect(findArtifactFile(dir, "missing.json")).toBeUndefined(); + }); +}); + +describe("renderCountersMarkdown", () => { + it("renders the aggregate tables with the verdict-fallback footnote", () => { + const counters = computeRunCounters([ + { + runId: "1", + verdict: "APPROVE", + postedCommentCount: 3, + validatorDecisions: [ + {source: "(unknown)", decision: "keep"}, + {source: "(unknown)", decision: "drop"}, + ], + cost: {usd: 4.2}, + }, + { + runId: "2", + verdict: "HOLD_FOR_HUMAN", + postedCommentCount: 0, + validatorDecisions: [], + }, + ]); + const markdown = renderCountersMarkdown(counters, 1); + expect(markdown).toContain("Aggregated over **2** review runs"); + expect(markdown).toContain("| 1 | 0 | 1 |"); + expect(markdown).toContain("*1 run(s) submitted no review"); + expect(markdown).toContain("| (unknown) | 2 | 1 | 50.0% |"); + expect(markdown).toContain("$4.20 total"); + expect(markdown).toContain("thumbs-sweep run's job summary"); + }); +}); diff --git a/workflows/review/lib/counters-report.ts b/workflows/review/lib/counters-report.ts new file mode 100644 index 00000000..8c8b3810 --- /dev/null +++ b/workflows/review/lib/counters-report.ts @@ -0,0 +1,372 @@ +/** + * CLI that turns a directory of downloaded per-run review artifacts into the + * live-counters report — the "wire the counters somewhere someone looks" half + * of `counters.ts`. The consumer repos' weekly `review-counters` workflows run: + * + * npx -y tsx gh-aw-review-lib/workflows/review/lib/counters-report.ts + * + * where `` holds one subdirectory per review run (named by run id), + * each containing whatever artifacts the workflow downloaded for that run. No + * GitHub calls happen here — the workflow does the (bounded) artifact download + * with `gh api`; this script is a pure aggregation over local files, so the + * counters themselves stay deterministic and unit-testable. + * + * Three artifact layouts are understood, newest first: + * + * 1. The conventional layout `counters.ts` documents (`claims.json`, + * `out/claim-validator.json`, `summary.json`). + * 2. Today's production reality: runs upload `out/**` (per-sub-agent JSON) + * but no `claims.json` or `summary.json`. Validator decisions then count + * under the `(unknown)` source, and the run summary is synthesized from + * two gh-aw engine artifacts every run already has: + * - `safeoutputs.jsonl` (the agent's emitted safe outputs) -> verdict + * (the `submit_pull_request_review` event) and, as a fallback, + * intended comment counts; + * - `safe-output-items.jsonl` (the processed handler log) -> comments + * actually posted; + * - `agent_usage.json` -> model cost (`ai_credits`, 1 credit = $0.01) + * and token totals. + * 3. Anything older/malformed: the affected counters degrade per + * `counters.ts`'s normalisation rules rather than failing the report. + * + * Files are located by basename anywhere under the run directory (shallowest + * match wins), which tolerates the known gh-aw artifact staging-path nesting + * rather than assuming exact zip layouts. + */ + +import {readdirSync, readFileSync, appendFileSync} from "node:fs"; +import {basename, join} from "node:path"; + +import { + computeRunCounters, + normalizeRunArtifacts, + type RunArtifacts, + type RunCounters, +} from "./counters.ts"; + +const isRecord = (value: unknown): value is Record => + typeof value === "object" && value !== null && !Array.isArray(value); + +/** Parse a JSONL string into records, skipping blank/malformed lines. */ +export const parseJsonl = (text: string): Record[] => { + const records: Record[] = []; + for (const line of text.split("\n")) { + const trimmed = line.trim(); + if (trimmed === "") { + continue; + } + try { + const parsed: unknown = JSON.parse(trimmed); + if (isRecord(parsed)) { + records.push(parsed); + } + } catch { + // A malformed line drops a data point, never the report. + } + } + return records; +}; + +/** The gh-aw engine artifacts a run summary can be synthesized from. */ +export type GhAwRunFiles = { + /** `safeoutputs.jsonl` — the agent's emitted safe outputs (with `event`). */ + safeOutputs?: string; + /** `safe-output-items.jsonl` — the handler's log of posted items. */ + postedItems?: string; + /** `agent_usage.json` — parsed engine usage roll-up. */ + agentUsage?: unknown; +}; + +const COMMENT_ITEM_TYPES = new Set([ + "create_pull_request_review_comment", + "add_comment", +]); + +/** + * Synthesize the `summary.json`-shaped object `normalizeRunArtifacts` expects + * from the gh-aw engine artifacts. Pure. Fields the artifacts cannot answer are + * left absent so normalisation applies its documented fallbacks (in particular + * a run with no submitted review normalises to `HOLD_FOR_HUMAN`; the report + * calls those runs out separately rather than hiding them). + */ +export const synthesizeSummaryFromGhAw = ( + files: GhAwRunFiles, +): Record => { + const summary: Record = {}; + + const emitted = + files.safeOutputs === undefined ? [] : parseJsonl(files.safeOutputs); + const posted = + files.postedItems === undefined ? [] : parseJsonl(files.postedItems); + + // Verdict: the review-submission event the agent emitted. (The processed + // items log records the posted review's URL but not its event, so the + // emitted output is the only artifact that carries APPROVE vs + // REQUEST_CHANGES.) The reviewer emits at most one submission; if a + // malformed artifact carries several, the stricter verdict wins. + for (const entry of emitted) { + if (entry["type"] !== "submit_pull_request_review") { + continue; + } + const event = entry["event"]; + if ( + (event === "APPROVE" || event === "REQUEST_CHANGES") && + summary["verdict"] !== "REQUEST_CHANGES" + ) { + summary["verdict"] = event; + } + } + + // Posted comments: prefer the handler's log of what actually landed; + // fall back to the agent's emitted intent when the log is absent. + const countComments = (entries: Record[]): number => + entries.filter((entry) => { + const type = entry["type"]; + return typeof type === "string" && COMMENT_ITEM_TYPES.has(type); + }).length; + summary["postedCommentCount"] = + files.postedItems !== undefined + ? countComments(posted) + : countComments(emitted); + + // Cost: gh-aw's agent_usage.json (ai_credits are cents). + if (isRecord(files.agentUsage)) { + const credits = files.agentUsage["ai_credits"]; + const inputTokens = files.agentUsage["input_tokens"]; + const outputTokens = files.agentUsage["output_tokens"]; + const cost: Record = {}; + if (typeof credits === "number" && Number.isFinite(credits)) { + cost["usd"] = credits / 100; + } + if ( + typeof inputTokens === "number" && + typeof outputTokens === "number" && + Number.isFinite(inputTokens) && + Number.isFinite(outputTokens) + ) { + cost["tokens"] = inputTokens + outputTokens; + } + if (Object.keys(cost).length > 0) { + summary["cost"] = cost; + } + } + + return summary; +}; + +/** + * Source label for validator decisions that cannot be joined to a claim + * (production artifacts do not include `claims.json`; see module docs). + */ +export const UNATTRIBUTED_SOURCE = "(unknown)"; + +/** One loaded run plus provenance the report footnotes. */ +export type LoadedRun = { + run: RunArtifacts; + /** Whether the run carried (or could synthesize) a real verdict. */ + hasVerdict: boolean; +}; + +type ReadFile = (path: string) => string; + +const defaultReadFile: ReadFile = (path) => readFileSync(path, "utf8"); + +/** + * Find a file by basename anywhere under `dir`, shallowest first (tolerates + * the gh-aw artifact staging-path nesting). Returns the path or undefined. + */ +export const findArtifactFile = ( + dir: string, + name: string, +): string | undefined => { + let frontier: string[] = [dir]; + while (frontier.length > 0) { + const next: string[] = []; + for (const current of frontier) { + let entries; + try { + entries = readdirSync(current, {withFileTypes: true}); + } catch { + continue; + } + for (const entry of entries) { + const path = join(current, entry.name); + if (entry.isFile() && entry.name === name) { + return path; + } + if (entry.isDirectory()) { + next.push(path); + } + } + } + frontier = next; + } + return undefined; +}; + +const readIfFound = ( + dir: string, + name: string, + readFile: ReadFile, +): string | undefined => { + const path = findArtifactFile(dir, name); + if (path === undefined) { + return undefined; + } + try { + return readFile(path); + } catch { + return undefined; + } +}; + +const parseIfFound = ( + dir: string, + name: string, + readFile: ReadFile, +): unknown => { + const text = readIfFound(dir, name, readFile); + if (text === undefined) { + return undefined; + } + try { + return JSON.parse(text) as unknown; + } catch { + return undefined; + } +}; + +/** Load one run directory into {@link RunArtifacts}, best-effort. */ +export const loadRunDir = ( + dir: string, + readFile: ReadFile = defaultReadFile, +): LoadedRun => { + const claims = parseIfFound(dir, "claims.json", readFile); + const validator = parseIfFound(dir, "claim-validator.json", readFile); + + let summary = parseIfFound(dir, "summary.json", readFile); + if (!isRecord(summary)) { + const files: GhAwRunFiles = {}; + const safeOutputs = readIfFound(dir, "safeoutputs.jsonl", readFile); + if (safeOutputs !== undefined) { + files.safeOutputs = safeOutputs; + } + const postedItems = readIfFound( + dir, + "safe-output-items.jsonl", + readFile, + ); + if (postedItems !== undefined) { + files.postedItems = postedItems; + } + const agentUsage = parseIfFound(dir, "agent_usage.json", readFile); + if (agentUsage !== undefined) { + files.agentUsage = agentUsage; + } + summary = synthesizeSummaryFromGhAw(files); + } + + const hasVerdict = isRecord(summary) && summary["verdict"] !== undefined; + const run = normalizeRunArtifacts( + {claims, validator, summary}, + basename(dir), + UNATTRIBUTED_SOURCE, + ); + return {run, hasVerdict}; +}; + +const pct = (value: number): string => `${(value * 100).toFixed(1)}%`; + +/** Render the weekly counters report as job-summary Markdown. */ +export const renderCountersMarkdown = ( + counters: RunCounters, + runsWithoutVerdict: number, +): string => { + const lines = [ + "## Review live counters", + "", + `Aggregated over **${counters.runCount}** review runs.`, + "", + "### Verdict mix", + "", + "| APPROVE | REQUEST_CHANGES | HOLD_FOR_HUMAN* |", + "| --- | --- | --- |", + `| ${counters.verdictMix.APPROVE} | ${counters.verdictMix.REQUEST_CHANGES} | ${counters.verdictMix.HOLD_FOR_HUMAN} |`, + "", + `*${runsWithoutVerdict} run(s) submitted no review this window (e.g. the` + + " redundant-approval skip) and count under the HOLD_FOR_HUMAN fallback.", + "", + "### Comments and validator", + "", + `- Posted comments: **${counters.totalComments}** total,` + + ` **${counters.commentsPerRun.toFixed(2)}**/run`, + `- Validator drop rate (all sources): **${pct( + counters.overallValidatorDropRate, + )}**`, + ]; + + if (counters.validatorDropBySource.length > 0) { + lines.push( + "", + "| Source | Judged | Dropped | Drop rate |", + "| --- | --- | --- | --- |", + ); + for (const source of counters.validatorDropBySource) { + lines.push( + `| ${source.source} | ${source.total} | ${ + source.dropped + } | ${pct(source.dropRate)} |`, + ); + } + } + + lines.push("", "### Thumbs and cost", ""); + lines.push( + counters.thumbs.agreeRate === null + ? "- Thumbs: none recorded in per-run artifacts (live tallies are" + + " reported by each thumbs-sweep run's job summary)" + : `- Thumbs: ${counters.thumbs.up} 👍 / ${counters.thumbs.down} 👎` + + ` (agree rate ${pct(counters.thumbs.agreeRate)})`, + ); + lines.push( + counters.cost.usdPerRun === null + ? "- Cost: not recorded" + : `- Cost: $${(counters.cost.totalUsd ?? 0).toFixed(2)} total,` + + ` $${counters.cost.usdPerRun.toFixed(2)}/run`, + ); + lines.push(""); + return lines.join("\n"); +}; + +const main = (): void => { + const runsDir = process.argv[2]; + if (runsDir === undefined) { + throw new Error("usage: counters-report.ts "); + } + + const runDirs = readdirSync(runsDir, {withFileTypes: true}) + .filter((entry) => entry.isDirectory()) + .map((entry) => join(runsDir, entry.name)) + .sort(); + + const loaded = runDirs.map((dir) => loadRunDir(dir)); + const counters = computeRunCounters(loaded.map(({run}) => run)); + const runsWithoutVerdict = loaded.filter( + ({hasVerdict}) => !hasVerdict, + ).length; + + const markdown = renderCountersMarkdown(counters, runsWithoutVerdict); + process.stdout.write(`${JSON.stringify(counters, null, 2)}\n`); + + const summaryPath = process.env["GITHUB_STEP_SUMMARY"]; + if (summaryPath !== undefined && summaryPath.trim() !== "") { + appendFileSync(summaryPath, markdown); + } else { + process.stderr.write(markdown); + } +}; + +// Run only when invoked directly, never on import (tests). +if (typeof require !== "undefined" && require.main === module) { + main(); +} diff --git a/workflows/review/lib/counters.ts b/workflows/review/lib/counters.ts index ce9435de..09f55b6b 100644 --- a/workflows/review/lib/counters.ts +++ b/workflows/review/lib/counters.ts @@ -332,9 +332,12 @@ export type RawRunArtifacts = { /** `claims.json`: each entry carries at least `id` and `source`. */ claims?: unknown; /** - * `out/claim-validator.json`: per-claim `keep`/`drop` verdicts. Accepts a - * bare array or an object with a `results`/`decisions` array; each entry - * carries an `id` and a `verdict` (`keep`/`drop`). + * `out/claim-validator.json`: per-claim validation outcomes. Accepts a bare + * array or an object with a `results`/`decisions`/`claims` array (the + * production validator writes `{"claims": […]}`). Each entry carries an + * `id` and either a `verdict` (`keep`/`drop`) or the three-state + * `verification` (`confirmed`/`plausible`/`refuted`), which folds to + * keep/keep/drop for the drop-rate counter. */ validator?: unknown; /** The run summary: verdict, posted-comment count, thumbs, cost. */ @@ -354,19 +357,51 @@ const extractValidatorEntries = (validator: unknown): unknown[] => { if (Array.isArray(validator["decisions"])) { return validator["decisions"]; } + if (Array.isArray(validator["claims"])) { + return validator["claims"]; + } } return []; }; /** - * Join `claims.json` (id -> source) with `claim-validator.json` (id -> verdict) - * into per-source {@link ValidatorDecision}s. A validator entry whose `id` has no - * matching claim, or whose verdict is neither `keep` nor `drop`, is skipped — a - * malformed artifact drops a data point rather than corrupting the counter. + * Fold a validator entry's outcome to the binary `keep`/`drop` the counters + * aggregate. Two vocabularies appear in real artifacts: the original + * `verdict: keep|drop`, and the three-state `verification: + * confirmed|plausible|refuted` (the recall/precision rebalance) — a `plausible` + * claim is kept-but-downgraded, so for drop-rate purposes it counts as `keep`. + */ +const decisionOf = ( + entry: Record, +): "keep" | "drop" | undefined => { + const verdict = entry["verdict"] ?? entry["verification"]; + if ( + verdict === "keep" || + verdict === "confirmed" || + verdict === "plausible" + ) { + return "keep"; + } + if (verdict === "drop" || verdict === "refuted") { + return "drop"; + } + return undefined; +}; + +/** + * Join `claims.json` (id -> source) with `claim-validator.json` (id -> outcome) + * into per-source {@link ValidatorDecision}s. A validator entry with no + * recognisable outcome is skipped — a malformed artifact drops a data point + * rather than corrupting the counter. An entry whose `id` has no matching claim + * is skipped too, unless `fallbackSource` is given, in which case it is counted + * under that source: production runs upload `out/**` but not `claims.json`, so + * without the fallback every real decision would be discarded and the overall + * drop rate would read as zero activity. */ export const joinValidatorDecisions = ( claims: unknown, validator: unknown, + fallbackSource?: string, ): ValidatorDecision[] => { const sourceById = new Map(); if (Array.isArray(claims)) { @@ -387,18 +422,15 @@ export const joinValidatorDecisions = ( continue; } const id = entry["id"]; - const verdict = entry["verdict"]; - if (typeof id !== "string") { - continue; - } - if (verdict !== "keep" && verdict !== "drop") { + const decision = decisionOf(entry); + if (typeof id !== "string" || decision === undefined) { continue; } - const source = sourceById.get(id); + const source = sourceById.get(id) ?? fallbackSource; if (source === undefined) { continue; } - decisions.push({source, decision: verdict}); + decisions.push({source, decision}); } return decisions; }; @@ -448,6 +480,7 @@ const normalizeCost = ( export const normalizeRunArtifacts = ( raw: RawRunArtifacts, fallbackRunId: string, + fallbackSource?: string, ): RunArtifacts => { const summary: Record = isRecord(raw.summary) ? raw.summary @@ -472,6 +505,7 @@ export const normalizeRunArtifacts = ( const validatorDecisions = joinValidatorDecisions( raw.claims, raw.validator, + fallbackSource, ); const thumbs = normalizeThumbs(summary); diff --git a/workflows/review/lib/run-thumbs-sweep.test.ts b/workflows/review/lib/run-thumbs-sweep.test.ts new file mode 100644 index 00000000..835b6fad --- /dev/null +++ b/workflows/review/lib/run-thumbs-sweep.test.ts @@ -0,0 +1,77 @@ +import {describe, it, expect} from "vitest"; + +import {renderSweepSummary} from "./run-thumbs-sweep.ts"; +import type {SweepResult} from "./thumbs-sweep.ts"; +import type {SweepTraversalStats} from "./thumbs-sweep-github.ts"; + +const stats: SweepTraversalStats = { + pullsScanned: 4, + apiRequests: 31, + reactions: {positive: 5, negative: 2}, + resolvedInlineThreads: 3, +}; + +const result: SweepResult = { + actions: [ + { + grain: "inline", + commentId: 11, + downvotes: 0, + posted: false, + reason: "no-downvote", + }, + { + grain: "summary", + commentId: 22, + downvotes: 2, + posted: true, + reason: "posted", + }, + { + grain: "inline", + commentId: 33, + downvotes: 1, + posted: false, + reason: "already-followed-up", + }, + ], + followupsPosted: 1, +}; + +describe("renderSweepSummary", () => { + it("renders the tallies and the downvoted-comment table", () => { + const markdown = renderSweepSummary(result, stats); + + expect(markdown).toContain("## Thumbs feedback sweep"); + expect(markdown).not.toContain("DRY RUN"); + expect(markdown).toContain( + "Reviewer comments swept: **3** across 4 recently-active PRs", + ); + expect(markdown).toContain("**5 positive / 2 negative**"); + expect(markdown).toContain("threads resolved: **3**"); + expect(markdown).toContain( + "Follow-ups posted this sweep: **1** (already followed up: 1)", + ); + expect(markdown).toContain("API requests used: 31"); + // Only the two downvoted comments appear in the table. + expect(markdown).toContain("| summary | 22 | 2 | posted |"); + expect(markdown).toContain("| inline | 33 | 1 | already-followed-up |"); + expect(markdown).not.toContain("| inline | 11 |"); + }); + + it("marks a dry run in the header", () => { + const markdown = renderSweepSummary(result, stats, {dryRun: true}); + expect(markdown).toContain( + "## Thumbs feedback sweep (DRY RUN — nothing was posted)", + ); + }); + + it("omits the table when nothing is downvoted", () => { + const markdown = renderSweepSummary( + {actions: [], followupsPosted: 0}, + {...stats, reactions: {positive: 0, negative: 0}}, + ); + expect(markdown).toContain("(already followed up: 0)"); + expect(markdown).not.toContain("| Grain |"); + }); +}); diff --git a/workflows/review/lib/run-thumbs-sweep.ts b/workflows/review/lib/run-thumbs-sweep.ts new file mode 100644 index 00000000..3b2191ea --- /dev/null +++ b/workflows/review/lib/run-thumbs-sweep.ts @@ -0,0 +1,195 @@ +/** + * CLI entry for the thumbs feedback sweep — the script the consumer repos' + * scheduled `review-feedback` workflows run: + * + * cd gh-aw-review-lib/workflows/review && npm ci --omit=dev && + * npx -y tsx lib/run-thumbs-sweep.ts + * + * (The `npm ci` supplies `octokit`, this package's one runtime + * dependency — unlike the router/investigation-cap scripts, the sweep talks to + * the GitHub API. It is pinned exactly in `package.json` and its transitive + * tree is locked by the committed `package-lock.json`, so a consumer run + * resolves the same client version this release was tested with.) + * + * All configuration is environment variables, so the consumer workflow is pure + * YAML with no arguments to quote: + * + * GITHUB_TOKEN required; the workflow's token + * (`pull-requests: write` is the only scope the + * sweep needs). + * GITHUB_REPOSITORY `owner/repo`; provided by Actions. + * REVIEW_SWEEP_BOT_LOGIN login the reviewer posts as + * (default `github-actions[bot]`). + * REVIEW_SWEEP_LOOKBACK_DAYS PR-activity window (default 14). + * REVIEW_SWEEP_MAX_PULLS traversal cap (default 200). + * REVIEW_SWEEP_CLOSED_GRACE_DAYS days a closed/merged PR stays in the + * sweep after closing (default 3). + * REVIEW_SWEEP_DRY_RUN `true` to traverse and decide without + * posting anything (first-run + * audit; default `false`). + * REVIEW_SWEEP_WORKFLOW_IDS comma-separated gh-aw workflow ids whose + * call-id markers identify the summary + * comment (default `review`). + * + * Output: the full {@link SweepResult} as JSON on stdout, and — when + * `GITHUB_STEP_SUMMARY` is set — a Markdown digest appended to the job summary + * so every sweep run is auditable from the Actions UI. + */ + +import {appendFileSync} from "node:fs"; + +import { + sweepThumbs, + type SweepResult, + type ThumbsSweepPort, +} from "./thumbs-sweep.ts"; +import { + GithubThumbsSweepPort, + type OctokitRequestFn, + type SweepTraversalStats, +} from "./thumbs-sweep-github.ts"; + +const env = (name: string): string | undefined => { + const value = process.env[name]; + return value === undefined || value.trim() === "" ? undefined : value; +}; + +const intEnv = (name: string): number | undefined => { + const raw = env(name); + if (raw === undefined) { + return undefined; + } + const value = Number.parseInt(raw, 10); + if (!Number.isInteger(value) || value <= 0) { + throw new Error(`${name} must be a positive integer, got: ${raw}`); + } + return value; +}; + +/** Render the auditable Markdown digest for the job summary. */ +export const renderSweepSummary = ( + result: SweepResult, + stats: SweepTraversalStats, + options: {dryRun?: boolean} = {}, +): string => { + const byReason = new Map(); + for (const action of result.actions) { + byReason.set(action.reason, (byReason.get(action.reason) ?? 0) + 1); + } + const downvoted = result.actions.filter((a) => a.downvotes > 0); + + const lines = [ + options.dryRun === true + ? "## Thumbs feedback sweep (DRY RUN — nothing was posted)" + : "## Thumbs feedback sweep", + "", + `- Reviewer comments swept: **${result.actions.length}** across ${stats.pullsScanned} recently-active PRs`, + `- Live reactions observed (bot's own excluded; 👍/❤️/🎉/🚀 vs 👎/😕): **${stats.reactions.positive} positive / ${stats.reactions.negative} negative**`, + `- Reviewer inline threads resolved: **${stats.resolvedInlineThreads}**`, + `- Follow-ups posted this sweep: **${result.followupsPosted}**` + + ` (already followed up: ${ + byReason.get("already-followed-up") ?? 0 + })`, + `- GitHub API requests used: ${stats.apiRequests}`, + ]; + + if (downvoted.length > 0) { + lines.push( + "", + "| Grain | Comment | 👎 | Action |", + "| --- | --- | --- | --- |", + ); + for (const action of downvoted) { + lines.push( + `| ${action.grain} | ${action.commentId} | ${action.downvotes} | ${action.reason} |`, + ); + } + } + + lines.push(""); + return lines.join("\n"); +}; + +const main = async (): Promise => { + const token = env("GITHUB_TOKEN"); + if (token === undefined) { + throw new Error("GITHUB_TOKEN is required"); + } + const repository = env("GITHUB_REPOSITORY"); + if (repository === undefined || !repository.includes("/")) { + throw new Error("GITHUB_REPOSITORY must be set to owner/repo"); + } + const [owner, repo] = repository.split("/", 2) as [string, string]; + + const botLogin = env("REVIEW_SWEEP_BOT_LOGIN") ?? "github-actions[bot]"; + const lookbackDays = intEnv("REVIEW_SWEEP_LOOKBACK_DAYS"); + const maxPulls = intEnv("REVIEW_SWEEP_MAX_PULLS"); + const closedGraceDays = intEnv("REVIEW_SWEEP_CLOSED_GRACE_DAYS"); + const dryRun = env("REVIEW_SWEEP_DRY_RUN") === "true"; + const reviewWorkflowIds = (env("REVIEW_SWEEP_WORKFLOW_IDS") ?? "review") + .split(",") + .map((id) => id.trim()) + .filter((id) => id !== ""); + + // `Octokit` from the `octokit` package ships the throttling/retry plugins, + // so secondary-rate-limit pauses are handled by the client instead of + // failing the scheduled run. Loaded with a dynamic import because the + // package is ESM-only while tsx runs this script as CJS (no `"type": + // "module"` in this package); `import()` crosses that boundary, a static + // import cannot. + const {Octokit} = await import("octokit"); + const octokit = new Octokit({auth: token}); + const request: OctokitRequestFn = (route, params) => + octokit.request(route, params); + + const port = new GithubThumbsSweepPort(request, { + owner, + repo, + botLogin, + reviewWorkflowIds, + ...(lookbackDays !== undefined ? {lookbackDays} : {}), + ...(maxPulls !== undefined ? {maxPulls} : {}), + ...(closedGraceDays !== undefined ? {closedGraceDays} : {}), + }); + + // Dry-run mode (`REVIEW_SWEEP_DRY_RUN=true`): traverse and decide exactly + // as a real sweep would, but swallow the write call. Useful for a + // first-run audit of what a repo's sweep WOULD post. + const effectivePort: ThumbsSweepPort = dryRun + ? { + listBotComments: (grain) => port.listBotComments(grain), + listExistingFollowups: () => port.listExistingFollowups(), + postFollowup: async () => {}, + } + : port; + + const result = await sweepThumbs(effectivePort, { + owner, + repo, + botLogin, + }); + const stats = port.stats(); + + process.stdout.write( + `${JSON.stringify({dryRun, result, stats}, null, 2)}\n`, + ); + + const summaryPath = env("GITHUB_STEP_SUMMARY"); + if (summaryPath !== undefined) { + appendFileSync( + summaryPath, + renderSweepSummary(result, stats, {dryRun}), + ); + } +}; + +// Run only when invoked directly (`npx tsx lib/run-thumbs-sweep.ts`), never +// on import (tests import `renderSweepSummary`). Same guard as the other lib +// CLIs (`investigation-cap.ts`). +if (typeof require !== "undefined" && require.main === module) { + main().catch((error: unknown) => { + // eslint-disable-next-line no-console + console.error(error); + process.exitCode = 1; + }); +} diff --git a/workflows/review/lib/thumbs-sweep-github.test.ts b/workflows/review/lib/thumbs-sweep-github.test.ts new file mode 100644 index 00000000..84d1a5b4 --- /dev/null +++ b/workflows/review/lib/thumbs-sweep-github.test.ts @@ -0,0 +1,336 @@ +import {describe, it, expect} from "vitest"; + +import { + GithubThumbsSweepPort, + INLINE_COMMENT_PREFIXES, + isReviewerInlineBody, + isReviewerSummaryBody, + type OctokitRequestFn, +} from "./thumbs-sweep-github.ts"; +import {buildFollowupMarker, sweepThumbs} from "./thumbs-sweep.ts"; + +/** + * Tests for the octokit-backed port. A fake `request` function dispatches on + * the route template and records writes, so the traversal, the two-grain + * classification, the reaction resolution, and the write routing are all + * exercised without a network. + */ + +const BOT = "github-actions[bot]"; +const NOW = Date.parse("2026-07-08T00:00:00Z"); +const RECENT = "2026-07-07T12:00:00Z"; // inside the 14-day window +const STALE = "2026-05-01T00:00:00Z"; // far outside it +const CLOSED_RECENTLY = "2026-07-07T00:00:00Z"; // inside the 3-day closed grace +const CLOSED_LONG_AGO = "2026-07-03T00:00:00Z"; // past the closed grace + +// Production summary comments carry gh-aw's engine-emitted call-id marker, +// not (yet) the pr-reviewer marker — mirror that shape here. +const SUMMARY_BODY = [ + "## Review Guidance", + "", +].join("\n"); + +const PR_REVIEWER_SUMMARY_BODY = [ + "", + "## Review Guidance", + "", +].join("\n"); + +type RecordedWrite = {route: string; params: Record}; + +/** + * A fake GitHub: one recent PR (#7) carrying reviewer comments at both grains + * plus decoys, and one stale PR (#1) that must never be traversed. + */ +const makeFakeGithub = () => { + const writes: RecordedWrite[] = []; + + const inlineComments = [ + { + id: 101, + user: {login: BOT}, + body: "**issue (blocking):** off-by-one in the prune loop.", + reactions: {total_count: 3}, + }, + { + // The sweep's own earlier follow-up reply: idempotency source, + // never a candidate. + id: 102, + user: {login: BOT}, + body: `${buildFollowupMarker("inline", 999)}\nThanks!`, + reactions: {total_count: 0}, + }, + { + // Bot-authored but not templated like a finding -> ignored. + id: 103, + user: {login: BOT}, + body: "some other workflow's inline note", + reactions: {total_count: 5}, + }, + { + // Human comment -> ignored. + id: 104, + user: {login: "human-dev"}, + body: "**issue (blocking):** looks reviewer-shaped but human.", + reactions: {total_count: 1}, + }, + ]; + + const issueComments = [ + { + id: 201, + user: {login: BOT}, + body: SUMMARY_BODY, + reactions: {total_count: 0}, + }, + { + // Bot-authored, no risks/patterns marker -> another workflow's. + id: 202, + user: {login: BOT}, + body: "Test results: all green", + reactions: {total_count: 3}, + }, + ]; + + const reactionsByComment: Record = { + 101: [ + {content: "-1", user: {login: "human-dev"}}, + {content: "heart", user: {login: "another-dev"}}, // positive set + {content: "+1", user: {login: BOT}}, // the bot's own; must not count + ], + }; + + // PR #7's review threads: the candidate 101's thread is resolved (counts); + // the resolved thread rooted at the human comment 104 must not count. + const reviewThreads = [ + {isResolved: true, comments: {nodes: [{databaseId: 101}]}}, + {isResolved: false, comments: {nodes: [{databaseId: 103}]}}, + {isResolved: true, comments: {nodes: [{databaseId: 104}]}}, + ]; + + const request: OctokitRequestFn = async (route, params = {}) => { + if (route === "GET /repos/{owner}/{repo}/pulls") { + const page = params["page"] as number; + return { + data: + page === 1 + ? [ + {number: 7, updated_at: RECENT, state: "open"}, + { + // Closed within the grace window -> swept. + number: 6, + updated_at: RECENT, + state: "closed", + closed_at: CLOSED_RECENTLY, + }, + { + // Closed past the grace window -> skipped. + number: 5, + updated_at: RECENT, + state: "closed", + closed_at: CLOSED_LONG_AGO, + }, + {number: 1, updated_at: STALE}, + ] + : [], + }; + } + if ( + route === "GET /repos/{owner}/{repo}/pulls/{pull_number}/comments" + ) { + expect([6, 7]).toContain(params["pull_number"]); + const items = + params["pull_number"] === 7 && params["page"] === 1 + ? inlineComments + : []; + return {data: items}; + } + if ( + route === "GET /repos/{owner}/{repo}/issues/{issue_number}/comments" + ) { + expect([6, 7]).toContain(params["issue_number"]); + const items = + params["issue_number"] === 7 && params["page"] === 1 + ? issueComments + : []; + return {data: items}; + } + if (route === "POST /graphql") { + const variables = params["variables"] as Record; + expect(variables["number"]).toBe(7); // only PRs with inline candidates + return { + data: { + data: { + repository: { + pullRequest: { + reviewThreads: { + pageInfo: { + hasNextPage: false, + endCursor: null, + }, + nodes: reviewThreads, + }, + }, + }, + }, + }, + }; + } + if ( + route === + "GET /repos/{owner}/{repo}/pulls/comments/{comment_id}/reactions" + ) { + const id = params["comment_id"] as number; + return { + data: params["page"] === 1 ? reactionsByComment[id] ?? [] : [], + }; + } + if ( + route === + "GET /repos/{owner}/{repo}/issues/comments/{comment_id}/reactions" + ) { + return {data: []}; + } + if (route.startsWith("POST ")) { + writes.push({route, params}); + return {data: {}}; + } + throw new Error(`unexpected route: ${route}`); + }; + + return {request, writes}; +}; + +const makePort = (request: OctokitRequestFn) => + new GithubThumbsSweepPort(request, { + owner: "Khan", + repo: "webapp", + botLogin: BOT, + now: NOW, + }); + +describe("comment identification", () => { + it("recognises every code-owned conventional label as an inline prefix", () => { + expect(INLINE_COMMENT_PREFIXES).toContain("**issue (blocking):**"); + expect(INLINE_COMMENT_PREFIXES).toContain( + "**suggestion (non-blocking, best-practice):**", + ); + expect(isReviewerInlineBody("**todo (blocking):** add the test.")).toBe( + true, + ); + expect(isReviewerInlineBody("regular prose")).toBe(false); + }); + + it("identifies the summary comment by either hidden marker", () => { + // The spec'd pr-reviewer marker matches unconditionally. + expect(isReviewerSummaryBody(PR_REVIEWER_SUMMARY_BODY)).toBe(true); + // The engine-emitted call-id marker matches when configured. + expect(isReviewerSummaryBody(SUMMARY_BODY)).toBe(false); + expect( + isReviewerSummaryBody(SUMMARY_BODY, [ + "", + ]), + ).toBe(true); + expect(isReviewerSummaryBody("Test results: all green")).toBe(false); + }); +}); + +describe("traversal and classification", () => { + it("lists reviewer comments at both grains, excluding decoys and follow-ups", async () => { + const {request} = makeFakeGithub(); + const port = makePort(request); + + const inline = await port.listBotComments("inline"); + expect(inline.map((c) => c.id)).toEqual([101]); + // Reactor logins survive so the sweep can exclude the bot's own. + expect(inline[0]?.reactions).toEqual([ + {content: "-1", user: "human-dev"}, + {content: "heart", user: "another-dev"}, + {content: "+1", user: BOT}, + ]); + + const summary = await port.listBotComments("summary"); + expect(summary.map((c) => c.id)).toEqual([201]); + + const followups = await port.listExistingFollowups(); + expect(followups).toHaveLength(1); + expect(followups[0]).toContain("comment-id=999"); + }); + + it("stays within the lookback + closed-grace windows and reports auditable stats", async () => { + const {request} = makeFakeGithub(); + const port = makePort(request); + await port.listBotComments("inline"); + + const stats = port.stats(); + // The open PR and the recently-closed PR are traversed; the PR closed + // past the grace window and the stale PR are not. + expect(stats.pullsScanned).toBe(2); + // The human 👎 and ❤️ count (shared reaction sets); the bot's own 👍 + // is excluded. + expect(stats.reactions).toEqual({positive: 1, negative: 1}); + // Candidate 101's thread is resolved; the non-candidate threads on the + // same PR are not counted. + expect(stats.resolvedInlineThreads).toBe(1); + expect(stats.apiRequests).toBeGreaterThan(0); + }); +}); + +describe("writes", () => { + it("routes follow-ups per grain (inline reply vs PR comment)", async () => { + const {request, writes} = makeFakeGithub(); + const port = makePort(request); + await port.listBotComments("inline"); // populate the comment->PR map + + await port.postFollowup({ + grain: "inline", + commentId: 101, + body: "why?", + }); + await port.postFollowup({ + grain: "summary", + commentId: 201, + body: "why?", + }); + + expect(writes.map((w) => w.route)).toEqual([ + "POST /repos/{owner}/{repo}/pulls/{pull_number}/comments/{comment_id}/replies", + "POST /repos/{owner}/{repo}/issues/{issue_number}/comments", + ]); + expect(writes[0]?.params["pull_number"]).toBe(7); + expect(writes[0]?.params["comment_id"]).toBe(101); + expect(writes[1]?.params["issue_number"]).toBe(7); + }); + + it("rejects a follow-up for a comment the traversal never saw", async () => { + const {request} = makeFakeGithub(); + const port = makePort(request); + await port.listBotComments("inline"); + await expect( + port.postFollowup({grain: "inline", commentId: 555, body: "?"}), + ).rejects.toThrow(/unknown comment/); + }); +}); + +describe("end-to-end with the sweep core", () => { + it("follows up the human 👎 exactly once", async () => { + const {request, writes} = makeFakeGithub(); + const port = makePort(request); + + const result = await sweepThumbs(port, { + owner: "Khan", + repo: "webapp", + botLogin: BOT, + }); + + // The human 👎 on 101 draws exactly one follow-up, threaded inline. + expect(result.followupsPosted).toBe(1); + const followupWrites = writes.filter((w) => + w.route.includes("replies"), + ); + expect(followupWrites).toHaveLength(1); + expect(String(followupWrites[0]?.params["body"])).toContain( + "review-thumbs-followup grain=inline comment-id=101", + ); + }); +}); diff --git a/workflows/review/lib/thumbs-sweep-github.ts b/workflows/review/lib/thumbs-sweep-github.ts new file mode 100644 index 00000000..60ca5860 --- /dev/null +++ b/workflows/review/lib/thumbs-sweep-github.ts @@ -0,0 +1,685 @@ +/** + * The octokit-backed {@link ThumbsSweepPort} — the production GitHub + * implementation of the side-effect boundary `thumbs-sweep.ts` defines. + * + * Division of labour (unchanged from the sweep module): `thumbs-sweep.ts` owns + * all control flow and idempotency; this module owns only the GitHub traversal — + * which comments are the reviewer's, at which grain, with which reactions and + * thread state — and the write call (`postFollowup`). It holds no sweep logic: + * a bug here can mis-list or mis-post, but it cannot re-ping, because that rule + * lives in the sweep core. + * + * How the reviewer's comments are identified (per grain): + * + * - `summary` — issue comments authored by `botLogin` that carry one of the + * workflow's hidden markers: the risks/patterns marker + * (`pr-reviewer:risks-and-patterns`, the exact marker line `review.md` + * Step 7 requires the comment to begin with) or gh-aw's engine-emitted + * `gh-aw-workflow-call-id` marker for the repo's review install (what + * observed production comments actually carry). The marker, not the + * author, is what scopes the sweep to the reviewer: + * `github-actions[bot]` authors many other workflows' comments. + * - `inline` — pull-request review comments authored by `botLogin` whose body + * starts with one of the reviewer's code-owned Conventional-Comment labels + * (`**issue (blocking):** …`, `render-comment.ts`'s taxonomy). Inline + * comments carry no hidden marker at current releases, so the label grammar + * is the identifying signature; it is code-owned and templated, so the match + * is exact, not heuristic prose-sniffing. + * + * Comments containing a thumbs-followup marker are never candidates at either + * grain — they are returned through `listExistingFollowups` instead, which is + * what makes the sweep idempotent across restarts with no state store. + * + * API-call bounding: the traversal reads only pull requests updated within + * `lookbackDays` (default 14), newest first, capped at `maxPulls`; closed or + * merged PRs are skipped once closed for more than `closedGraceDays` (default + * 3) — feedback lands around merge time, and a landed PR's reactions stop + * changing shortly after. Reactions are fetched per comment only when the + * comment's reaction summary shows any reactions at all. Resolved inline + * threads are counted with one GraphQL query per PR that has reviewer inline + * comments (thread resolution is not on the REST comment listing). Every + * request is counted and reported via {@link GithubThumbsSweepPort.stats} so + * each run's API budget is auditable. + */ + +import type { + BotComment, + FeedbackGrain, + PostedFollowup, + Reaction, + ThumbsSweepPort, +} from "./thumbs-sweep.ts"; +import { + NEGATIVE_REACTIONS, + parseFollowupMarkers, + POSITIVE_REACTIONS, +} from "./thumbs-sweep.ts"; +import {BLOCKING_LABELS, NON_BLOCKING_LABELS} from "./render-comment.ts"; + +/** + * The one octokit surface this module needs: `octokit.request`. Kept this + * narrow so tests fake a single function and the module never depends on + * octokit's types; the real client (constructed in `run-thumbs-sweep.ts`) + * satisfies it directly. + */ +export type OctokitRequestFn = ( + route: string, + params?: Record, +) => Promise<{data: unknown}>; + +/** + * The hidden marker that identifies the reviewer's risks/patterns summary + * comment (`review.md` Step 7 requires the comment to begin with this exact + * marker line). Matched as a substring so surrounding whitespace or the + * version marker never break identification. + */ +export const SUMMARY_COMMENT_MARKER = ""; + +/** + * The engine-emitted marker gh-aw appends to every comment a workflow posts: + * ``. Observed + * production summary comments carry this but not the `pr-reviewer` marker + * (the orchestrator's marker line is not reliably emitted on older pins), so + * the sweep accepts either. The reviewer's only `add-comment` output is the + * risks/patterns comment (`max: 1`, status comments disabled), so scoping by + * the review workflow's call id is exact, not heuristic. + */ +export const workflowCallIdMarker = ( + owner: string, + repo: string, + workflowId: string, +): string => ``; + +/** + * Body prefixes that identify a reviewer inline comment: the code-owned + * Conventional-Comment label taxonomy, exactly as `renderComment` templates it + * (`**