Roadmap#14
Conversation
github-actions
Bot
added
no-issue-linked
WalkthroughThis PR cleans leading bot mentions from message content before building thread titles, conversation context, prompts, and gap logs, and it allows explicit bot mentions to trigger processing outside the configured channel/thread. It also expands the roadmap document with phased plans, operating rules, and reference material. ChangesBot query cleanup
Roadmap planning
Sequence Diagram(s)sequenceDiagram
participant process_message
participant clean_bot_mention
participant _build_conversation_context
participant Ollama
participant _log_gap
process_message->>clean_bot_mention: strip leading bot mention from message.content
process_message->>_build_conversation_context: build cleaned conversation context
_build_conversation_context->>clean_bot_mention: clean historical messages and current_query
process_message->>Ollama: send full_prompt with cleaned_query
Ollama-->>process_message: response or fallback
process_message->>_log_gap: record cleaned_query on fallback or error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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
size/XL
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 `@bot.py`:
- Around line 254-260: The message gate in bot.py is too strict for off-channel
conversations: after the first bot mention, later replies in the thread no
longer pass the configured-channel-or-mention check and never reach the
thread-continuation handling. Update the early return logic around bot_mentioned
and is_in_configured_channel so replies inside a thread spawned from an
off-channel mention are allowed through, using the existing thread-aware path in
the message processing flow.
In `@roadmap.md`:
- Line 15: The task text in roadmap.md misstates the thread behavior by implying
context is preserved per user, which conflicts with _get_or_create_thread in
bot.py that keeps one thread per conversation and does not reuse threads by user
ID. Update the Thread-per-query item to describe that each user interaction
spawns a dedicated thread and that context is continuous only within that
thread, not across separate queries or threads.
- Around line 120-137: The inline mention example should be updated to match the
real mention-stripping logic used by clean_bot_mention in bot.py, since the
current on_message sample only does a simple replace and misses nickname
mentions, repeated leading mentions, and leading whitespace. Adjust the roadmap
example to reference or mirror clean_bot_mention’s regex-based behavior instead
of inline string replacement, so the documented flow stays consistent with the
actual bot.py implementation.
- Around line 44-46: Fix the markdownlint warnings in roadmap.md by updating the
fenced code blocks to use a language tag like text or plaintext in each affected
block, and make sure every fence is surrounded by blank lines as required. Also
adjust the sentence near the first fenced block in the roadmap content from “*
have a look at tutorial” to “* Take a look at the tutorial” for consistent
American English. Use the nearby fenced block sections and list items around the
reported occurrences to locate and apply the same formatting fixes throughout
the document.
- Around line 254-270: The gap log entry in emit_gap_signal should match the
actual _log_gap contract and use timezone-aware timestamps. Replace
datetime.utcnow().isoformat() with datetime.now(timezone.utc).isoformat(), and
update the emitted keys from question/channel to the schema used by _log_gap
(query, reason, and optional thread_id) while keeping repo, timestamp, and
confidence aligned. Ensure the field names and structure match bot.py’s logging
format so downstream consumers continue to work.
- Around line 203-223: The ChromaDB snippet has two issues: it leaks file
handles when reading skill files and the `**/*.md` pattern won’t recurse as
written. Update `index_repo_skills` to read each file via a context manager
instead of calling `open(...).read()`, and fix the directory traversal by either
importing and using `glob(..., recursive=True)` or switching to
`pathlib.Path.rglob` in the `index_repo_skills` flow. Keep the changes localized
to `index_repo_skills` and preserve the existing
`client.get_or_create_collection` / `collection.upsert` logic.
- Around line 539-545: The documentation references in the roadmap list are
broken: the image path and the sibling roadmap links do not resolve to tracked
files. Update the markdown in roadmap.md so the image reference matches the
actual asset name used in the repo, and adjust or remove the entries for Skill
Updater Roadmap and PR Dashboard Roadmap so they point to valid locations.
Locate the affected bullets in the reference/resources section and fix the link
targets without changing the surrounding 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
Run ID: 0e729d7a-fe4d-431c-9509-1b061ab05eeb
⛔ Files ignored due to path filters (1)
image-1.pngis excluded by!**/*.png
📒 Files selected for processing (2)
bot.pyroadmap.md
| bot_mentioned = (client.user in message.mentions) if client.user else False | ||
|
|
||
| # Process the message if: | ||
| # 1. It is in the configured channel (or in a thread inside it), OR | ||
| # 2. The bot is explicitly mentioned/tagged | ||
| if not is_in_configured_channel and not bot_mentioned: | ||
| return |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Allow follow-ups in threads created from off-channel mentions.
This gate only admits messages that are either under DISCORD_CHANNEL_ID_INT or explicitly mention the bot. The first off-channel mention passes, but every later reply in the thread created for that conversation fails both checks and returns before the thread-continuation path runs. That breaks the new cross-channel flow after one turn.
🤖 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 `@bot.py` around lines 254 - 260, The message gate in bot.py is too strict for
off-channel conversations: after the first bot mention, later replies in the
thread no longer pass the configured-channel-or-mention check and never reach
the thread-continuation handling. Update the early return logic around
bot_mentioned and is_in_configured_channel so replies inside a thread spawned
from an off-channel mention are allowed through, using the existing thread-aware
path in the message processing flow.
|
|
||
| **Tasks:** | ||
|
|
||
| - [ ] Thread-per-query — every user interaction spawns a dedicated thread, continuous context preserved per user |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial
Clarify "continuous context preserved per user" to match actual thread semantics.
The claim "continuous context preserved per user" is misleading. bot.py:168-218 (_get_or_create_thread) explicitly states "One thread per conversation — never reuses threads by user ID." Context is preserved per thread, not per user across threads. If a user starts a new query, they get a new thread with fresh context.
Update the task description to:
- Thread-per-query — every user interaction spawns a dedicated thread; context is continuous within that thread only
🤖 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 `@roadmap.md` at line 15, The task text in roadmap.md misstates the thread
behavior by implying context is preserved per user, which conflicts with
_get_or_create_thread in bot.py that keeps one thread per conversation and does
not reuse threads by user ID. Update the Thread-per-query item to describe that
each user interaction spawns a dedicated thread and that context is continuous
only within that thread, not across separate queries or threads.
| ``` | ||
| * have a look at tutorial titled How to create a Discord Chat Bot with Ollama [Python]: https://www.youtube.com/watch?v=f5VTTwbTz28 | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Fix markdownlint warnings for fenced code blocks.
Add language specifications to all fenced code blocks and ensure blank lines surround them:
| Line | Issue | Fix |
|---|---|---|
| 44 | Missing blank line after fence | Add blank line before * at line 45 |
| 57 | Missing language spec | Add text or plaintext after opening fence |
| 88 | Missing language spec | Add text or plaintext |
| 158 | Missing blank lines and language spec | Surround with blank lines; add text |
| 163 | Missing blank lines and language spec | Surround with blank lines; add text |
| 240 | Missing language spec | Add text or plaintext |
| 314 | Missing language spec | Add text or plaintext |
| 363 | Missing language spec | Add text or plaintext |
| 427 | Missing language spec | Add text or plaintext |
Also, line 45: change "* have a look at tutorial" to "* Take a look at the tutorial" for American English convention.
Also applies to: 57-57, 88-88, 158-158, 163-163, 240-240, 314-314, 363-363, 427-427
🧰 Tools
🪛 LanguageTool
[locale-violation] ~45-~45: In American English, “take a look” is more commonly used.
Context: ...consumed by Skill Updater Phase 3 ``` * have a look at tutorial titled How to create a Disc...
(HAVE_A_LOOK)
🪛 markdownlint-cli2 (0.22.1)
[warning] 44-44: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 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 `@roadmap.md` around lines 44 - 46, Fix the markdownlint warnings in roadmap.md
by updating the fenced code blocks to use a language tag like text or plaintext
in each affected block, and make sure every fence is surrounded by blank lines
as required. Also adjust the sentence near the first fenced block in the roadmap
content from “* have a look at tutorial” to “* Take a look at the tutorial” for
consistent American English. Use the nearby fenced block sections and list items
around the reported occurrences to locate and apply the same formatting fixes
throughout the document.
Source: Linters/SAST tools
| ```python | ||
| @bot.event | ||
| async def on_message(message): | ||
| # Mode 1: dedicated #ai-chat channel | ||
| if message.channel.name == "ai-chat" and not message.author.bot: | ||
| await handle_query(message) | ||
|
|
||
| # Mode 2: inline mention in any channel by maintainer | ||
| if bot.user in message.mentions and is_maintainer(message.author): | ||
| # Strip the mention, extract the actual question | ||
| query = message.content.replace(f"<@{bot.user.id}>", "").strip() | ||
| # Check if there's a replied-to message to use as context | ||
| context = "" | ||
| if message.reference: | ||
| ref_msg = await message.channel.fetch_message(message.reference.message_id) | ||
| context = ref_msg.content | ||
| await handle_query(message, query=query, context=context) | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial
Update inline mention example to match actual clean_bot_mention implementation.
The roadmap's naive .replace() approach for mention stripping is inconsistent with the actual clean_bot_mention() in bot.py:39-48, which uses a regex to handle:
- Nickname mentions (
<@!{id}>) - Multiple consecutive leading mentions
- Leading whitespace
The example should reference or mirror the actual helper to avoid copy-paste drift when this phase is implemented.
# Actual implementation from bot.py
def clean_bot_mention(content: str) -> str:
if client.user:
content = re.sub(
rf'^\s*(?:<@!?{client.user.id}>\s*)+',
'',
content,
count=1,
)
return content.strip()🤖 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 `@roadmap.md` around lines 120 - 137, The inline mention example should be
updated to match the real mention-stripping logic used by clean_bot_mention in
bot.py, since the current on_message sample only does a simple replace and
misses nickname mentions, repeated leading mentions, and leading whitespace.
Adjust the roadmap example to reference or mirror clean_bot_mention’s
regex-based behavior instead of inline string replacement, so the documented
flow stays consistent with the actual bot.py implementation.
| ```python | ||
| import chromadb | ||
|
|
||
| client = chromadb.PersistentClient(path="./chroma_skills") | ||
|
|
||
| # Collection per repo — loaded at startup, updated on git pull | ||
| def index_repo_skills(repo_name: str, skill_path: str): | ||
| collection = client.get_or_create_collection(repo_name) | ||
| for skill_file in glob(f"{skill_path}/**/*.md"): | ||
| content = open(skill_file).read() | ||
| collection.upsert( | ||
| ids=[skill_file], | ||
| embeddings=[embed(content)], | ||
| documents=[content] | ||
| ) | ||
|
|
||
| # Query the right collection per channel | ||
| def resolve_with_skills(query: str, repo_name: str) -> str: | ||
| collection = client.get_collection(repo_name) | ||
| results = collection.query(query_embeddings=[embed(query)], n_results=3) | ||
| return build_answer(query, results["documents"][0]) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Fix resource leak and missing recursive in ChromaDB snippet.
Line 212: open(skill_file).read() leaks a file handle. Use a context manager.
Line 211: glob(f"{skill_path}/**/*.md") requires recursive=True for ** to traverse subdirectories; also needs import glob or use pathlib.Path.rglob.
- for skill_file in glob(f"{skill_path}/**/*.md"):
- content = open(skill_file).read()
+ for skill_file in glob(f"{skill_path}/**/*.md", recursive=True):
+ with open(skill_file, encoding="utf-8") as f:
+ content = f.read()📝 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.
| ```python | |
| import chromadb | |
| client = chromadb.PersistentClient(path="./chroma_skills") | |
| # Collection per repo — loaded at startup, updated on git pull | |
| def index_repo_skills(repo_name: str, skill_path: str): | |
| collection = client.get_or_create_collection(repo_name) | |
| for skill_file in glob(f"{skill_path}/**/*.md"): | |
| content = open(skill_file).read() | |
| collection.upsert( | |
| ids=[skill_file], | |
| embeddings=[embed(content)], | |
| documents=[content] | |
| ) | |
| # Query the right collection per channel | |
| def resolve_with_skills(query: str, repo_name: str) -> str: | |
| collection = client.get_collection(repo_name) | |
| results = collection.query(query_embeddings=[embed(query)], n_results=3) | |
| return build_answer(query, results["documents"][0]) |
🤖 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 `@roadmap.md` around lines 203 - 223, The ChromaDB snippet has two issues: it
leaks file handles when reading skill files and the `**/*.md` pattern won’t
recurse as written. Update `index_repo_skills` to read each file via a context
manager instead of calling `open(...).read()`, and fix the directory traversal
by either importing and using `glob(..., recursive=True)` or switching to
`pathlib.Path.rglob` in the `index_repo_skills` flow. Keep the changes localized
to `index_repo_skills` and preserve the existing
`client.get_or_create_collection` / `collection.upsert` logic.
| ```python | ||
| import json | ||
| from datetime import datetime | ||
|
|
||
| GAP_LOG_PATH = "gap_log.json" | ||
|
|
||
| def emit_gap_signal(query: str, channel: str, repo: str, confidence: float): | ||
| entry = { | ||
| "question": query, | ||
| "channel": channel, | ||
| "repo": repo, | ||
| "timestamp": datetime.utcnow().isoformat(), | ||
| "confidence": round(confidence, 3) | ||
| } | ||
| with open(GAP_LOG_PATH, "a") as f: | ||
| json.dump(entry, f) | ||
| f.write("\n") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Replace deprecated datetime.utcnow() and align gap log schema with actual implementation.
Line 265: datetime.utcnow().isoformat() is deprecated since Python 3.12 and produces timezone-naive timestamps. Use datetime.now(timezone.utc).isoformat() as in bot.py:82.
Also, the schema uses "question" and "channel" but actual _log_gap() in bot.py:75-83 uses "query", "reason", and optional "thread_id". Align field names to prevent integration breakage with Skill Updater.
- "timestamp": datetime.utcnow().isoformat(),
+ "timestamp": datetime.now(timezone.utc).isoformat(),And update the entry structure to match the actual contract:
entry = {
- "question": query,
- "channel": channel,
+ "query": query,
+ "reason": reason, # e.g., "llm_fallback"
"repo": repo,
- "timestamp": datetime.utcnow().isoformat(),
+ "timestamp": datetime.now(timezone.utc).isoformat(),
"confidence": round(confidence, 3)
}🤖 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 `@roadmap.md` around lines 254 - 270, The gap log entry in emit_gap_signal
should match the actual _log_gap contract and use timezone-aware timestamps.
Replace datetime.utcnow().isoformat() with
datetime.now(timezone.utc).isoformat(), and update the emitted keys from
question/channel to the schema used by _log_gap (query, reason, and optional
thread_id) while keeping repo, timestamp, and confidence aligned. Ensure the
field names and structure match bot.py’s logging format so downstream consumers
continue to work.
| - [discord.py](https://discordpy.readthedocs.io/) — Discord bot framework | ||
| - [Ollama local models](https://ollama.ai/) — Hermes3, qwen2.5, llama3.1 (all local) | ||
| - [ChromaDB persistent client](https://docs.trychroma.com/reference/py-client) — per-repo skill index (Phase 3) | ||
| - [Skill Updater Roadmap](../skill-updater/ROADMAP.md) — gap signal consumer (Phase 4) | ||
| - [PR Dashboard Roadmap](../pr-dashboard/ROADMAP.md) — shares unified dashboard (Phase 8) | ||
| - [skill-bot-ask-ai](https://github.com/kpj2006/skill-bot-ask-ai-) — Discord REST fetch pattern reference | ||
| - [BERTopic Online Learning](https://maartengr.github.io/BERTopic/getting_started/online/online.html) — clustering reference (used in Skill Updater Phase 1, informs Phase 7 routing) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify referenced files exist
test -f ../skill-updater/ROADMAP.md && echo "skill-updater ROADMAP exists" || echo "MISSING: ../skill-updater/ROADMAP.md"
test -f ../pr-dashboard/ROADMAP.md && echo "pr-dashboard ROADMAP exists" || echo "MISSING: ../pr-dashboard/ROADMAP.md"
test -f public/image-1.png && echo "image-1.png exists" || echo "MISSING: public/image-1.png"Repository: AOSSIE-Org/SkillBot
Length of output: 258
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Top-level files/dirs:\n'
find . -maxdepth 2 \( -name 'ROADMAP.md' -o -path './public/image-1.png' \) | sort
printf '\nAll ROADMAP.md files:\n'
git ls-files '**/ROADMAP.md' | sort
printf '\nAny image-1.png files:\n'
git ls-files | grep -F 'image-1.png' || true
printf '\nroadmap.md references:\n'
rg -n '\.\./skill-updater/ROADMAP\.md|\.\./pr-dashboard/ROADMAP\.md|public/image-1\.png' roadmap.mdRepository: AOSSIE-Org/SkillBot
Length of output: 483
Fix the broken doc links The image reference points to public/image-1.png, but the tracked asset is image-1.png; the two roadmap links also resolve to missing sibling paths. Update the paths or remove the references.
🤖 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 `@roadmap.md` around lines 539 - 545, The documentation references in the
roadmap list are broken: the image path and the sibling roadmap links do not
resolve to tracked files. Update the markdown in roadmap.md so the image
reference matches the actual asset name used in the repo, and adjust or remove
the entries for Skill Updater Roadmap and PR Dashboard Roadmap so they point to
valid locations. Locate the affected bullets in the reference/resources section
and fix the link targets without changing the surrounding content.
1 participant
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
Bug Fixes
Documentation