add skills - #21
Open
kpj2006 wants to merge 23 commits into
Open
Conversation
git-subtree-dir: repos/SocialShareButton git-subtree-split: 231c06f347bcae0e9dc39bf32cdbd4983fcaf063
git-subtree-dir: repos/Template-Repo-Main git-subtree-split: ba634f3439b6d2b4d8457b526f601e9b3d434ad5
git-subtree-dir: repos/OrgExplorer git-subtree-split: 6e5e6278478b4fa40a2a6bed1156b05ef190e027
…ude bot routing tests
…text, improve logging, and add new GSoC Proposal Assistant project files.
…eads and append new entries to gap logs
Contributor
|
Warning Review limit reached
Next review available in: 51 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
WalkthroughThe bot now routes messages to repository-specific context, clarifies unmapped requests, and records routing gaps. Repository metadata, synchronization automation, project context documents, security guidance, checklist records, and routing validation scripts are added or updated. ChangesRepository-aware bot flow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Contributor
participant DiscordBot
participant RepoRouter
participant Ollama
participant RepoContext
Contributor->>DiscordBot: submit project question
DiscordBot->>RepoRouter: detect repository
RepoRouter->>Ollama: classify ambiguous query
Ollama-->>RepoRouter: repository or none
RepoRouter->>RepoContext: load repository context
RepoContext-->>DiscordBot: context text
DiscordBot-->>Contributor: contextual response or clarification
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
no-issue-linked
Contributor
|
Member
Author
|
@CodeRabbit review |
Contributor
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 62
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
repos/GSoC-Proposal-Assistant/.agent/instructions/format-guide.md (1)
81-88: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winLabel the PR contribution code fence and separate it with blank lines.
This fence triggers
MD031andMD040.🤖 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 `@repos/GSoC-Proposal-Assistant/.agent/instructions/format-guide.md` around lines 81 - 88, Update the PR examples in the format guide by adding the appropriate language identifier to the contribution code fence and placing blank lines before and after the fenced block, resolving the MD031 and MD040 violations without changing its example content.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 @.clinerules:
- Line 9: Update the project examples in the clarification prompt within
.clinerules to match the valid keys in repo_metadata.py: SocialShareButton,
Template-Repo-Main, OrgExplorer, and GSoC-Proposal-Assistant; remove
PullRequestDashboard so every suggested project can be resolved by
detect_repo_by_keywords.
In @.github/workflows/sync-subtrees.yml:
- Around line 8-9: Update the workflow permissions and action references: set
workflow-level contents permission to read, grant contents write only in the
sync job’s permissions, and pin actions/checkout@v4 and actions/setup-python@v5
to their full commit SHAs. Preserve the existing sync job behavior while scoping
write access to jobs.sync.
- Around line 30-35: Update the “Push updates” workflow step after running
scripts/update_subtrees.py to stage generated repos/ changes, create a commit
when changes exist, and then push the commit. Remove the “|| echo” fallback so
authentication, network, and non-fast-forward push failures propagate; handle
the no-changes case without attempting an empty commit.
- Around line 3-6: Update the workflow trigger configuration under on to
serialize scheduled and workflow_dispatch sync runs, using a shared concurrency
group for this workflow and cancel-in-progress disabled so an active run
completes before the next begins.
- Around line 20-31: Add a dependency-installation step after the “Set up
Python” step and before “Sync subtrees,” using pip to install the requirements
declared in requirements.txt. Keep the existing Python setup, Git identity
configuration, and scripts/update_subtrees.py invocation unchanged.
In `@BestPracticesChecklist.md`:
- Around line 124-125: Revise the vulnerability_report_response checklist item
so it does not claim six-month historical compliance based solely on the 48-hour
self-certification note. Either add anonymized, dated evidence demonstrating
reports met the 14-day requirement, or change the checkbox wording to state only
the 48-hour response commitment.
- Around line 155-156: Update the tests_are_added checklist evidence to
reference a recent pull request or CI result that visibly demonstrates test
changes, replacing the current Danger workflow link; alternatively, uncheck the
item until valid evidence is available.
- Around line 224-231: Update the dynamic_analysis checklist entries and their
associated notes to distinguish ordinary functional test-suite validation from
security-focused dynamic analysis such as fuzzing or web scanning. Remove the
Python-language justification, and ensure the status and scoring consistently
reflect whether qualifying dynamic-analysis tooling is actually used, including
the related entries around dynamic_analysis_fixed and the referenced AI/LLM
evidence.
- Around line 92-93: Remove the checked release_notes_vulns criterion and its
OSV scanner workflow evidence link in BestPracticesChecklist.md. Mark the
criterion unmet or N/A until evidence directly demonstrates that release notes
identify every publicly known vulnerability fixed in the release.
- Around line 28-34: Update the checklist score generation logic in the
workflow’s table replacement to preserve blank lines around the generated table,
replacing the current table.strip()-based handling. Ensure the rendered
BestPracticesChecklist.md table is separated from the following horizontal rule
so markdownlint MD058 passes.
- Around line 81-82: Update the version_semver checklist note to remove the
stale hard-coded v0.1.0 value or replace it with the current VERSION value,
1.0.0, while preserving the existing SemVer evidence.
In `@bot.py`:
- Around line 291-295: The INFO logging in bot.py around the message-receipt
logging block exposes display_name, username, and verbatim cleaned_query; demote
those fields to DEBUG or redact/truncate them, while retaining only
channel/thread and message IDs at INFO. In gap_log.json lines 31-180, remove the
committed runtime entries, add gap_log.json to .gitignore, and replace the
tracked file with an empty example file as requested.
- Around line 287-288: Update the bot mention handling around cleaned_query to
detect blank or whitespace-only input before calling detect_repo_by_keywords,
classify_repo_with_llm, or recording gap entries. Immediately invoke
send_clarification_request and return for empty queries, while preserving the
existing routing flow for non-empty cleaned_query values.
- Around line 322-325: Ensure every Ollama invocation is serialized through
ollama_lock, including both classify_repo_with_llm calls and the clarification
generate_ollama_response call, not only the mapped-repo path. Prefer a shared
helper that acquires and releases ollama_lock around each invocation, then route
all affected call sites through it while preserving their existing arguments and
results.
- Around line 289-298: Extract the repository-detection cascade from
process_message into an async _resolve_repo(message, cleaned_query,
available_repos) returning tuple[str | None, str | None]. Move the routing
stages covering the current detection logic into this helper, preserve their
ordering and fallback behavior, and have process_message call it before
continuing with thread handling, context loading, and response dispatch.
- Around line 418-424: Update the response-prefix condition after
generate_ollama_response so the repository-context header is added only when
used_fallback is false and response_text does not already start with “According
to the”. Leave fallback and error responses unprefixed.
- Around line 496-507: Update the exception handler in the thread-history lookup
within the backlog replay flow to log the caught failure at an appropriate debug
or warning level before continuing. Preserve the existing control flow, but do
not silently swallow errors, and ensure failures in get_thread, fetch_channel,
or thread.history remain observable.
In `@repo_metadata.py`:
- Line 21: Update the keyword string in the metadata configuration from
“starters template” to “starter template” so natural phrasing matches correctly.
- Line 3: The repository metadata URLs for SocialShareButton, OrgExplorer, and
GSoC-Proposal-Assistant currently target a personal fork and feature branches.
Update these entries in the repository metadata configuration to the
corresponding canonical AOSSIE-Org repositories, preserving the existing
repository names and URL structure.
- Around line 16-23: Update the keyword lists used by detect_repo_by_keywords to
remove generic single-word terms such as “repo”, “main”, “template”, and
“dashboard”. Replace them with distinctive multi-word phrases for
Template-Repo-Main, and add explicit “orgexplorer” or “org explorer” terms for
OrgExplorer so unrelated contributor and dashboard queries are not misrouted.
In `@repo_router.py`:
- Around line 53-63: Add an early allowlist guard at the start of get_repo_path:
return None when repo_name is not a key in REPO_METADATA, before constructing or
checking any candidate paths. Preserve the existing candidate lookup for
recognized repository names.
- Around line 92-101: Remove the unnecessary f-string prefixes from the static
prompt fragments in the prompt construction. In the classifier failure handling
around the existing exception logger, replace logger.error with logger.exception
so the traceback is retained while preserving the current failure behavior.
- Around line 127-133: Move the blocking load_repo_context execution off the
event loop by updating its await in process_message to run load_repo_context via
asyncio.to_thread with mapped_repo and cleaned_query arguments. Preserve the
existing returned context and downstream processing behavior.
- Around line 234-235: Refine the readme_keywords used by the query-routing
condition so broad substring matches such as “how” and “help” cannot trigger
README inclusion for unrelated queries. Remove or replace them with whole-word
or clearly README-specific intent terms, while preserving the existing README
routing for explicit overview, description, and similar requests.
- Around line 76-81: Update the keyword iteration in get_repo_from_thread_name
and the nearby repository-matching logic to access metadata with
details.get("keywords", []) instead of direct details["keywords"] indexing,
preserving the existing keyword normalization and matching behavior.
- Around line 219-231: Update the local skill discovery loop in the dynamic
intent detection block to use the simplified rglob pattern "SKILL.md" instead of
the redundant "**/SKILL.md" pattern, preserving the existing sorting, filtering,
and file-reading behavior.
- Around line 138-218: Cap every .agent Markdown file read in the
context-loading flow, including operational data, architecture, core, and
instruction files, using a shared per-file helper such as _read_capped
consistent with the existing README handling. Track the accumulated context size
and stop appending files once MAX_TOTAL_CHARS is reached, while preserving the
current loading and error-logging behavior within the repo context assembly
function.
In `@repos/GSoC-Proposal-Assistant/.agent/core/architecture.md`:
- Around line 31-40: Update the reviewer workflow to consistently cover all
seven checklist blocks: in
repos/GSoC-Proposal-Assistant/.agent/core/architecture.md lines 31-40, change
the six-block and Blocks 0–5 references to seven blocks and Blocks 0–6; in
repos/GSoC-Proposal-Assistant/.agent/instructions/checklist.md lines 63-66,
preserve and explicitly include Block 6 in the documented workflow.
- Line 7: Fix Markdown lint violations across the documentation: in
repos/GSoC-Proposal-Assistant/.agent/core/architecture.md#L7-L7 label the
architecture fence as mermaid; add blank lines after headings in
repos/GSoC-Proposal-Assistant/.agent/info/operational-data.md#L3-L9,
repos/GSoC-Proposal-Assistant/.agent/instructions/bad-patterns.md#L7-L8,
`#L14-L15`, `#L21-L22`, and `#L28-L29`, and repos/OrgExplorer/AGENTS.md#L16-L17,
`#L19-L20`, `#L22-L23`, and `#L26-L27`; label the user-flow fence as text in
repos/OrgExplorer/README.md#L265-L277 and the conceptual-flow fence as text in
repos/SocialShareButton/.agent/core/architecture.md#L21-L27; add blank lines
around the command blocks in
repos/SocialShareButton/.agent/instructions/deployment.md#L18-L24 and `#L32-L42`.
In `@repos/GSoC-Proposal-Assistant/.agent/instructions/checklist.md`:
- Around line 70-74: Update the “Final Verdict” rules in the checklist to
explicitly classify proposals with zero ❌ results and 3–4 ⚠️ warnings, assigning
them to an appropriate verdict such as “Needs Work.”
- Around line 55-59: Update BLOCK 5 in the checklist to explicitly require both
novel Y and Z extensions beyond the base idea, with each extension mapped to an
AOSSIE theme and technically justified. Replace the single-extension requirement
while preserving the existing validation criteria for theme alignment and
justification.
In `@repos/GSoC-Proposal-Assistant/.agent/instructions/format-guide.md`:
- Around line 11-16: Update the fenced code block containing the format-guide
header example to specify the text language, using a text-labeled fence while
preserving the example contents unchanged.
- Around line 85-86: Update the “Pending PRs” example in the contribution-format
guidance to include an explicit mentor attribution alongside the PR title and
URL. Ensure the example demonstrates that every pending contribution claim names
the mentor and links the relevant active Pull Request.
In `@repos/GSoC-Proposal-Assistant/README.md`:
- Around line 23-35: Label the flow diagram code fence in the README with an
appropriate language identifier, such as text, to satisfy Markdown lint rule
MD040 while preserving the diagram content.
- Around line 5-8: Use one reviewed, immutable source for all AI instructions:
update the provider URLs in README.md and the reference links in SKILL.md to
point to a pinned reviewed commit or repository-owned synced files instead of
the mutable kpj2006/proposal/main branch. Apply the change at README.md lines
5-8 and SKILL.md lines 148-156, preserving the existing provider prompts and
referenced documents.
In `@repos/GSoC-Proposal-Assistant/references/bad-patterns.md`:
- Around line 125-129: Fix Markdown fence formatting at
repos/GSoC-Proposal-Assistant/references/bad-patterns.md:125-129 by adding blank
lines around the fence and using the markdown language identifier; apply the
same surrounding-blank-line and language-label updates at
repos/GSoC-Proposal-Assistant/references/format-guide.md:11-16 (text) and
:148-156 (markdown), add the text identifier at
repos/OrgExplorer/.agent/core/architecture.md:22-30, and add required blank
lines around the indented fence at
repos/OrgExplorer/.agent/instructions/deployment.md:14-18.
In `@repos/GSoC-Proposal-Assistant/references/checklist.md`:
- Around line 60-65: Update the “Future Expansion / Innovation” checklist in
BLOCK 5 so it explicitly requires both named Extension Y and Extension Z,
replacing the “at least one extension” criterion. Retain the existing
requirements for AOSSIE theme mapping, technical justification, and independent
consideration.
- Around line 103-107: Update the “Final Verdict” criteria to explicitly
classify proposals with 3–4 ⚠️ items, including that range under “Needs Work”
while preserving the existing ❌ and architecture-diagram conditions.
In `@repos/GSoC-Proposal-Assistant/SKILL.md`:
- Around line 86-115: The skill’s references must use the same canonical
instruction source as the runtime loader. Update the guidance around
“format-guide.md”, “checklist.md”, and “bad-patterns.md” to resolve through the
`.agent/instructions` directory loaded by `repo_router.py`, or add explicit
synchronization validation if separate copies remain; preserve the existing
reviewer behavior while preventing inconsistent rule evaluation.
- Line 65: Update the drafting gate in SKILL.md to require intake item 3
alongside items 1, 2, 4, 5, and 6. Ensure drafting cannot proceed until the
applicant provides a Proof of Concept repository link or explicitly commits to
creating one.
In `@repos/OrgExplorer/.agent/info/operational-data.md`:
- Line 15: Update the “OrgExplorer project channel” link in the operational-data
instructions to use the actual project-specific Discord channel URL containing
its channel ID; do not leave it pointing to the server-level channels page.
In `@repos/OrgExplorer/.agent/instructions/testing.md`:
- Around line 20-21: Update the PAT testing instructions around PATModal to
require a disposable, least-privilege test token and explicitly prohibit using
production credentials or exposing tokens in screenshots, recordings, logs, or
bug reports.
In `@repos/OrgExplorer/README.md`:
- Around line 1-7: Add the top-level “OrgExplorer” H1 heading near the beginning
of the README, and update the org-explorer-logo.png image in the centered logo
markup with descriptive alt text (or explicitly mark it decorative if
appropriate).
In `@repos/SocialShareButton/.agent/instructions/setup.md`:
- Around line 30-34: Add blank lines before and after the nested npm start code
fence in the setup instructions, while preserving the existing commands and
surrounding guidance.
In `@repos/SocialShareButton/AGENTS.md`:
- Around line 18-32: Update the headings in AGENTS.md for Onboarding & Setup,
Writing & Modifying Code, Testing & Verification, and Pull Requests & Community
so each is surrounded by blank lines, satisfying the MD022 heading-spacing
requirement while preserving the existing links and content.
In `@repos/SocialShareButton/README.md`:
- Around line 114-187: Update the README headings in each framework-specific
section to avoid duplicate “Step 1” and “Step 2” headings triggering MD024.
Scope each heading with its framework name, such as “Create React App — Step 1”,
while preserving the existing instructions and section structure.
- Around line 147-161: Make all CDN examples in
repos/SocialShareButton/README.md use a consistent readiness pattern: in the
React useEffect block at lines 147-161, retry initialization until
window.SocialShareButton is available and retain cleanup; in the Preact example
at lines 463-477, add the same retry behavior and cleanup on unmount; in the
generic example at lines 417-426, guard construction and add a delayed-load
fallback. Reuse the existing initialization and destruction behavior across all
three examples.
- Around line 673-683: Clarify the quick-start guidance alongside the “React
Wrapper Component (Optional)” section: the prohibition should target duplicate
ad hoc wrapper files, while explicitly allowing the documented reusable
SocialShareButton wrapper to be copied when needed. Keep the existing wrapper
usage instructions unchanged.
In `@repos/Template-Repo-Main/README.md`:
- Around line 80-100: Update the Tech Stack section in README.md by adding blank
lines before and after each subsection heading: Frontend, Backend, AI/ML (if
applicable), and Blockchain (if applicable), while preserving the existing list
content.
- Line 192: Replace the malformed replacement character in the “Getting Started”
heading with the intended valid emoji, preserving the heading text and
formatting.
- Around line 5-8: Add descriptive alt text to the TODO project logo img element
in the centered logo block, matching the existing accessibility pattern used by
the AOSSIE logo and resolving the MD045 warning.
- Around line 140-169: Label both fenced code blocks under “Architecture
Diagram” and “User Flow” with an appropriate language identifier, such as text,
so they no longer use unlabeled fences and satisfy MD040.
- Around line 50-51: Update the Best Practices badge URL in the README so its
raw GitHub repository path uses the repository placeholder instead of the
literal AOSSIE-Org/repo path. Preserve the existing checklist-status.json
endpoint, query, label, and badge formatting.
In `@scripts/test_bot_routing.py`:
- Line 43: Renumber the duplicate test-section headings in the script: update
the second “--- 4. ---” heading to “--- 5. ---” and the trailing section heading
to “--- 6. ---”, preserving all test logic.
- Around line 15-18: Convert test_all into a real pytest test with assertions
for deterministic routing behavior, including thread-name mapping and keyword
detection, and mark it with pytest.mark.asyncio if it remains asynchronous;
remove print-only validation. Alternatively, rename the script and entry
function to routing_smoke_check.py and main() so pytest does not collect it.
- Line 63: Before the Ollama-dependent sections that call
generate_ollama_response, probe http://localhost:11434/ for reachability and
skip sections 4 and 5 when the service is unavailable. Reuse the availability
result for both calls, while preserving the existing response and fallback
behavior when Ollama is reachable.
- Around line 6-7: Guard the stdout TextIOWrapper reassignment near the UTF-8
output setup with sys.platform == "win32", leaving stdout unchanged on other
platforms to preserve redirection and capture behavior.
- Line 13: Update scripts/test_bot_routing.py to avoid importing bot at module
load time, preventing bot.py environment setup and Discord client initialization
during routing-only tests. Move generate_ollama_response and load_skill_context
to a shared module and import them there, or defer importing bot until the
Ollama-specific test paths while keeping repo_router tests independent.
In `@scripts/update_subtrees.py`:
- Around line 67-80: Update sync_repo_context() to track HTTP 4xx/5xx responses
and transport exceptions separately from expected 404s, surface failures for
required context files with a non-success result, and avoid reporting a
successful sync when any required file fails. Define and apply a deletion policy
for local files absent from the remote KNOWN_CONTEXT_FILES set so stale copies
are not reused.
- Around line 18-33: Update the KNOWN_CONTEXT_FILES list in
scripts/update_subtrees.py to include .agent/core/code-mapping.md,
.agent/core/edge-cases.md, and .agent/core/examples.md alongside the existing
architecture.md entry, ensuring all core context guidance supported by
repo_router.py is synchronized.
- Around line 37-45: Update parse_github_url to capture the entire tree suffix,
including slashes, so branch refs such as foo/bar are preserved. Ensure
repository metadata provides explicit non-default refs, or resolve each bare
URL’s actual default branch instead of assuming main; retain main only when it
is the confirmed default.
In `@SECURITY.md`:
- Around line 24-34: Update the “What to Include in Your Report” section in
SECURITY.md to explicitly instruct reporters to redact secrets and personal data
from proof-of-concept code, logs, screenshots, environment variables, tokens,
message content, and other submitted artifacts before uploading them.
---
Outside diff comments:
In `@repos/GSoC-Proposal-Assistant/.agent/instructions/format-guide.md`:
- Around line 81-88: Update the PR examples in the format guide by adding the
appropriate language identifier to the contribution code fence and placing blank
lines before and after the fenced block, resolving the MD031 and MD040
violations without changing its example content.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3d466f34-3628-4ee8-a074-02208a772eae
📒 Files selected for processing (37)
.clinerules.github/workflows/sync-subtrees.ymlBestPracticesChecklist.mdSECURITY.mdbot.pychecklist-status.jsongap_log.jsonrepo_metadata.pyrepo_router.pyrepos/GSoC-Proposal-Assistant/.agent/core/architecture.mdrepos/GSoC-Proposal-Assistant/.agent/info/operational-data.mdrepos/GSoC-Proposal-Assistant/.agent/instructions/bad-patterns.mdrepos/GSoC-Proposal-Assistant/.agent/instructions/checklist.mdrepos/GSoC-Proposal-Assistant/.agent/instructions/format-guide.mdrepos/GSoC-Proposal-Assistant/AGENTS.mdrepos/GSoC-Proposal-Assistant/README.mdrepos/GSoC-Proposal-Assistant/SKILL.mdrepos/GSoC-Proposal-Assistant/references/bad-patterns.mdrepos/GSoC-Proposal-Assistant/references/checklist.mdrepos/GSoC-Proposal-Assistant/references/format-guide.mdrepos/OrgExplorer/.agent/core/architecture.mdrepos/OrgExplorer/.agent/info/operational-data.mdrepos/OrgExplorer/.agent/instructions/deployment.mdrepos/OrgExplorer/.agent/instructions/setup.mdrepos/OrgExplorer/.agent/instructions/testing.mdrepos/OrgExplorer/AGENTS.mdrepos/OrgExplorer/README.mdrepos/SocialShareButton/.agent/core/architecture.mdrepos/SocialShareButton/.agent/info/operational-data.mdrepos/SocialShareButton/.agent/instructions/deployment.mdrepos/SocialShareButton/.agent/instructions/setup.mdrepos/SocialShareButton/.agent/instructions/testing.mdrepos/SocialShareButton/AGENTS.mdrepos/SocialShareButton/README.mdrepos/Template-Repo-Main/README.mdscripts/test_bot_routing.pyscripts/update_subtrees.py
| detected = detect_repo_by_keywords(q, repos) | ||
| print(f"Query: '{q}' -> Detected: {detected}") | ||
|
|
||
| print("\n--- 4. Dynamic Query Context Loading Test ---") |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Duplicate section number.
Both sections are labelled --- 4. ---; the second should be 5. (and the trailing one 6.).
Also applies to: 54-54
🤖 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/test_bot_routing.py` at line 43, Renumber the duplicate test-section
headings in the script: update the second “--- 4. ---” heading to “--- 5. ---”
and the trailing section heading to “--- 6. ---”, preserving all test logic.
Comment on lines
+124
to
+125
| - [x] 🔴 **vulnerability_report_response** — Initial response to any vulnerability report received in the last 6 months was within 14 days. | ||
| - *Self-certification note:* Initial response time commitment for vulnerability reports is within 48 hours. |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Distinguish response policy from historical evidence.
A 48-hour response commitment proves policy, not that reports received during the last six months met the 14-day requirement. Provide anonymized dated evidence or revise the checkbox to describe only the commitment.
🤖 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 `@BestPracticesChecklist.md` around lines 124 - 125, Revise the
vulnerability_report_response checklist item so it does not claim six-month
historical compliance based solely on the 48-hour self-certification note.
Either add anonymized, dated evidence demonstrating reports met the 14-day
requirement, or change the checkbox wording to state only the 48-hour response
commitment.
Comment on lines
+234
to
+235
| readme_keywords = ["readme", "overview", "about", "description", "what is", "how", "help"] | ||
| if not q or any(kw in q for kw in readme_keywords): |
Contributor
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
readme_keywords are broad enough to fire on nearly every query.
"how" and "help" match substrings of most contributor questions (and "how" also hits words like "show"), so the README excerpt is appended almost unconditionally, inflating the prompt. Restrict to whole-word/intent-specific terms.
🤖 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 `@repo_router.py` around lines 234 - 235, Refine the readme_keywords used by
the query-routing condition so broad substring matches such as “how” and “help”
cannot trigger README inclusion for unrelated queries. Remove or replace them
with whole-word or clearly README-specific intent terms, while preserving the
existing README routing for explicit overview, description, and similar
requests.
| sys.path.insert(0, str(bot_root)) | ||
|
|
||
| from repo_router import get_available_repos, load_repo_context, detect_repo_by_keywords, get_repo_from_thread_name | ||
| from bot import generate_ollama_response, load_skill_context |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect bot.py module-level side effects that run on import.
fd -t f 'bot.py' --max-depth 2 --exec sed -n '1,60p' {}Repository: AOSSIE-Org/SkillBot
Length of output: 1669
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the target script and relevant bot.py sections without executing repo code.
fd -t f 'test_bot_routing.py|bot.py' --max-depth 4 --exec sh -c '
echo "===== {} ====="
wc -l "{}"
ast-grep outline "{}" --view compact || true
'
echo "===== scripts/test_bot_routing.py ====="
sed -n '1,220p' scripts/test_bot_routing.pyRepository: AOSSIE-Org/SkillBot
Length of output: 3806
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import ast
bot_path = Path("bot.py")
script_path = Path("scripts/test_bot_routing.py")
boot = ast.parse(bot_path.read_text()).body
imports = [n for n in boot if isinstance(n, ast.ImportFrom) and n.module == "dotenv"]
has_load_dotenv = any(isinstance(n.body[0], ast.Call) and getattr(n.body[0].func, "id", None) == "load_dotenv" for n in imports)
has_client_global = any(isinstance(e, ast.Assign) and isinstance(e.value, ast.Call) and
isinstance(e.value.func, ast.Name) and e.value.func.id == "discord.Client"
for e in boot if isinstance(e, (ast.Assign, ast.AnnAssign)))
print({"bot_uses_load_dotenv_at_module_top": has_load_dotenv})
repo_router_line = next((i+1 for i, n in enumerate(boot) if isinstance(n, ast.ImportFrom) and n.module == "repo_router"), None)
dotenv_line = next((i+1 for i, n in enumerate(boot) if isinstance(n, ast.ImportFrom) and n.module == "dotenv"), None)
print({"repo_router_import_line": repo_router_line, "load_dotenv_import_line": dotenv_line, "client_assignment": has_client_global})
script = ast.parse(script_path.read_text()).body
bot_import = next((i+1 for i, n in enumerate(script) if isinstance(n, ast.ImportFrom) and n.module == "bot"), None)
print({"bot_import_line": bot_import})
PY
echo "===== bot.py imports/constants/client initialization ====="
sed -n '1,80p' bot.pyRepository: AOSSIE-Org/SkillBot
Length of output: 329
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import ast
def expr_name(n):
if isinstance(n, ast.Name):
return n.id
if isinstance(n, ast.Attribute):
return n.attr
return None
bot_path = Path("bot.py")
script_path = Path("scripts/test_bot_routing.py")
boot = ast.parse(bot_path.read_text()).body
imports = [n for n in boot if isinstance(n, ast.ImportFrom) and n.module == "dotenv"]
has_load_dotenv_call = any(isinstance(n, ast.ImportFrom) and n.module == "dotenv" and
any(isinstance(stmt, ast.Call) and expr_name(stmt.func) == "load_dotenv" for stmt in n.body)
for n in imports)
repo_router_line = next((i+1 for i, n in enumerate(boot) if isinstance(n, ast.ImportFrom) and n.module == "repo_router"), None)
dotenv_line = next((i+1 for i, n in enumerate(boot) if isinstance(n, ast.ImportFrom) and n.module == "dotenv"), None)
has_discord_client = any(isinstance(e, ast.Assign) and isinstance(e.value, ast.Call) and
isinstance(e.value.func, ast.Attribute) and expr_name(e.value.func.value) == "discord"
and expr_name(e.value.func) == "Client"
for e in boot if isinstance(e, ast.Assign))
print({"bot_uses_load_dotenv_at_module_top": has_load_dotenv_call})
print({"repo_router_import_line": repo_router_line, "load_dotenv_import_line": dotenv_line, "client_assignment": has_discord_client})
script = ast.parse(script_path.read_text()).body
bot_import = next((i+1 for i, n in enumerate(script) if isinstance(n, ast.ImportFrom) and n.module == "bot"), None)
print({"bot_import_line": bot_import})
PY
echo "===== bot.py imports/constants/client initialization ====="
sed -n '1,80p' bot.pyRepository: AOSSIE-Org/SkillBot
Length of output: 331
Avoid importing bot during routing-only tests.
scripts/test_bot_routing.py imports generate_ollama_response / load_skill_context from bot, which runs bot.py’s module-level environment setup and Discord client initialization before the script can exercise repo_router. Move those helpers into a shared module and import them directly, or defer the bot import until the Ollama response sections.
🤖 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/test_bot_routing.py` at line 13, Update scripts/test_bot_routing.py
to avoid importing bot at module load time, preventing bot.py environment setup
and Discord client initialization during routing-only tests. Move
generate_ollama_response and load_skill_context to a shared module and import
them there, or defer importing bot until the Ollama-specific test paths while
keeping repo_router tests independent.
| print(f"User Query: '{sample_query}'") | ||
| print("Sending request to Ollama...") | ||
|
|
||
| response, fallback = await generate_ollama_response(sample_query, repo_context) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Ollama-dependent sections have no availability guard.
Sections 4 and 5 call generate_ollama_response, which retries MAX_RETRIES times with 2s sleeps and a 120s timeout per attempt before returning its fallback string. Without a reachability pre-check this script hangs for minutes on a machine with no Ollama running. Probe http://localhost:11434/ first and skip these sections when unreachable.
Also applies to: 75-75
🤖 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/test_bot_routing.py` at line 63, Before the Ollama-dependent sections
that call generate_ollama_response, probe http://localhost:11434/ for
reachability and skip sections 4 and 5 when the service is unavailable. Reuse
the availability result for both calls, while preserving the existing response
and fallback behavior when Ollama is reachable.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
scripts/update_subtrees.py (2)
55-121: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffFunction complexity flagged by static analysis.
Ruff reports 16 branches (>12) and 52 statements (>50) for
sync_repo_context. Consider extracting the per-file download loop and the stale-file deletion pass into separate helper functions to improve readability and testability.🤖 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/update_subtrees.py` around lines 55 - 121, Reduce the complexity of sync_repo_context by extracting the per-file download and required-file/error tracking logic into a helper, and extracting the stale local-file deletion pass into another helper. Keep sync_repo_context responsible for setup, orchestration, final required-file validation, and success reporting, while preserving the existing download, cleanup, logging, and return behavior.Source: Linters/SAST tools
77-114: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDeletion policy wipes locally-synced files on transient errors, not just confirmed upstream removal.
downloaded_filesonly gets an entry on HTTP 200. Any non-404 HTTP error or transport exception (Lines 92-101) also fails to add the file, and the deletion loop (Lines 103-114) then deletes that file locally becauserel_p not in downloaded_files— even though the remote file wasn't actually removed, just temporarily unreachable. Deletion also runs before thefailed_requiredcheck, so it happens even when the overall sync is later reported as failed. A single flaky fetch can therefore permanently drop a previously-good context file until the next successful sync of that specific file.🐛 Proposed fix: only delete files confirmed removed upstream (404), not files that merely failed to (re)download this run
headers = {"User-Agent": "SkillBot-Context-Sync"} downloaded_files = set() + confirmed_removed = set() failed_required = [] http_errors = 0 transport_errors = 0 for rel_file in KNOWN_CONTEXT_FILES: raw_url = f"https://raw.githubusercontent.com/{owner}/{repo}/{ref}/{rel_file}" try: res = client.get(raw_url, headers=headers) if res.status_code == 200: ... downloaded_files.add(rel_file) elif res.status_code == 404: logger.debug(f"File not found on remote (404): {rel_file} for {repo_name}") + confirmed_removed.add(rel_file) if rel_file in REQUIRED_CONTEXT_FILES: ... else: ... except Exception as e: ... - # Deletion policy: remove local files absent from KNOWN_CONTEXT_FILES or not downloaded in current sync + # Deletion policy: remove local files no longer tracked, or confirmed removed upstream (404). + # Do NOT delete files that merely failed to download this run due to a transient HTTP/transport error. known_set = set(KNOWN_CONTEXT_FILES) if target_dir.exists(): for p in list(target_dir.rglob("*")): if p.is_file(): rel_p = p.relative_to(target_dir).as_posix() - if rel_p not in known_set or rel_p not in downloaded_files: + if rel_p not in known_set or rel_p in confirmed_removed: try: p.unlink()🤖 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/update_subtrees.py` around lines 77 - 114, Update the deletion policy in the sync loop so local files are removed only when they are absent from KNOWN_CONTEXT_FILES or their fetch returned a confirmed 404. Do not use downloaded_files to determine deletion, and preserve local files after transient HTTP errors or transport exceptions; ensure deletion does not occur for files whose fetch failed without confirming upstream removal.
🤖 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 @.github/workflows/checklist-score.yml:
- Around line 130-136: Update the regex in the checklist content replacement
around formatted_table and new_content so its end lookahead targets a standalone
line containing only the horizontal rule, rather than the first --- within the
table separator row. Preserve the existing auto-update anchor and regenerate the
checklist to remove the duplicated malformed block.
In @.github/workflows/sync-subtrees.yml:
- Around line 25-27: Replace the literal <verified-full-commit-sha> placeholder
in the actions/setup-python step of the sync workflow with the full, verified
commit SHA for the intended setup-python v5 release, preserving the existing
python-version configuration.
---
Outside diff comments:
In `@scripts/update_subtrees.py`:
- Around line 55-121: Reduce the complexity of sync_repo_context by extracting
the per-file download and required-file/error tracking logic into a helper, and
extracting the stale local-file deletion pass into another helper. Keep
sync_repo_context responsible for setup, orchestration, final required-file
validation, and success reporting, while preserving the existing download,
cleanup, logging, and return behavior.
- Around line 77-114: Update the deletion policy in the sync loop so local files
are removed only when they are absent from KNOWN_CONTEXT_FILES or their fetch
returned a confirmed 404. Do not use downloaded_files to determine deletion, and
preserve local files after transient HTTP errors or transport exceptions; ensure
deletion does not occur for files whose fetch failed without confirming upstream
removal.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: aadf0967-31d9-4500-b1e1-1689618be7ae
📒 Files selected for processing (9)
.github/workflows/checklist-score.yml.github/workflows/sync-subtrees.ymlBestPracticesChecklist.mdchecklist-status.jsonrepo_metadata.pyrepo_router.pyscripts/routing_smoke_check.pyscripts/test_bot_routing.pyscripts/update_subtrees.py
Comment on lines
+130
to
136
| formatted_table = f"\n\n{table.strip()}\n\n" | ||
| new_content = re.sub( | ||
| r'(?<=<!-- Auto-updated by checklist-score\.yml workflow — do not edit manually -->\n).*?(?=\n---)', | ||
| table.strip(), | ||
| r'(?<=<!-- Auto-updated by checklist-score\.yml workflow — do not edit manually -->).*?(?=---)', | ||
| formatted_table, | ||
| content, | ||
| flags=re.DOTALL | ||
| ) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Anchor the replacement at the standalone horizontal rule.
The (?=---) lookahead matches the first --- inside the table separator row, so each workflow run leaves a malformed duplicate block in BestPracticesChecklist.md (currently Lines 37-44). Match a line containing only ---, then regenerate the checklist.
Suggested fix
new_content = re.sub(
- r'(?<=<!-- Auto-updated by checklist-score\.yml workflow — do not edit manually -->).*?(?=---)',
+ r'(?<=<!-- Auto-updated by checklist-score\.yml workflow — do not edit manually -->).*?(?=^---\s*$)',
formatted_table,
content,
- flags=re.DOTALL
+ flags=re.DOTALL | re.MULTILINE
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| formatted_table = f"\n\n{table.strip()}\n\n" | |
| new_content = re.sub( | |
| r'(?<=<!-- Auto-updated by checklist-score\.yml workflow — do not edit manually -->\n).*?(?=\n---)', | |
| table.strip(), | |
| r'(?<=<!-- Auto-updated by checklist-score\.yml workflow — do not edit manually -->).*?(?=---)', | |
| formatted_table, | |
| content, | |
| flags=re.DOTALL | |
| ) | |
| new_content = re.sub( | |
| r'(?<=<!-- Auto-updated by checklist-score\.yml workflow — do not edit manually -->).*?(?=^---\s*$)', | |
| formatted_table, | |
| content, | |
| flags=re.DOTALL | re.MULTILINE | |
| ) |
🤖 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 @.github/workflows/checklist-score.yml around lines 130 - 136, Update the
regex in the checklist content replacement around formatted_table and
new_content so its end lookahead targets a standalone line containing only the
horizontal rule, rather than the first --- within the table separator row.
Preserve the existing auto-update anchor and regenerate the checklist to remove
the duplicated malformed block.
Comment on lines
+25
to
+27
| uses: actions/setup-python@<verified-full-commit-sha> | ||
| with: | ||
| python-version: '3.11' |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
git ls-remote https://github.com/actions/setup-python refs/tags/v5*Repository: AOSSIE-Org/SkillBot
Length of output: 675
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Workflow line 20-30:"
sed -n '1,45p' .github/workflows/sync-subtrees.yml | cat -n
echo
echo "Action reference values in workflow:"
rg -n "uses:\s*actions/setup-python@" .github/workflows/sync-subtrees.yml || true
echo
echo "All literal placeholder occurrences:"
rg -n "<verified-full-commit-sha>|<actual-40-char-sha-for-v5\.x>" .github/workflows/sync-subtrees.yml || true
echo
echo "Resolve actions/setup-python refs/tags/v5 object to commit:"
python3 - <<'PY'
import urllib.request, json
raw = urllib.request.urlopen("https://api.github.com/repos/actions/setup-python/git/refs/tags/v5", timeout=20).read()
ref = json.loads(raw)
print("ref_object_type:", ref.get("object", {}).get("type"))
print("ref_object_sha:", ref.get("object", {}).get("sha"))
sha = ref["object"]["sha"]
obj = json.loads(urllib.request.urlopen(f"https://api.github.com/repos/actions/setup-python/git/tags/{sha}", timeout=20).read())
print("tag_object_type:", obj["object"].get("type"))
print("tag_object_commit_sha:", obj["object"].get("sha"))
PYRepository: AOSSIE-Org/SkillBot
Length of output: 4426
Replace the placeholder SHA before merging.
.github/workflows/sync-subtrees.yml:25 references actions/setup-python@<verified-full-commit-sha>, which is literal placeholder text and will make GitHub Actions fail to resolve the action. Pin it to the full commit SHA for the intended release, for example resolving refs/tags/v5 via git ls-remote https://github.com/actions/setup-python HEAD or the tag object.
🧰 Tools
🪛 zizmor (1.26.1)
[error] 25-25: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/sync-subtrees.yml around lines 25 - 27, Replace the
literal <verified-full-commit-sha> placeholder in the actions/setup-python step
of the sync workflow with the full, verified commit SHA for the intended
setup-python v5 release, preserving the existing python-version configuration.
Source: Linters/SAST tools
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.
Addressed Issues:
Fixes #(issue number)
Screenshots/Recordings:
Additional Notes:
Checklist
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact.
Summary by CodeRabbit
New Features
Improvements