Repository navigation
gh search commands returns duplicate data when --limit greater than 100 but not a multiple of 100 #9749
Description
Activity
Thanks for opening this up, @wknapik, that is some weird behavior! 🤔 I'd like to share my theory and propose a few alternatives for reproducing this.
Essentially, I'm not sure
wc -lorsort -u | wc -lare appropriate ways to manipulate and understand this data. It would help to preserve the output in files that we can compare and understand why they are differing.Let's try to reproduce this
Here are some alterations I like to propose understanding what's going on:
-
Let's pick a concrete date to look for merges
Let's base this on the date you opened this issue:
2024-09-14 -
Let's consider preserving the output from the command into a file so we can compare how they are different
-
Let's use
jqto count the length of the arrays rather thanwc -l_We have to use
jqrather than--jqif we want to preserve the output, should only require:... | jq '. | length'
The result looks something like this:
# We should be able to see deviation in GitHub API responses after 10 attempts with the same arguments for i in {1..10}; do \ echo "Attempt $i"; gh search prs --limit 256 --merged --merged-at ">2024-09-14" --base master --repo brave/brave-core --json number > "cli-9749-$i.json"; done # This is a more accurate way to look purely at the number of items find . -name "cli-9749-*.json" -print -exec jq '. | length' {} \; for i in {2..10}; do \ diff <(jq 'sort' cli-9749-1.json) <(jq 'sort' "cli-9749-$i.json"); done
In theory, this will check the number of results and the actual contents.
How did this work out?
10 attempts all consistent both number of results and actual contents
andyfeller@Andys-MBP:~ $ gh version gh version 2.58.0 (2024-10-01) https://github.com/cli/cli/releases/tag/v2.58.0 andyfeller@Andys-MBP:~ $ for i in {1..10}; do \ echo "Attempt $i"; gh search prs --limit 256 --merged --merged-at ">2024-09-14" --base master --repo brave/brave-core --json number > "cli-9749-$i.json"; done Attempt 1 Attempt 2 Attempt 3 Attempt 4 Attempt 5 Attempt 6 Attempt 7 Attempt 8 Attempt 9 Attempt 10 andyfeller@Andys-MBP:~$ find . -name "cli-9749-*.json" -print -exec jq '. | length' {} \; ./cli-9749-2.json 256 ./cli-9749-3.json 256 ./cli-9749-8.json 256 ./cli-9749-4.json 256 ./cli-9749-5.json 256 ./cli-9749-10.json 256 ./cli-9749-9.json 256 ./cli-9749-6.json 256 ./cli-9749-7.json 256 ./cli-9749-1.json 256 andyfeller@Andys-MBP:~ $ for i in {2..10}; do \ diff <(jq 'sort' cli-9749-1.json) <(jq 'sort' "cli-9749-$i.json"); done andyfeller@Andys-MBP:~ $
I don't believe
gh searchcaches results of these calls, however it should be easy enough to addgh config clear-cachebetweengh search prscalls to ensure it clears any potential cache.Next steps
@wknapik : I'd like you to attempt reproducing this problem while preserving the results from the
gh search prscommands and sharing your thoughts on where you are seeing variants.-
Hi @andyfeller. Your code does not appear to check for duplicates, only for consistency. When I run it, I get the same results you do. There's 256 results, including duplicates, identical every time, not affected by running
gh config clear-cache.I think we might be misunderstanding each other.
The point is that there are duplicate results. There shouldn't be any, regardless of what value is passed to
--limit.The commands I shared are meant to illustrate that for the
--limitvalue of 256, I got 56 duplicates, for 275 I got 50 duplicates and for 300 there were no duplicates.It's also interesting to note that up to a point, the duplicates are added until the total number of results returned is equal to the value of
--limit. Past that point, there are no duplicates anymore, regardless of the--limitvalue.cli-9749-1.json ends with
{"number":25808},{"number":25806},{"number":25805},{"number":25804},{"number":25802},{"number":25799},{"number":25798},{"number":25797},{"number":25796},{"number":25795},{"number":25794},{"number":25793},{"number":25792},{"number":25788},{"number":25785},{"number":25784},{"number":25783},{"number":25780},{"number":25779},{"number":25777},{"number":25776},{"number":25775},{"number":25774},{"number":25771},{"number":25770},{"number":25769},{"number":25768},{"number":25767},{"number":25764},{"number":25761},{"number":25760},{"number":25759},{"number":25758},{"number":25757},{"number":25752},{"number":25750},{"number":25749},{"number":25748},{"number":25747},{"number":25744},{"number":25743},{"number":25740},{"number":25738},{"number":25737},{"number":25736},{"number":25735},{"number":25734},{"number":25731},{"number":25730},{"number":25726},{"number":25721},{"number":25720},{"number":25719},{"number":25718},{"number":25717},{"number":25716}which is a repetition of an identical sequence earlier in the file - those are the 56 duplicates
@andyfeller are you, or anyone else looking into this? Please at least remove the
needs-triage/needs-user-inputlabels.Reacted by Andy FellerI think we might be misunderstanding each other.
The point is that there are duplicate results. There shouldn't be any, regardless of what value is passed to
--limit.The commands I shared are meant to illustrate that for the
--limitvalue of 256, I got 56 duplicates, for 275 I got 50 duplicates and for 300 there were no duplicates.@wknapik : I don't think we're misunderstanding each other, but rather there isn't enough information to understand why your results are inconsistent.
% gh -R brave/brave-core search prs --limit 256 --merged --merged-at ">$(date -I -d '1 month ago')" --base=master --json number -q '.[].number'|wc -l 256 % gh -R brave/brave-core search prs --limit 256 --merged --merged-at ">$(date -I -d '1 month ago')" --base=master --json number -q '.[].number'|sort -u|wc -l 200
From these commands, we're solely counting the number of lines in a JSON document. I don't know if the content from these calls can be shared, but one thing that is misleading to me is uniquely sorting a JSON document and counting lines vs total lines especially when there is extra lines due to JSON prettifying.
cli-9749-1.json ends with
{"number":25808},{"number":25806},{"number":25805},{"number":25804},{"number":25802},{"number":25799},{"number":25798},{"number":25797},{"number":25796},{"number":25795},{"number":25794},{"number":25793},{"number":25792},{"number":25788},{"number":25785},{"number":25784},{"number":25783},{"number":25780},{"number":25779},{"number":25777},{"number":25776},{"number":25775},{"number":25774},{"number":25771},{"number":25770},{"number":25769},{"number":25768},{"number":25767},{"number":25764},{"number":25761},{"number":25760},{"number":25759},{"number":25758},{"number":25757},{"number":25752},{"number":25750},{"number":25749},{"number":25748},{"number":25747},{"number":25744},{"number":25743},{"number":25740},{"number":25738},{"number":25737},{"number":25736},{"number":25735},{"number":25734},{"number":25731},{"number":25730},{"number":25726},{"number":25721},{"number":25720},{"number":25719},{"number":25718},{"number":25717},{"number":25716}which is a repetition of an identical sequence earlier in the file - those are the 56 duplicates
This is helpful, thank you!
Could I trouble you to redirect the output of the calls to files and attach them to this issue please? Also running these commands with
GH_DEBUG=apiwould help provide more information on what the API is returning.While you do that, we can begin digging into the code to understand why there might be duplicates.
@andyfeller are you, or anyone else looking into this? Please at least remove the
needs-triage/needs-user-inputlabels.@wknapik : I'd like to get the actual JSON output from the commands you are running and attaching them to this issue and identify where the bug actually is before we can confirm it and figure out a path forward.
What is
gh search prsdoing under the covers and how might it be returning duplicate results?Doing some cursory digging, I wanted to highlight the core of PR searching code:
Lines 133 to 195 in e390874
- initial call to the REST search API retrieves as much as we can get
- it iterates onto next pages as the API response indicates there is more data
- we loop as there are pages and we still need data
Again, I think having the actual output from the commands before running them through
wc -land theGH_DEBUG=apiwould let us understand whether there are responses coming back from the GitHub API that are retrieving more data than necessary or if the API is returning redundant results. 🤔From these commands, we're solely counting the number of lines in a JSON document. I don't know if the content from these calls can be shared, but one thing that is misleading to me is uniquely sorting a JSON document and counting lines vs total lines especially when there is extra lines due to JSON prettifying.
This isn't a JSON document. It's a list of integers (PR numbers), one per line.
Here's the original file produced by your code snippet on 2024-10-16:
gh search prs --limit 256 --merged --merged-at ">2024-09-14" --base master --repo brave/brave-core --json number > "cli-9749-$i.json";
Could I trouble you to redirect the output of the calls to files and attach them to this issue please? Also running these commands with GH_DEBUG=api would help provide more information on what the API is returning.
Here's a file produced just now via:
GH_DEBUG=api gh search prs --limit 256 --merged --merged-at ">2024-09-14" --base master --repo brave/brave-core --json number >cli-debug.txt 2>&1
The contents of the file is now different than a week ago, but it still contains 56 duplicates:
% jq -r '.[].number'|wc -l 256 % jq -r '.[].number'|sort -u|wc -l 200 %
Once again, the duplicates are at the end - the following sequence is a repetition of an identical sequence of 56 numbers earlier in the file:
{"number":25926},{"number":25923},{"number":25920},{"number":25918},{"number":25917},{"number":25914},{"number":25913},{"number":25912},{"number":25911},{"number":25910},{"number":25909},{"number":25908},{"number":25907},{"number":25906},{"number":25905},{"number":25904},{"number":25898},{"number":25897},{"number":25896},{"number":25895},{"number":25892},{"number":25889},{"number":25888},{"number":25887},{"number":25883},{"number":25880},{"number":25879},{"number":25878},{"number":25877},{"number":25875},{"number":25874},{"number":25873},{"number":25870},{"number":25868},{"number":25865},{"number":25864},{"number":25863},{"number":25862},{"number":25861},{"number":25860},{"number":25857},{"number":25855},{"number":25854},{"number":25853},{"number":25852},{"number":25851},{"number":25850},{"number":25847},{"number":25846},{"number":25845},{"number":25844},{"number":25843},{"number":25842},{"number":25840},{"number":25839},{"number":25838}Reacted by Andy FellerThanks, @wknapik, I see what you mean and trying to reverse engineer how this is happening 👍
Personal investigation notes
From the following commands, it appears that
gh search prsis issuing the appropriate GitHub Search API calls to pull data forpageandper_page🤔GH_DEBUG=api gh search prs --limit 256 --merged --merged-at ">2024-09-14" --base master --repo brave/brave-core --json numberGET /search/issues?page=1&per_page=100&q=base%3Amaster+is%3Amerged+merged%3A%3E2024-09-14+repo%3Abrave%2Fbrave-core+type%3Apr HTTP/1.1GET /search/issues?page=2&per_page=100&q=base%3Amaster+is%3Amerged+merged%3A%3E2024-09-14+repo%3Abrave%2Fbrave-core+type%3Apr HTTP/1.1GET /search/issues?page=3&per_page=56&q=base%3Amaster+is%3Amerged+merged%3A%3E2024-09-14+repo%3Abrave%2Fbrave-core+type%3Apr HTTP/1.1
GH_DEBUG=api gh search prs --limit 275 --merged --merged-at ">2024-09-14" --base master --repo brave/brave-core --json numberGET /search/issues?page=1&per_page=100&q=base%3Amaster+is%3Amerged+merged%3A%3E2024-09-14+repo%3Abrave%2Fbrave-core+type%3Apr HTTP/1.1GET /search/issues?page=2&per_page=100&q=base%3Amaster+is%3Amerged+merged%3A%3E2024-09-14+repo%3Abrave%2Fbrave-core+type%3Apr HTTP/1.1GET /search/issues?page=3&per_page=75&q=base%3Amaster+is%3Amerged+merged%3A%3E2024-09-14+repo%3Abrave%2Fbrave-core+type%3Apr HTTP/1.1
Yet as reported, it seems there redundant search results are being returned.
Dropping the
--json numberand processing the displayed table, you can confirm the duplicates so it isn't something unique to--json:brave/brave-core #25905 Don't store ContentSettingsClient ref as it may be destroryed. CI/skip-ios about 21 days ago brave/brave-core #25905 Don't store ContentSettingsClient ref as it may be destroryed. CI/skip-ios about 21 days ago brave/brave-core #25906 [ubsan] Fix null deref on `OnboardingTest` about 21 days ago brave/brave-core #25906 [ubsan] Fix null deref on `OnboardingTest` about 21 days ago brave/brave-core #25907 Move Inactive tabs setting to Appearance section CI/storybook-url about 17 days ago brave/brave-core #25907 Move Inactive tabs setting to Appearance section CI/storybook-url about 17 days ago brave/brave-core #25908 [ads] Add missing brave_adaptive_captcha dependency about 22 days ago brave/brave-core #25908 [ads] Add missing brave_adaptive_captcha dependency about 22 days ago brave/brave-core #25909 fix(privacy: Shred button spacing in private tabs CI/skip-android, CI/skip-macos-x64, CI/skip-windows-x64, CI/skip-macos-arm64 about 16 days ago brave/brave-core #25909 fix(privacy: Shred button spacing in private tabs CI/skip-android, CI/skip-macos-x64, CI/skip-windows-x64, CI/skip-macos-arm64 about 16 days ago brave/brave-core #25910 Branch migration - master branch CI/skip, CI/run-audit-deps about 21 days ago brave/brave-core #25910 Branch migration - master branch CI/skip, CI/run-audit-deps about 21 days ago brave/brave-core #25911 remove include rules that no longer apply CI/run-audit-deps, feature/web3/wallet, feature/web3/wallet/core, puLL-Merge about 20 days ago brave/brave-core #25911 remove include rules that no longer apply CI/run-audit-deps, feature/web3/wallet, feature/web3/wallet/core, puLL-Merge about 20 days ago brave/brave-core #25912 Hide caption options in media control popup about 21 days ago brave/brave-core #25912 Hide caption options in media control popup about 21 days ago brave/brave-core #25913 [Vertical Tabs]: Add hover effect about 14 days ago brave/brave-core #25913 [Vertical Tabs]: Add hover effect about 14 days ago brave/brave-core #25914 [Brave News]: Don't use TestingProfile in component unittests puLL-Merge about 15 days ago brave/brave-core #25914 [Brave News]: Don't use TestingProfile in component unittests puLL-Merge about 15 days ago brave/brave-core #25917 [ubsan] Avoid early downcast of `BraveTabStrip` about 21 days ago brave/brave-core #25917 [ubsan] Avoid early downcast of `BraveTabStrip` about 21 days ago brave/brave-core #25918 [ubsan] `SidebarContainerView` requires precocious downcast about 21 days ago brave/brave-core #25918 [ubsan] `SidebarContainerView` requires precocious downcast about 21 days ago brave/brave-core #25920 Replace requests to `laptop-updates.brave.com` with requests to `usage-ping.brave.com` and `feedback.brave.com` CI/run-network-audit about 10 days ago brave/brave-core #25920 Replace requests to `laptop-updates.brave.com` with requests to `usage-ping.brave.com` and `feedback.brave.com` CI/run-network-audit about 10 days ago brave/brave-core #25923 Fixed navigation bar color at private tab about 19 days ago brave/brave-core #25923 Fixed navigation bar color at private tab about 19 days ago brave/brave-core #25926 [CodeHealth] Avoid unnecessary uses of `vector` about 20 days ago brave/brave-core #25926 [CodeHealth] Avoid unnecessary uses of `vector` about 20 days ago brave/brave-core #25927 Updated Tor button style. about 9 days ago brave/brave-core #25927 Updated Tor button style. about 9 days ago brave/brave-core #25928 [CodeHealth] Remove empty param lists for lambdas feature/web3/wallet, feature/web3/wallet/core about 20 days ago brave/brave-core #25928 [CodeHealth] Remove empty param lists for lambdas feature/web3/wallet, feature/web3/wallet/core about 20 days ago brave/brave-core #25929 Leo annual subscription on Android CI/skip-macos-x64, CI/skip-ios, CI/skip-windows-x64, puLL-Merge, CI/skip-macos-arm64 about 13 days ago brave/brave-core #25929 Leo annual subscription on Android CI/skip-macos-x64, CI/skip-ios, CI/skip-windows-x64, puLL-Merge, CI/skip-macos-arm64 about 13 days ago brave/brave-core #25931 Internationalization Support for Leo CI/storybook-url, puLL-Merge about 3 days ago brave/brave-core #25931 Internationalization Support for Leo CI/storybook-url, puLL-Merge about 3 days ago brave/brave-core #25934 Upgrade from Chromium 130.0.6723.31 to Chromium 130.0.6723.44 CI/run-network-audit, CI/run-audit-deps about 20 days ago brave/brave-core #25934 Upgrade from Chromium 130.0.6723.31 to Chromium 130.0.6723.44 CI/run-network-audit, CI/run-audit-deps about 20 days ago brave/brave-core #25935 Add secteam reviews for SCT exceptions CI/skip about 20 days ago brave/brave-core #25935 Add secteam reviews for SCT exceptions CI/skip about 20 days ago brave/brave-core #25936 [CrButton]: Add custom size support CI/storybook-url about 20 days ago brave/brave-core #25936 [CrButton]: Add custom size support CI/storybook-url about 20 days ago brave/brave-core #25937 Change default font in desktop settings to system fonts CI/storybook-url about 16 days ago brave/brave-core #25937 Change default font in desktop settings to system fonts CI/storybook-url about 16 days ago brave/brave-core #25938 Update rate dialog UI CI/skip-macos-x64, CI/skip-ios, CI/skip-windows-x64, unused-CI/skip-linux-x64, CI/skip-macos-arm64 about 13 days ago brave/brave-core #25938 Update rate dialog UI CI/skip-macos-x64, CI/skip-ios, CI/skip-windows-x64, unused-CI/skip-linux-x64, CI/skip-macos-arm64 about 13 days ago brave/brave-core #25939 Update laptop icon for sync CI/skip-android, CI/skip-macos-x64, CI/skip-windows-x64, unused-CI/skip-linux-x64, CI/skip-macos-arm64 about 20 days ago brave/brave-core #25939 Update laptop icon for sync CI/skip-android, CI/skip-macos-x64, CI/skip-windows-x64, unused-CI/skip-linux-x64, CI/skip-macos-arm64 about 20 days ago brave/brave-core #25940 [ubsan] Fix `BraveNewsController*` null deref about 20 days ago brave/brave-core #25940 [ubsan] Fix `BraveNewsController*` null deref about 20 days ago brave/brave-core #25943 [DanglingPtr] Fix `AdBlockPrefService` dangling access about 20 days ago brave/brave-core #25943 [DanglingPtr] Fix `AdBlockPrefService` dangling access about 20 days ago brave/brave-core #25944 [DanglingPtr] Add observation to `ViewCounterService` about 20 days ago brave/brave-core #25944 [DanglingPtr] Add observation to `ViewCounterService` about 20 days ago brave/brave-core #25946 [DanglinPtr] Fix various unit test violations about 20 days ago brave/brave-core #25946 [DanglinPtr] Fix various unit test violations about 20 days ago brave/brave-core #25947 Fixed flaky sidebar browser test about 20 days ago brave/brave-core #25947 Fixed flaky sidebar browser test about 20 days ago brave/brave-core #25948 Adjust create wallet account ui CI/storybook-url, feature/web3/wallet, feature/web3/wallet/core, puLL-Merge about 10 days ago brave/brave-core #25948 Adjust create wallet account ui CI/storybook-url, feature/web3/wallet, feature/web3/wallet/core, puLL-Merge about 10 days ago brave/brave-core #25949 Remove Greaselion dependency from Rewards service (v2) CI/storybook-url, CI/run-upstream-tests, needs-security-review, puLL-Merge about 6 days ago brave/brave-core #25949 Remove Greaselion dependency from Rewards service (v2) CI/storybook-url, CI/run-upstream-tests, needs-security-review, puLL-Merge about 6 days ago brave/brave-core #25952 Fix placement of inactive tabs setting about 17 days ago brave/brave-core #25952 Fix placement of inactive tabs setting about 17 days ago brave/brave-core #25953 fix(wallet): Old Activity Route Persisted in Panel CI/storybook-url, feature/web3/wallet about 16 days ago brave/brave-core #25953 fix(wallet): Old Activity Route Persisted in Panel CI/storybook-url, feature/web3/wallet about 16 days ago brave/brave-core #25954 fix(privacy): Fix Shred on app close CI/skip-android, CI/skip-macos-x64, CI/skip-windows-x64, CI/skip-macos-arm64 about 10 days ago brave/brave-core #25954 fix(privacy): Fix Shred on app close CI/skip-android, CI/skip-macos-x64, CI/skip-windows-x64, CI/skip-macos-arm64 about 10 days ago brave/brave-core #25955 [Rewards 3.0] Fix Default AC amount display CI/storybook-url about 17 days ago brave/brave-core #25955 [Rewards 3.0] Fix Default AC amount display CI/storybook-url about 17 days ago brave/brave-core #25956 Content picker follow up CI/storybook-url about 16 days ago brave/brave-core #25956 Content picker follow up CI/storybook-url about 16 days ago brave/brave-core #25957 fix(wallet): Fix tx submission screen gets dismissed automatically after tx is confirmed CI/skip-android, CI/skip-macos-x64, CI/skip-windows-x64, CI/skip-macos-arm64, CI/skip-teamcity about 16 days ago brave/brave-core #25957 fix(wallet): Fix tx submission screen gets dismissed automatically after tx is confirmed CI/skip-android, CI/skip-macos-x64, CI/skip-windows-x64, CI/skip-macos-arm64, CI/skip-teamcity about 16 days ago brave/brave-core #25960 Improves teardown to prevent failing test on Win tests, release-notes/exclude, QA/No about 13 days ago brave/brave-core #25960 Improves teardown to prevent failing test on Win tests, release-notes/exclude, QA/No about 13 days ago brave/brave-core #25961 [CodeHealth][ads] Use `base::ScopedObservation` pt.1 about 17 days ago brave/brave-core #25961 [CodeHealth][ads] Use `base::ScopedObservation` pt.1 about 17 days ago brave/brave-core #25962 [CodeHealth] Use `string_view` with l10n utils about 17 days ago brave/brave-core #25962 [CodeHealth] Use `string_view` with l10n utils about 17 days ago brave/brave-core #25963 Roll perf profiles update (update-perf-profile-2024-10-13-16-16) CI/skip about 17 days ago brave/brave-core #25963 Roll perf profiles update (update-perf-profile-2024-10-13-16-16) CI/skip about 17 days ago brave/brave-core #25964 update adblock-rust to v0.9.2 CI/run-audit-deps about 10 days ago brave/brave-core #25964 update adblock-rust to v0.9.2 CI/run-audit-deps about 10 days ago brave/brave-core #25966 [Toggle]: Fix toggles not toggling CI/storybook-url about 20 hours ago brave/brave-core #25966 [Toggle]: Fix toggles not toggling CI/storybook-url about 20 hours ago brave/brave-core #25971 Fixed device duplication when Google Account cookies are deleted about 13 days ago brave/brave-core #25971 Fixed device duplication when Google Account cookies are deleted about 13 days ago brave/brave-core #25972 [CodeHealth][rewards] Use `base::ScopedObservation` pt.2 puLL-Merge about 16 days ago brave/brave-core #25972 [CodeHealth][rewards] Use `base::ScopedObservation` pt.2 puLL-Merge about 16 days ago brave/brave-core #25975 encapsulate rust json code in cpp feature/web3/wallet, feature/web3/wallet/core, puLL-Merge about 15 days ago brave/brave-core #25975 encapsulate rust json code in cpp feature/web3/wallet, feature/web3/wallet/core, puLL-Merge about 15 days ago brave/brave-core #25977 [ads] Deprecate `ReplaceBraveSchemeWithChromeScheme` about 16 days ago brave/brave-core #25977 [ads] Deprecate `ReplaceBraveSchemeWithChromeScheme` about 16 days ago brave/brave-core #25978 [CodeHealth] Use span of bytes to read arbitrary data puLL-Merge about 14 days ago brave/brave-core #25978 [CodeHealth] Use span of bytes to read arbitrary data puLL-Merge about 14 days ago brave/brave-core #25979 Add IPFS Component build flag, some other review suggestions follow up about 14 days ago brave/brave-core #25979 Add IPFS Component build flag, some other review suggestions follow up about 14 days ago
Following up internally with issue and PR teams to understand why changing the
per_pageon search API calls is resulting in results from the previous page being returned again.In capturing
GH_API=debugto see the actual API endpoints called, I can confirm GitHub API is doing something unexpected:gh api "/search/issues?page=1&per_page=100&q=base%3Amaster+is%3Amerged+merged%3A%3E2024-09-14+repo%3Abrave%2Fbrave-core+type%3Apr" > 256-part-1.json gh api "/search/issues?page=2&per_page=100&q=base%3Amaster+is%3Amerged+merged%3A%3E2024-09-14+repo%3Abrave%2Fbrave-core+type%3Apr" > 256-part-2.json gh api "/search/issues?page=3&per_page=56&q=base%3Amaster+is%3Amerged+merged%3A%3E2024-09-14+repo%3Abrave%2Fbrave-core+type%3Apr" > 256-part-3.json gh api "/search/issues?page=1&per_page=100&q=base%3Amaster+is%3Amerged+merged%3A%3E2024-09-14+repo%3Abrave%2Fbrave-core+type%3Apr" > 275-part-1.json gh api "/search/issues?page=2&per_page=100&q=base%3Amaster+is%3Amerged+merged%3A%3E2024-09-14+repo%3Abrave%2Fbrave-core+type%3Apr" > 275-part-2.json gh api "/search/issues?page=3&per_page=75&q=base%3Amaster+is%3Amerged+merged%3A%3E2024-09-14+repo%3Abrave%2Fbrave-core+type%3Apr" > 275-part-3.json
Note
It appears
gh searchresults are unique when--limitis a denomination of 100.Reacted by Wojciech Knapik1 remaining item
- changed the title
[-]PR search returns duplicate results for some values of --limit[/-][+]`gh search` returns duplicate data when `--limit` greater than 100 but less than the maximum page size[/+]on Nov 1, 2024 @wknapik : I've been able to confirm that the PR Search API is apparently optimized for
--limitin 100 intervals. Unfortunately, that leavesghin a weird situation:-
Does
ghattempt to add intelligence to de-duplicate results and continue pulling more data than specified to try filling up the bucket? -
Do
ghusers specify--limit< 100 or in intervals of 100 until the search API indexing can be properly fixed?
I'm hesitant on the former, however I'm going to bring this up with the other maintainers.
UPDATED: Able to confirm this affects issues in
gh search issues --limit 256 --repo cli/cli | sort | uniq -d | wc -l-
- changed the title
[-]`gh search` returns duplicate data when `--limit` greater than 100 but less than the maximum page size[/-][+]`gh search prs` returns duplicate data when `--limit` greater than 100 but less than the maximum page size[/+]on Nov 1, 2024 A server-side optimization would be transparent to the user.
If a valid request returns and invalid result, I'd call that a bug.I don't believe documenting a bug makes it not a bug.
And shifting the burden of working around a bug to the end user is arguably user-hostile.This is a public API. The CLI is not its only consumer, so while it could try to hide the issue, to the benefit of the user, ultimately, the problem should be resolved at the source.
That's just my 2c. Am I misunderstanding something?
A server-side optimization would be transparent to the user. If a valid request returns and invalid result, I'd call that a bug.
So let me rephrase an earlier statement: the GitHub Search REST APIs aren't the problem here so much as how
gh searchcommands are trying to use it.These endpoints don't have a concept of cursor like GraphQL, so how
gh searchis trying to retrieve partial buckets is the error here as the partial buckets are the wrong coordinates:Lines 61 to 155 in 30066b0
As such, this appears to be an issue with all of the
gh searchcommands, so something for us to fix 👍Thanks to @williammartin for throwing together a proof of concept on how what fixing these commands would entail in trunk...wm/spike-search-dupe-fix; again I think this affects the whole commandset and would need thorough testing:
Lines 133 to 159 in 2c9ed1f
- changed the title
[-]`gh search prs` returns duplicate data when `--limit` greater than 100 but less than the maximum page size[/-][+]`gh search` commands returns duplicate data when `--limit` greater than 100 but not a multiple of 100[/+]on Nov 1, 2024 I see, thanks for the explanation
Proposed Acceptance Criteria
Given I have more than 100 results for a search query
When I rungh search <resource> --limit 110
Then I get 110 unique items (rather than having the last 10 items be duplicated)@andyfeller does that about cover the A/C for this?
Reacted by Andy FellerThis has happened to me every so often when the results tip over 100. Using 200 as the limit works much better so I'll do that until this hopefully gets fixed
