feat(repo-finder): compile search filters from user preferences#29
Open
imkp1 wants to merge 3 commits into
Open
feat(repo-finder): compile search filters from user preferences#29imkp1 wants to merge 3 commits into
imkp1 wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthroughThe change adds user-owned repository preferences, strict parsing and query compilation, run-scoped finder overrides, deterministic candidate and issue processing, shortlist and triage contract updates, and shell word-splitting regression coverage. ChangesRepository discovery workflow
Shell word-splitting safety
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
actor User
participant Preferences
participant RepoFinder
participant QueryBuilder
participant GitHub
participant IssueSelection
participant Shortlist
User->>Preferences: author filters and notes
RepoFinder->>Preferences: read resolved preferences
RepoFinder->>QueryBuilder: compile and validate queries
QueryBuilder-->>RepoFinder: deterministic search queries
RepoFinder->>GitHub: search repositories and fetch issues
GitHub-->>RepoFinder: projected candidates and timeline data
RepoFinder->>IssueSelection: filter, score, and select issues
IssueSelection-->>RepoFinder: ranked repository records
RepoFinder->>Shortlist: validate and render shortlist JSON
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
imkp1
force-pushed
the
feat/repo-finder-preferences
branch
from
e69837a to
acedffb
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
agents/repo-finder.md (1)
671-724: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winExample
criteria.queriesarray is inconsistent with the stated "one query per topic" rule.The example lists two topic entries (
backend,llm) but only one query string inqueries, contradicting the rule stated just above it ("one search query per entry... Query count is|topics|"). This is illustrative-only, but a reader copying this shape would infer one query covers both topics.📝 Proposed fix
- "queries": ["language:go language:rust topic:backend stars:>2000 archived:false"], + "queries": [ + "language:go language:rust topic:backend stars:>2000 archived:false", + "language:go language:rust topic:llm stars:>5000 archived:false" + ],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agents/repo-finder.md` around lines 671 - 724, Update the example criteria.queries array so it contains one distinct search query for each topic entry in criteria.topics, demonstrating separate queries for backend and llm while preserving the resolved filters and other payload fields.
🧹 Nitpick comments (1)
commands/repo-finder.md (1)
76-86: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAd hoc
sedre-parses the saved preferences file instead of reusing the shared parser.Extracting
languages:/topics:/stars:viased -n 's/^[[:space:]]*languages:[[:space:]]*//p'duplicates logic thatscripts/lib/preferences.sh(sourced elsewhere forprefs_path(), per context) presumably already implements with proper validation/normalization. If the preferences file format gains quoting, comments, or multi-line values later, this sed-based reader silently drifts from the real parser's semantics instead of failing loudly.Do you want me to check whether
scripts/lib/preferences.shalready exposes a per-field accessor this wrapper could call instead?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@commands/repo-finder.md` around lines 76 - 86, Replace the ad hoc sed extraction in the LANGS, TOPICS, and MIN_STARS inheritance block with the shared preferences parser/accessors from scripts/lib/preferences.sh. Reuse its existing per-field lookup and normalization behavior for the saved profile, while preserving the current rule that only unset command-line axes inherit saved values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agents/SHARED_STATE.md`:
- Around line 56-59: The agents/SHARED_STATE.md section at lines 56-59 should
describe global/ as repo-agnostic state, not exclusively plugin-owned or
machine-written state, while preserving the distinction that preferences.md is
user-owned. Update commands/preferences.md lines 6-8 to identify root-owned
preferences.md without claiming all global/ files are machine-written.
In `@commands/preferences.md`:
- Around line 10-11: Document that the default profile uses a >5K-star floor for
topic:llm while the ai-ml, java, python, and devtools searches use >20K. Update
prefs_summary in scripts/lib/preferences.sh and the one-time nudge in
scripts/repo-finder/build_queries.sh to describe these distinct active filters
accurately; apply the documentation change in commands/preferences.md as well.
- Around line 94-104: Update the topic-validation loop around the GitHub query
to normalize each topic before querying: strip any trailing numeric floor such
as “:5000”, skip the reserved “any” entry entirely, and use the normalized topic
in the search and warning message. Keep validation based on total_count for
actual GitHub topics.
In `@commands/repo-finder.md`:
- Around line 24-56: Update the argument tokenization around set -- $ARGUMENTS
to disable pathname expansion before parsing, while preserving the existing
whitespace splitting and case-based flag handling. Restore the shell’s prior
globbing state after tokenization so the rest of the script is unaffected.
In `@scripts/lib/preferences.sh`:
- Around line 135-172: Update the topic parsing flow around topic_list to track
whether any parsed topic is the literal “any” entry, then reject the
configuration with exit 10 when that entry is present and lang_list is empty.
Preserve existing topic validation and default-topic behavior, and add a
regression test covering empty languages with any.
---
Outside diff comments:
In `@agents/repo-finder.md`:
- Around line 671-724: Update the example criteria.queries array so it contains
one distinct search query for each topic entry in criteria.topics, demonstrating
separate queries for backend and llm while preserving the resolved filters and
other payload fields.
---
Nitpick comments:
In `@commands/repo-finder.md`:
- Around line 76-86: Replace the ad hoc sed extraction in the LANGS, TOPICS, and
MIN_STARS inheritance block with the shared preferences parser/accessors from
scripts/lib/preferences.sh. Reuse its existing per-field lookup and
normalization behavior for the saved profile, while preserving the current rule
that only unset command-line axes inherit saved values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 70a05970-b7f4-403f-b7b7-2db17c6a728e
📒 Files selected for processing (11)
agents/SHARED_STATE.mdagents/repo-finder.mdcommands/preferences.mdcommands/repo-finder.mdschemas/repo_shortlist.schema.jsonscripts/lib/preferences.shscripts/repo-finder/build_queries.shtests/fixtures/default_queries.txttests/scripts/test_build_queries.shtests/scripts/test_parse_preferences.shtests/scripts/test_repo_shortlist_schema.sh
imkp1
force-pushed
the
feat/repo-finder-preferences
branch
from
acedffb to
8838f62
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/scripts/test_build_queries.sh (1)
151-153: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClarify that
none.mdis intentionally never created.
$tmpdir/none.mdis referenced repeatedly (Lines 153, 193, 208-209, 216) as a nonexistent path standing in for "no preferences file exists," but nowrite none.mdcall ever creates it, and nothing states this is deliberate. A future contributor could easily "fix" this by adding awrite none <<EOF ... EOFcall, silently changing these tests from exercising the absent-file path to the present-empty-file path.📝 Suggested clarifying comment
+# `none.md` is deliberately never created by `write` — its absence is what +# exercises the "no preferences file" / DEFAULT_PROFILE path below. ALL="$tmpdir/all.txt" { run cross3; run anytopic; run floors bash "$BUILD" --file "$tmpdir/none.md" --no-nudge 2>/dev/null; } > "$ALL"Also applies to: 190-194
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/scripts/test_build_queries.sh` around lines 151 - 153, Add an explicit comment near the first `$tmpdir/none.md` usage in the test setup stating that `none.md` is intentionally not created and represents an absent preferences file. Keep the existing references and test behavior unchanged so these cases continue exercising the nonexistent-file path.schemas/repo_shortlist.schema.json (1)
8-14: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winConsider requiring
criteriato enforce the disclosure guarantee it documents.The
criteriadescription explicitly frames this field as the audit trail for "what actually ran" — but it's not in the top-levelrequiredlist, so a write path that drops it would still validate. Since the whole point of this feature is that a mismatch between intent and actual search is visible in the file, making it optional lets that guarantee silently lapse.♻️ Suggested fix
- "required": ["generated_at", "repos"], + "required": ["generated_at", "repos", "criteria"],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@schemas/repo_shortlist.schema.json` around lines 8 - 14, Update the top-level required list in the schema to include criteria alongside generated_at and repos, preserving the existing criteria property definition and its disclosure semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agents/repo-finder.md`:
- Around line 686-688: Update the example queries associated with the topics
configuration so it contains one query per topic: retain the backend query with
its configured minimum stars and add the corresponding llm query using
stars:>5000, both preserving the existing language and archived filters. Ensure
the example’s criteria.queries count matches the two entries in topics.
---
Nitpick comments:
In `@schemas/repo_shortlist.schema.json`:
- Around line 8-14: Update the top-level required list in the schema to include
criteria alongside generated_at and repos, preserving the existing criteria
property definition and its disclosure semantics.
In `@tests/scripts/test_build_queries.sh`:
- Around line 151-153: Add an explicit comment near the first `$tmpdir/none.md`
usage in the test setup stating that `none.md` is intentionally not created and
represents an absent preferences file. Keep the existing references and test
behavior unchanged so these cases continue exercising the nonexistent-file path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c60ef077-fa57-41b3-bcb6-9f61a4b7e702
📒 Files selected for processing (15)
CHANGELOG.mdCONTRIBUTING.mdREADME.mdagents/SHARED_STATE.mdagents/repo-finder.mdcommands/preferences.mdcommands/repo-finder.mdschemas/repo_shortlist.schema.jsonscripts/lib/preferences.shscripts/repo-finder/build_queries.shskills/superhuman/SKILL.mdtests/fixtures/default_queries.txttests/scripts/test_build_queries.shtests/scripts/test_parse_preferences.shtests/scripts/test_repo_shortlist_schema.sh
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/fixtures/default_queries.txt
- agents/SHARED_STATE.md
- scripts/lib/preferences.sh
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
179-180: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocument
/repo-finderoverrides in the Codex equivalent table.The adjacent mapping still shows only
/repo-finder [N], so Codex users are not told how to pass the new language, topic, andmin-starsper-run overrides documented elsewhere. Add the supported flags and clarify that they are not persisted.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 179 - 180, Update the Codex equivalent table entry for /repo-finder in README.md to document the supported per-run language, topic, and min-stars overrides, and explicitly state that these flags are not persisted. Keep the existing /repo-finder [N] behavior and adjacent mappings unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@README.md`:
- Around line 179-180: Update the Codex equivalent table entry for /repo-finder
in README.md to document the supported per-run language, topic, and min-stars
overrides, and explicitly state that these flags are not persisted. Keep the
existing /repo-finder [N] behavior and adjacent mappings unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8fcdb126-2ccf-4b70-8921-cdcb03171ea0
📒 Files selected for processing (7)
README.mdagents/SHARED_STATE.mdcommands/preferences.mdcommands/repo-finder.mdscripts/lib/preferences.shscripts/repo-finder/build_queries.shtests/scripts/test_parse_preferences.sh
🚧 Files skipped from review as they are similar to previous changes (6)
- agents/SHARED_STATE.md
- scripts/lib/preferences.sh
- commands/repo-finder.md
- tests/scripts/test_parse_preferences.sh
- commands/preferences.md
- scripts/repo-finder/build_queries.sh
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/scripts/test_zsh_word_splitting.sh (1)
41-53: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueInterpolated
$ROOT/$tmpinzsh -c "..."(CWE-78 pattern, low real risk here).Static analysis flags both
zsh -ccalls for splicing variables into the-cscript string. In practice$ROOT(frompwd) and$tmp(frommktemp -d) aren't attacker-controlled in this test, so exploitability is minimal — but a repo path containing a stray quote would still break the test. Passing values as positional args instead of interpolating would harden this if you want to close the gap.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/scripts/test_zsh_word_splitting.sh` around lines 41 - 53, Harden both zsh -c invocations in the validate_json checks by passing ROOT, tmp, and the relevant filenames as positional arguments to the shell script instead of interpolating them into the command string. Update the embedded script to consume those arguments while preserving the existing valid-document acceptance and missing-field rejection assertions.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agents/issue-selector.md`:
- Around line 69-84: Update the NORM tokenization command in the label-filter
block to stop treating hyphens as delimiters by removing `-` from the `tr`
delimiter set. Preserve the existing case arms and newline splitting so
hyphenated labels such as `release-notes` and `copy-edit` remain intact and
match correctly.
---
Nitpick comments:
In `@tests/scripts/test_zsh_word_splitting.sh`:
- Around line 41-53: Harden both zsh -c invocations in the validate_json checks
by passing ROOT, tmp, and the relevant filenames as positional arguments to the
shell script instead of interpolating them into the command string. Update the
embedded script to consume those arguments while preserving the existing
valid-document acceptance and missing-field rejection assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 27f538c4-cdbe-4360-a26a-b898f58ba817
📒 Files selected for processing (7)
agents/builder.mdagents/issue-selector.mdagents/repo-finder.mdcommands/contribution-fleet.mdcommands/repo-finder.mdscripts/lib/state.shtests/scripts/test_zsh_word_splitting.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- commands/repo-finder.md
- agents/repo-finder.md
imkp1
force-pushed
the
feat/repo-finder-preferences
branch
3 times, most recently
from
81e0c57 to
fb86cb3
Compare
Contributor
Author
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agents/repo-finder.md`:
- Around line 494-514: Remove the silent `|| echo '[]'` fallbacks and stderr
suppression from both label-filtered `gh issue list` calls in the `$ISSUES`
fetch block. Make failures from either command propagate under the documented
abort-on-error behavior, while preserving the existing union, deduplication, and
successful empty-result behavior for repositories without matching labels.
In `@commands/repo-finder.md`:
- Around line 131-138: Update the override validation block around
build_queries.sh so failures from build_queries.sh, including malformed
preferences, cannot be masked by the sed pipeline. Enable pipefail for this
command or separate the commands while preserving the existing Invalid override
cleanup and exit behavior.
- Around line 109-119: Scope the saved-value extraction in the LANGS, TOPICS,
and MIN_STARS inheritance blocks to the ## Filters section only, stopping before
the next section heading. Preserve the existing key-specific extraction and
fallback behavior while preventing matching lines in ## Notes or other sections
from being used as filters.
In `@README.md`:
- Around line 182-189: Update the default-profile description in the Usage
section to distinguish the topic:llm threshold: state that llm repositories use
the >5K-star cutoff while AI/ML, Java, Python, and dev-tools use >20K, matching
DEFAULT_PROFILE in build_queries.sh and the corresponding preferences
documentation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7f533af3-a00d-4593-9f30-d0f581d39193
📒 Files selected for processing (27)
CHANGELOG.mdCONTRIBUTING.mdREADME.mdagents/SHARED_STATE.mdagents/builder.mdagents/issue-selector.mdagents/repo-finder.mdcommands/contribution-fleet.mdcommands/preferences.mdcommands/repo-finder.mdschemas/repo_shortlist.schema.jsonscripts/lib/preferences.shscripts/lib/state.shscripts/orchestrator/reputation_gate.shscripts/orchestrator/triage_filter.shscripts/repo-finder/build_queries.shscripts/repo-finder/curated_list_filter.shskills/superhuman/SKILL.mdtests/fixtures/default_queries.txttests/scripts/test_build_queries.shtests/scripts/test_curated_list_filter.shtests/scripts/test_parse_preferences.shtests/scripts/test_repo_finder_args.shtests/scripts/test_repo_shortlist_schema.shtests/scripts/test_reputation_gate.shtests/scripts/test_triage_filter.shtests/scripts/test_zsh_word_splitting.sh
🚧 Files skipped from review as they are similar to previous changes (9)
- tests/fixtures/default_queries.txt
- scripts/lib/state.sh
- scripts/lib/preferences.sh
- agents/SHARED_STATE.md
- agents/builder.md
- commands/contribution-fleet.md
- agents/issue-selector.md
- tests/scripts/test_build_queries.sh
- CONTRIBUTING.md
imkp1
force-pushed
the
feat/repo-finder-preferences
branch
from
fb86cb3 to
ac6523d
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
scripts/repo-finder/build_queries.sh (1)
78-89: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUnquoted
for tok in $qrisks pathname expansion on glob metacharacters.Word splitting on
$qis intentional here, but it also enables pathname expansion. If a user'slanguage:/topic:value contains a glob character (*,?,[), this loop can silently expand it against files in the working directory instead of validating the literal token — defeating the "loud failure" guarantee this whole function exists to provide. None of the visible charset guards (boolean/paren/negation/ceiling checks) block glob metacharacters specifically.🛡️ Proposed fix: disable globbing for the tokenizing loop
+ set -f for tok in $q; do case "$tok" in *:*) key="${tok%%:*}" ;; *) die "bare word in query: '$tok' in '$q' (free text is not a filter)" ;; esac case "$key" in language) langs=$((langs + 1)) ;; topic) topics=$((topics + 1)) ;; stars|archived) ;; *) die "unknown qualifier key '$key' in '$q' (allowed: language, topic, stars, archived)" ;; esac done + set +fPlease confirm whether
scripts/lib/preferences.shalready restricts language/topic charset to exclude glob metacharacters — if so this is just defensive hardening; if not, it's a real gap.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/repo-finder/build_queries.sh` around lines 78 - 89, Update the tokenizing loop around the `for tok in $q` statement to disable pathname expansion while preserving intentional word splitting, then restore the previous globbing setting after the loop. Keep the existing qualifier validation and `die` behavior unchanged, and verify whether `scripts/lib/preferences.sh` already excludes glob metacharacters from language/topic values; if not, retain this hardening as the required validation safeguard.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@commands/preferences.md`:
- Line 160: Add the text language identifier to the fenced code block containing
the interactive compile-output example in preferences.md, preserving its
existing contents.
In `@commands/repo-finder.md`:
- Line 145: Update the Agent example code fence in repo-finder.md to include the
text language identifier, satisfying MD040 while preserving the example content.
- Around line 104-108: Replace predictable temporary paths with securely
allocated mktemp files: in commands/repo-finder.md lines 104-108 allocate
OVERRIDE exclusively, and in lines 122-129 write only to it and remove it via
cleanup traps on every exit path; in commands/preferences.md lines 117-123
securely allocate the live-check profile and query output, and in lines 188-191
securely create the persisted-file temporary path before atomic rename. Preserve
existing SAVED, OVERRIDE, and preference-processing behavior.
In `@README.md`:
- Line 186: Update the three newly added fenced code blocks in README.md to
include explicit language identifiers: use markdown, text, and bash
respectively, preserving each block’s existing contents.
In `@scripts/lib/preferences.sh`:
- Around line 135-189: Update the reserved-topic guard in the preferences parser
around topic_list construction and validation so it recognizes any topic entry
whose name is “any”, regardless of an optional star floor such as any:5000.
Reject it when lang_list is empty before build_queries emits output, preserving
the existing bare-any behavior; add a regression case in the parser tests
covering any:5000 with empty languages.
In `@tests/scripts/test_zsh_word_splitting.sh`:
- Around line 62-64: Broaden the unsafe shell-expansion pattern assigned to hits
in test_zsh_word_splitting.sh to detect braced and unbraced forms in for loops,
set --, and command substitutions, while allowing variable and function names
containing uppercase letters and digits. Preserve the existing exclusions and
failure behavior, or replace the regex check with an equivalent ShellCheck
SC2086-based validation.
- Around line 33-34: Update the zsh invocations around validate_json to keep the
-c script body constant and pass the root, schema, and valid-file paths as
positional arguments, referenced via $1, $2, and $3 inside the script. Apply the
same change to both affected test cases, preserving their existing validation
behavior while preventing path contents from being interpreted as shell syntax.
---
Nitpick comments:
In `@scripts/repo-finder/build_queries.sh`:
- Around line 78-89: Update the tokenizing loop around the `for tok in $q`
statement to disable pathname expansion while preserving intentional word
splitting, then restore the previous globbing setting after the loop. Keep the
existing qualifier validation and `die` behavior unchanged, and verify whether
`scripts/lib/preferences.sh` already excludes glob metacharacters from
language/topic values; if not, retain this hardening as the required validation
safeguard.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 617bd757-1e93-4377-84f6-562841433ffa
📒 Files selected for processing (27)
CHANGELOG.mdCONTRIBUTING.mdREADME.mdagents/SHARED_STATE.mdagents/builder.mdagents/issue-selector.mdagents/repo-finder.mdcommands/contribution-fleet.mdcommands/preferences.mdcommands/repo-finder.mdschemas/repo_shortlist.schema.jsonscripts/lib/preferences.shscripts/lib/state.shscripts/orchestrator/reputation_gate.shscripts/orchestrator/triage_filter.shscripts/repo-finder/build_queries.shscripts/repo-finder/curated_list_filter.shskills/superhuman/SKILL.mdtests/fixtures/default_queries.txttests/scripts/test_build_queries.shtests/scripts/test_curated_list_filter.shtests/scripts/test_parse_preferences.shtests/scripts/test_repo_finder_args.shtests/scripts/test_repo_shortlist_schema.shtests/scripts/test_reputation_gate.shtests/scripts/test_triage_filter.shtests/scripts/test_zsh_word_splitting.sh
🚧 Files skipped from review as they are similar to previous changes (13)
- tests/scripts/test_repo_shortlist_schema.sh
- CONTRIBUTING.md
- agents/issue-selector.md
- tests/fixtures/default_queries.txt
- scripts/repo-finder/curated_list_filter.sh
- scripts/lib/state.sh
- scripts/orchestrator/triage_filter.sh
- commands/contribution-fleet.md
- agents/builder.md
- schemas/repo_shortlist.schema.json
- tests/scripts/test_triage_filter.sh
- tests/scripts/test_build_queries.sh
- agents/repo-finder.md
/repo-finder searched a hardcoded profile. It now compiles GitHub search qualifiers from ~/.superhuman/preferences.md, authored by /preferences. The file has two halves that bind differently. `## Filters` is mechanical: build_queries.sh compiles languages, topics and a star floor 1:1 into qualifiers, and rejects a profile it cannot compile rather than degrading to an unfiltered scan. `## Notes` is advisory prose: it applies at tie-breaks and issue selection, discloses itself in the shortlist's `notes` field when it does, and may never move a numeric score. Languages OR inside one query and cost nothing; topics AND, so each costs one query — hence a 20-topic cap and no cap on languages. `stars` is a floor with no ceiling: a ceiling guesses at what the responsiveness scorer already measures per repo, and silently deletes the largest active repos. Filters are read through parse_preferences, never by grepping the file: a `sed` over the whole document also matches `## Notes`, so a profile whose Filters block omits an axis would inherit prose into a hard filter. Per-run overrides compile first and print second, so build_queries.sh's exit 10 is not swallowed by the pipeline that formats its output. Also in this change: - curated_list_filter.sh: hard-skip curated lists and courses. Lists answer issues fast and merge outside PRs readily, so they score well on 60% of the rubric while offering nothing to contribute. Pedagogy topics are strong signals: `language=null` cannot fire on a profile that pins `languages:`, since every candidate then arrives from a query carrying `language:go language:python`. - triage_filter.sh: Stage-A issue triage, with KEEP rows carrying the issue payload the Step-4 rubric scores and may not re-fetch. - Stage-B timelines chunk at 20 GraphQL aliases; more exceed the node limit, which errors rather than truncating. - The issue fetch no longer falls back to `[]` on error. An unused label already returns `[]` with exit 0, so the fallback only ever masked real failures. - state.sh: warn when jsonschema is absent — the fallback checks top-level keys only, so a shortlist with null rows would validate clean.
imkp1
force-pushed
the
feat/repo-finder-preferences
branch
from
ac6523d to
9fb88eb
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@commands/preferences.md`:
- Around line 95-145: Update the live query loop around the gh api invocation to
distinguish command failures from legitimate zero counts. Capture the API result
with explicit failure handling, emit an API-specific error, and exit or
otherwise stop the check when the request fails; only apply the existing
zero-result and ranking warnings when a successful response yields a numeric
total_count.
In `@tests/scripts/test_parse_preferences.sh`:
- Line 8: Update the CLAUDE_PLUGIN_ROOT assignment in test_parse_preferences.sh
to use the same CDPATH= and cd -- root-resolution pattern as the sibling test
scripts, preserving the existing dirname and path traversal behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0c3a413d-6efb-462b-8b17-14550cfa422b
📒 Files selected for processing (27)
CHANGELOG.mdCONTRIBUTING.mdREADME.mdagents/SHARED_STATE.mdagents/builder.mdagents/issue-selector.mdagents/repo-finder.mdcommands/contribution-fleet.mdcommands/preferences.mdcommands/repo-finder.mdschemas/repo_shortlist.schema.jsonscripts/lib/preferences.shscripts/lib/state.shscripts/orchestrator/reputation_gate.shscripts/orchestrator/triage_filter.shscripts/repo-finder/build_queries.shscripts/repo-finder/curated_list_filter.shskills/superhuman/SKILL.mdtests/fixtures/default_queries.txttests/scripts/test_build_queries.shtests/scripts/test_curated_list_filter.shtests/scripts/test_parse_preferences.shtests/scripts/test_repo_finder_args.shtests/scripts/test_repo_shortlist_schema.shtests/scripts/test_reputation_gate.shtests/scripts/test_triage_filter.shtests/scripts/test_zsh_word_splitting.sh
🚧 Files skipped from review as they are similar to previous changes (17)
- tests/fixtures/default_queries.txt
- scripts/repo-finder/curated_list_filter.sh
- agents/issue-selector.md
- scripts/lib/state.sh
- tests/scripts/test_repo_shortlist_schema.sh
- agents/SHARED_STATE.md
- tests/scripts/test_triage_filter.sh
- schemas/repo_shortlist.schema.json
- CONTRIBUTING.md
- agents/builder.md
- commands/contribution-fleet.md
- scripts/orchestrator/triage_filter.sh
- scripts/lib/preferences.sh
- commands/repo-finder.md
- README.md
- tests/scripts/test_build_queries.sh
- agents/repo-finder.md
Distinguish gh api failures (rate limit / auth / network) from genuine zero-result queries in the preferences live check, and add the CDPATH=/-- cd guard to test_parse_preferences.sh to match sibling test scripts.
…ll survivors with fill-to-N gaurav0107#3: install.sh now installs the jsonschema Python package (escalating past PEP 668), so validate_json runs the real schema validator on user machines instead of silently degrading to a top-level-key check that cannot see inside repos[]/scores/best_issue. gaurav0107#2: Step 1 stops truncating to a fixed top-50 before scoring; Step 3 scores every survivor in deterministic order; Step 4 walks the score-ranked list filling until N repos are kept or survivors are exhausted, replacing the hardcoded top-15 slice. A repo shortfall now means the field is thin, not that the scan stopped early. Coverage {candidates, survivors, scored, returned} is recorded in the shortlist (new optional schema field) and rendered, so a partial scan is visible in the artifact.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@install.sh`:
- Around line 109-112: Update the jsonschema installation fallback in the
install.sh dependency-install flow to remove the unconditional
--break-system-packages attempt. Prefer installing into a project-owned virtual
environment, or only invoke that flag when an explicit user opt-in is enabled,
while preserving the existing non-breaking installation paths and failure
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ec810a18-e1d7-4a89-9666-764d14fc1d56
📒 Files selected for processing (4)
agents/repo-finder.mdcommands/repo-finder.mdinstall.shschemas/repo_shortlist.schema.json
🚧 Files skipped from review as they are similar to previous changes (3)
- schemas/repo_shortlist.schema.json
- commands/repo-finder.md
- agents/repo-finder.md
Comment on lines
+109
to
+112
| python3 -m pip install --quiet jsonschema 2>/dev/null \ | ||
| || python3 -m pip install --quiet --user jsonschema 2>/dev/null \ | ||
| || python3 -m pip install --quiet --break-system-packages jsonschema 2>/dev/null \ | ||
| || true |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate install.sh and inspect the relevant section with context.
git ls-files | rg '(^|/)install\.sh$'
# Show a compact map of the file first, then the surrounding lines.
FILE="$(git ls-files | rg '(^|/)install\.sh$' | head -n 1)"
echo "FILE=$FILE"
wc -l "$FILE"
sed -n '1,220p' "$FILE" | cat -n | sed -n '80,140p'Repository: gaurav0107/superhuman
Length of output: 3459
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files | rg '(^|/)install\.sh$'Repository: gaurav0107/superhuman
Length of output: 170
Avoid automatic --break-system-packages fallback in install.sh:109-112.
This can alter a distro-managed Python environment once the normal install paths fail. Gate it behind explicit user opt-in or use a project-owned virtual environment instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@install.sh` around lines 109 - 112, Update the jsonschema installation
fallback in the install.sh dependency-install flow to remove the unconditional
--break-system-packages attempt. Prefer installing into a project-owned virtual
environment, or only invoke that flag when an explicit user opt-in is enabled,
while preserving the existing non-breaking installation paths and failure
behavior.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #28 — merge that first. Its four commits ride along in this diff until it lands. The files this PR owns are
scripts/lib/preferences.sh,scripts/repo-finder/build_queries.sh,commands/preferences.md, the two new tests, and the preference-related edits toagents/repo-finder.md,commands/repo-finder.md,agents/SHARED_STATE.md, andschemas/repo_shortlist.schema.json.The problem
/repo-finder's filters are constants.agents/repo-finder.mdStep 1 hardcodeslanguage:javaandlanguage:python, and no command accepts a preference —/repo-findertakes an integer N and rejects anything else. Ask for Go, get Python. There is no path by which "I want Go" changes a query.The design
~/.superhuman/preferences.md, owned by the user. It sits at the root of~/.superhuman/, not underglobal/, because everything underglobal/is machine-written state the plugin owns. This file is theirs.The two halves bind differently, and being explicit about which is which is the point:
## Filtersis mechanical. Parsed byscripts/lib/preferences.sh, compiled 1:1 into GitHub search qualifiers byscripts/repo-finder/build_queries.sh. GitHub topics are the tag taxonomy, so topics pass straight through — nothing invented, nothing to maintain.## Notesis advisory. No code can filter on "focused rather than sprawling". The agent applies it at tie-breaks and issue selection only, never to a numeric score, and must disclose in the shortlist'snotesfield when it did. Treating prose as a filter is how the current version ended up ignoring its ownstars:>20000.Query shape follows the qualifier asymmetry
language:language:go language:rust topic:cli→ first page is Go ∪ Rust, nothing outsidetopic:topic:cli topic:terminal→ 21 results vs 109/44 aloneSo all languages collapse into one query and each topic gets its own. Query count is
|topics|, not|languages| × |topics|— adding a language costs zero queries. Emitting twotopic:qualifiers on one line silently returns only repos tagged with every topic, often none, exit 0, no error;build_queries.shrefuses to emit one.Everything fails loudly
Each of these is reported by GitHub as success, which is why they are enforced at emit time rather than trusted:
langauge:go) is parsed as free text →total_count: 0, exit 0ORor a parenthesized qualifier-OR →total_count: 0, exit 0topic:→ a quietly intersected, usually empty setMalformed preferences exit 10 rather than degrade to an unfiltered scan. A
stars: 2000-60000range is a loud error, not a quietly honored ceiling.No star ceiling
mainhas never had one — all nine current queries are lower-bound only. A ceiling is a guess about what the scorer already measures per repo (median response time, median merge time, external-merge ratio, pre-approval templates), and it fails asymmetrically:ollama(~176K),ragflow(~85K) andnetdata(~80K) sit above any plausible cap, so it would silently delete the categoryDEFAULT_PROFILEexists to find.The default path is a pure refactor
No preferences file means
DEFAULT_PROFILE, whose five queries are byte-identical to the ones the agent carried inline (golden-file test, no network)./contribute,/contribute-loopand/contribution-fleeton a fresh machine cannot regress, because the queries are literally the same bytes. It deliberately keeps union semantics — generating it from the strictlanguage AND topicshape would drop a Rust ML repo and an untagged Java repo from a fresh machine's candidate set, a composition change for users who never asked for one. The strict shape is opt-in.One nudge line points at
/preferences, shown once per install via apreferences_nudgedkey in the existinglast_version.json.Scoring
Category Preference Order and the
3ecategory bonus are deleted, not replaced. With filtering server-side every candidate matches by construction, so a bonus for matching is a constant added to every row and ranks nothing. Its 10% goes to signals that discriminate: responsiveness 30→35%, outside-contributor track 20→25%.Testing
tests/scripts/test_parse_preferences.shandtest_build_queries.sh, both network-free and bash 3.2-clean, so they gate on ubuntu and macOS. They pin: exactly onetopic:per query; query count == topic count; adding a language adds zero queries; the qualifier-key allowlist; no boolean operators (repeatedlanguage:is not one); no negation; no catch-all; floor-only stars;anycompiles to a topic-less query; omittedtopics:≡topics: any; per-entry floors override the global one; a## Notessentence changes no query; the nudge fires once; and byte-equality of the default profile.No
yq:scripts/profiler/parse_workflows.shalready depends on it and it is not installed on a stock macOS box. Since CI installs it, a second dependency would go green in CI and dead on the user's laptop. awk + jq only.Verified live: a
go, rust × cli, llm:5000, anyprofile returns only Go and Rust repos, one query per topic, with the 5K floor applied tollmalone.Summary by CodeRabbit
New Features
/preferencesvia~/.superhuman/preferences.md(languages/topics/min stars) with advisory## Notes./repo-finderwith per-run override flags (non-persistent) and deterministic repo/issue selection behavior.Documentation
Bug Fixes
Tests / Chores