Skip to content

Add initial Gitcord engine with GitHub/Discord ingestion and audit-first dry-run workflow#1

Open
shubham5080 wants to merge 61 commits intoAOSSIE-Org:mainfrom
shubham5080:main
Open

Add initial Gitcord engine with GitHub/Discord ingestion and audit-first dry-run workflow#1
shubham5080 wants to merge 61 commits intoAOSSIE-Org:mainfrom
shubham5080:main

Conversation

@shubham5080
Copy link

@shubham5080 shubham5080 commented Jan 20, 2026

Summary

  • integrate the Gitcord engine source under src/ghdcbot
  • add config example, docs, tests, and packaging
  • update README with structure, workflow, and safe startup

Testing

  • pytest

Summary by CodeRabbit

  • New Features

    • Installable CLI and Discord bot with identity linking, mentor/issue workflows, deterministic planning proposing GitHub assignments and Discord role changes, dry‑run/observer safety modes, audit/activity exports (JSON/MD), and configurable notifications.
  • Documentation

    • README rewritten; comprehensive docs added (config, demo, architecture, verification, testing, guides).
  • Storage & Packaging

    • Local SQLite-backed storage and packaging with an installable console script.
  • Integrations

    • GitHub and Discord integrations with robust rate‑limit and permission handling.
  • Tests

    • Large test suite covering config, ingestion, scoring, planning, writers, gating, and exports.
  • Chores

    • .gitignore updated for Python/environment artifacts.

…rst dry-run workflow

- Permission-aware GitHub and Discord readers
- Deterministic planning with explicit identity mapping
- Audit reports (JSON + Markdown)
- Strict mutation gating (dry-run default)
@coderabbitai
Copy link

coderabbitai bot commented Jan 20, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a complete offline-first CLI automation engine: configuration models and loader, GitHub/Discord adapters and writers, SQLite storage, orchestrator with scoring/planning/reporting, identity-linking and Discord bot, JSON logging, plugin loader, packaging/entrypoint, extensive docs, and a large pytest suite.

Changes

Cohort / File(s) Summary
Project & Packaging
pyproject.toml, .gitignore, .env.example
Add pyproject with console-script entrypoint; replace LaTeX ignores with Python/env-focused ignores; add example env file.
Top-level docs & examples
README.md, config/example.yaml, docs/*, TESTS.md, GITCORD_DEEP_DIVE_REPORT.md, ORG_AGNOSTIC_AUDIT_REPORT.md
Introduce extensive user/developer docs, demo, config example, verification/audit reports, and test guidance.
CLI & Entrypoints
src/ghdcbot/cli.py, src/ghdcbot/__main__.py, src/ghdcbot/__init__.py
Add CLI wiring, build_orchestrator, multiple subcommands (run-once, export-audit, identity commands), and python -m launcher.
Configuration
src/ghdcbot/config/...
src/ghdcbot/config/models.py, src/ghdcbot/config/loader.py
Add Pydantic BotConfig models and loader with env-var expansion, validation, caching, and ConfigError handling.
Core contracts & models
src/ghdcbot/core/...
models.py,interfaces.py,errors.py,modes.py
Add frozen dataclasses for domain models, Protocol interfaces for adapters/strategies, custom errors, RunMode enum and MutationPolicy utilities.
Engine & workflows
src/ghdcbot/engine/...
orchestrator.py,planning.py,scoring.py,reporting.py,...
Implement Orchestrator (run_once/close), deterministic planning, WeightedScoreStrategy, reporting (audit.json/md, activity.md), merge-role rules, metrics, identity-linking, issue/mentor flows, PR context and many helpers.
GitHub adapters
src/ghdcbot/adapters/github/...
rest.py,writer.py,identity.py,__init__.py
Add GitHub REST ingestion with pagination/rate-limit/permission handling, repo filtering with user fallback, identity verification (bio/gist), and writer with dedupe and gating.
Discord adapters
src/ghdcbot/adapters/discord/...
api.py,writer.py,__init__.py
Add Discord API client with rate-limit awareness, member/role listing, and a DiscordPlanWriter that resolves role IDs and applies add/remove actions respecting MutationPolicy and modes.
Storage adapter
src/ghdcbot/adapters/storage/...
sqlite.py,__init__.py
Add SqliteStorage with schema init, contribution persistence, summaries, score upserts, cursors, identity claims/links, issue requests, audit_events.jsonl, and UTC normalization.
Writers / Executors
src/ghdcbot/adapters/.../writer.py
github/writer.py,discord/writer.py
Plan writers to execute GitHub and Discord plans with gating, deduplication, structured logging, retries and error handling.
Discord bot & interaction
src/ghdcbot/bot.py
Add a feature-rich Discord bot module (identity linking, verify, status, summary, issue/mentor flows) and run_bot/main entrypoints.
Logging, plugins & utils
src/ghdcbot/logging/setup.py, src/ghdcbot/plugins/registry.py, src/ghdcbot/.../__init__.py
Add JsonFormatter/configure_logging, plugin loader (load_adapter/build_adapter), and package markers across packages.
Identity & notifications
src/ghdcbot/engine/identity_linking.py, src/ghdcbot/adapters/github/identity.py, src/ghdcbot/engine/notifications.py
Add identity-claim lifecycle, GitHub identity reader (bio/gist search), and verified-only notification dispatch with dedupe and auditing.
Issue/mentor flows & embeds
src/ghdcbot/engine/issue_assignment.py,issue_request_flow.py,pr_context.py
Add issue URL parsing, fetch/context helpers, eligibility and mentor embeds, request grouping, and PR context/embed builders.
Audit export & metrics
src/ghdcbot/engine/audit_export.py,metrics.py
Add pure functions to filter/format audit exports (CSV/MD), and read-only contribution metrics, ranking, and window helpers.
Tests
tests/*, tests/conftest.py
Add extensive pytest suite covering config validation, ingestion, planning determinism/correctness, scoring (difficulty/quality), merge-role rules, identity linking, writers safety/gating, audit export, metrics, and CLI/readme run flows.

Sequence Diagram(s)

sequenceDiagram
    participant CLI as CLI
    participant Orch as Orchestrator
    participant GH as GitHubRestAdapter
    participant DB as SqliteStorage
    participant Score as WeightedScoreStrategy
    participant Plan as PlanningEngine
    participant DC as DiscordApiAdapter
    participant Report as Reporting

    CLI->>Orch: run_once()
    activate Orch

    Orch->>DB: init_schema()
    Orch->>GH: list_contributions(since)
    GH-->>Orch: contributions
    Orch->>DB: record_contributions(contributions)
    Orch->>DB: set_cursor(github)

    Orch->>Score: compute_scores(contributions, period_end)
    Score-->>Orch: scores
    Orch->>DB: upsert_scores(scores)

    Orch->>DC: list_member_roles()
    DC-->>Orch: member_roles

    Orch->>Plan: plan_discord_roles(member_roles, scores, ...)
    Plan-->>Orch: discord_plans
    Orch->>GH: list_open_issues()
    Orch->>GH: list_open_pull_requests()
    Orch->>Plan: plan_github_assignments(issues, prs, ...)
    Plan-->>Orch: github_plans

    Orch->>Report: write_reports(discord_plans, github_plans)
    Report-->>Orch: audit_paths

    Orch->>GH: apply_plans(github_plans, policy)
    Orch->>DC: apply_plans(discord_plans, policy)

    deactivate Orch
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Poem

🐰 I nibbled through configs, tokens warm and bright,

Collected plans in silence, kept the garden right,
Dry-run saved the flowers, observer watched the sky,
Audits in my basket, deterministic why,
A rabbit hums: safe runs all through the night.

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 45.66% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title clearly describes the main change: adding the initial Gitcord engine with GitHub/Discord ingestion and an audit-first dry-run workflow. It is specific, actionable, and directly reflects the primary objectives of the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 Fix all issues with AI agents
In `@src/ghdcbot/__init__.py`:
- Line 1: Replace the EN DASH character in the module docstring with a standard
ASCII hyphen: edit the docstring in src/ghdcbot/__init__.py (the top-line
triple-quoted string) and change “Discord–GitHub automation engine.” to
“Discord-GitHub automation engine.” so the module-level string uses a normal
hyphen.

In `@src/ghdcbot/adapters/discord/writer.py`:
- Around line 36-44: In _apply_plan, do not default to DELETE for unknown
plan.action values; explicitly handle "add" and "remove" (or "delete") and treat
anything else as invalid: determine method only when plan.action == "add" (set
method="PUT") or plan.action == "remove" (set method="DELETE"), and for other
values call self._log_plan(plan, result="failed (unknown action:
{plan.action})") and return without making the request; ensure you reference
_apply_plan, plan.action, method, and _log_plan when implementing the validation
and logging change.

In `@src/ghdcbot/adapters/storage/sqlite.py`:
- Around line 49-89: Normalize all created_at datetimes to UTC when writing and
when reading: in record_contributions convert event.created_at to an aware UTC
datetime before serializing (use astimezone(timezone.utc)) and store its ISO
string with timezone (or a trailing Z), and in list_contributions parse the
stored timestamp as UTC-aware datetime (or convert the parsed datetime to UTC)
before constructing ContributionEvent; apply the same normalization for any
other methods that persist or query cursor/created_at values (the other similar
block referenced around lines 132-149) so all comparisons use consistent
UTC-aware timestamps.

In `@src/ghdcbot/config/loader.py`:
- Around line 18-25: The load_config function currently only checks
Path.exists() and lets read_text OSError propagate; update load_config to first
ensure the path is a file (use config_path.is_file()) and then wrap the call to
config_path.read_text(encoding="utf-8") in a try/except catching OSError (and/or
IOError), raising ConfigError with a clear message that includes the original
exception (use "raise ConfigError(...) from exc") so unreadable files or
directories produce consistent ConfigError behavior; keep the existing
yaml.YAMLError handling for parsing errors.

In `@src/ghdcbot/engine/orchestrator.py`:
- Around line 38-45: The cursor is being set to period_end which can miss events
arriving during the fetch and cause duplicates; instead, after calling
self.github_reader.list_contributions and before self.storage.set_cursor,
compute the latest ingested timestamp from the returned contributions (e.g.,
max(item.created_at) or equivalent), optionally add a tiny epsilon, and call
self.storage.set_cursor("github", latest_ingested_timestamp) so the cursor
reflects the actual last ingested event; update references around
list_contributions, record_contributions, and set_cursor to use that computed
timestamp rather than period_end.

In `@tests/test_user_repo_fallback.py`:
- Around line 18-41: Replace the direct assignment to loader._ACTIVE_CONFIG in
the test with monkeypatch.setattr so the global is restored after the test;
specifically, use the test's monkeypatch fixture to set loader._ACTIVE_CONFIG to
the prepared config (monkeypatch.setattr(loader, "_ACTIVE_CONFIG", config))
instead of assigning loader._ACTIVE_CONFIG = config, ensuring no cross-test
leakage.
🧹 Nitpick comments (23)
pyproject.toml (1)

1-16: Add [project.scripts] entry point to make the CLI directly invocable.

Build system, metadata, and dependencies are well-structured with consistent Python version targeting (3.11). The CLI module at src/ghdcbot/cli.py exists with a proper main() entry point. Add the following to enable installation as an executable command:

Suggested configuration
[project.scripts]
ghdcbot = "ghdcbot.cli:main"

Also consider adding a license field for open-source clarity, though this can be deferred.

tests/test_repo_filtering.py (1)

21-31: Remove unused caplog parameter.

The caplog fixture is declared but not used in this test function. Either remove it or add a log assertion for consistency with the other tests.

Suggested fix
-def test_repo_filter_deny_mode(caplog) -> None:
+def test_repo_filter_deny_mode() -> None:
     repos = [
         {"name": "repo-a"},
         {"name": "repo-b"},
         {"name": "repo-c"},
     ]
     repo_filter = RepoFilterConfig(mode="deny", names=["repo-b"])

     filtered = _apply_repo_filter(repos, repo_filter, logging.getLogger("test"))

     assert [repo["name"] for repo in filtered] == ["repo-a", "repo-c"]
src/ghdcbot/config/models.py (1)

95-104: Inconsistent default and validation for role_mappings.

The field role_mappings has default_factory=list suggesting it's optional with an empty list default, but the validator immediately rejects empty lists. This creates a confusing API where the field appears optional but instantiation without it will fail validation.

Consider either:

  1. Remove the default to make it explicitly required, or
  2. Keep the default but document that it must be populated
Option 1: Make field explicitly required (preferred)
-    role_mappings: list[RoleMappingConfig] = Field(default_factory=list)
+    role_mappings: list[RoleMappingConfig]
tests/test_writer_safety.py (1)

6-11: Silence Ruff ARG002/TRY003 to avoid test lint failures.

If Ruff is enforced in CI, _FailingClient will trip ARG002 (unused args) and TRY003. Prefer underscore‑prefixed args or a local # noqa to keep the intent clear.

💡 Suggested tweak
 class _FailingClient:
-    def request(self, *args, **kwargs):
+    def request(self, *_args, **_kwargs):
         raise AssertionError("HTTP call should not occur with empty plans")

-    def post(self, *args, **kwargs):
+    def post(self, *_args, **_kwargs):
         raise AssertionError("HTTP call should not occur with empty plans")
tests/test_config.py (1)

1-26: Use pytest.raises(ValidationError) instead of catching all exceptions.

Catching Exception can hide unrelated failures. A targeted ValidationError assertion is clearer and stricter. Line 21–26.

♻️ Suggested update
-from ghdcbot.config.models import BotConfig
+import pytest
+from pydantic import ValidationError
+from ghdcbot.config.models import BotConfig
@@
-    try:
-        BotConfig.model_validate(payload)
-    except Exception as exc:  # noqa: BLE001
-        assert "role_mappings" in str(exc)
-    else:
-        raise AssertionError("Expected role_mappings validation error")
+    with pytest.raises(ValidationError) as excinfo:
+        BotConfig.model_validate(payload)
+    assert "role_mappings" in str(excinfo.value)
src/ghdcbot/core/errors.py (1)

5-6: Consider renaming to avoid shadowing Python's built-in PermissionError.

The custom PermissionError class shadows the built-in exception, which is poor practice despite being unused in the codebase. If this class is intended for future use, rename it (e.g., GitcordPermissionError) to maintain clarity and follow naming conventions.

tests/test_empty_org_behavior.py (1)

43-46: Mock function signature should include underscore prefix for self.

The mock fake_list_repos_from_path is being assigned to an instance method via monkeypatch.setattr(adapter, "_list_repos_from_path", ...). When called on the instance, Python won't pass self since you're replacing it directly on the instance (not the class), so this is actually correct. However, the unused path parameter triggers a linter warning.

Consider using _ to silence the unused argument warning:

Suggested tweak
-    def fake_list_repos_from_path(path: str):
+    def fake_list_repos_from_path(_path: str):
         return [], 200
README.md (1)

27-38: Add language identifiers to fenced code blocks.

The markdownlint warnings about missing language specifiers are valid. Adding language identifiers improves syntax highlighting and accessibility.

Suggested fixes
 ## Repository Structure
-```
+```text
 src/ghdcbot/
   adapters/      # GitHub/Discord/storage adapters (IO)
 1. Create a virtual environment and install dependencies:
-```
+```bash
 python -m venv .venv
 2. Export required tokens as environment variables:
-```
+```bash
 export GITHUB_TOKEN="your_github_token"
 3. Copy and edit the example config:
-```
+```bash
 cp config/example.yaml /tmp/ghdcbot-config.yaml
 6. Run a dry‑run cycle:
-```
+```bash
 python -m ghdcbot.cli --config /tmp/ghdcbot-config.yaml run-once
 7. Expected output files:
-```
+```text
 <data_dir>/reports/audit.json
 Run the safety‑focused test suite:
-```
+```bash
 pytest

Also applies to: 52-56, 58-61, 63-65, 69-71, 73-76, 104-106

src/ghdcbot/plugins/registry.py (1)

11-18: Handle edge case where dotted path contains multiple colons.

dotted_path.split(":") will produce more than two elements if the path contains multiple colons (e.g., "module:Class:extra"), causing the unpacking to fail with a ValueError. While this is caught and wrapped in AdapterError, using split(":", 1) would be more explicit about the expected format.

Suggested fix
 def load_adapter(dotted_path: str) -> type[T]:
     try:
-        module_path, class_name = dotted_path.split(":")
+        module_path, class_name = dotted_path.split(":", 1)
         module = importlib.import_module(module_path)
         adapter_cls = getattr(module, class_name)
     except (ValueError, ImportError, AttributeError) as exc:
         raise AdapterError(f"Unable to load adapter: {dotted_path}") from exc
     return adapter_cls
src/ghdcbot/cli.py (2)

45-45: Replace EN DASH with HYPHEN-MINUS for consistency.

The string contains an EN DASH (–) which may cause issues with character encoding or searching. Use a standard hyphen-minus (-) instead.

Suggested fix
-    parser = argparse.ArgumentParser(description="Discord–GitHub automation engine")
+    parser = argparse.ArgumentParser(description="Discord-GitHub automation engine")

44-54: Consider adding top-level error handling for better CLI UX.

Currently, exceptions from build_orchestrator or run_once will propagate with full tracebacks. For production use, consider catching known exceptions (ConfigError, AdapterError) and providing user-friendly error messages with appropriate exit codes.

Suggested improvement
 def main() -> None:
     parser = argparse.ArgumentParser(description="Discord-GitHub automation engine")
     parser.add_argument("--config", required=True, help="Path to config YAML file")
     sub = parser.add_subparsers(dest="command", required=True)
     sub.add_parser("run-once", help="Run a single orchestration cycle")

     args = parser.parse_args()
-    orchestrator = build_orchestrator(args.config)
-
-    if args.command == "run-once":
-        orchestrator.run_once()
+    try:
+        orchestrator = build_orchestrator(args.config)
+        if args.command == "run-once":
+            orchestrator.run_once()
+    except Exception as exc:
+        logging.getLogger("CLI").error("Fatal error: %s", exc)
+        raise SystemExit(1) from exc
tests/test_mutation_policy_gating.py (2)

11-16: Consider extracting _FailingClient to a shared test utility.

This class duplicates _FailingClient in tests/test_writer_safety.py. Extract to a shared conftest.py or test utilities module to reduce duplication.

Suggested approach

In tests/conftest.py:

import pytest

class FailingHttpClient:
    """Test double that fails if any HTTP call is attempted."""
    def request(self, *_args, **_kwargs):
        raise AssertionError("HTTP call should not occur in gated modes")

    def post(self, *_args, **_kwargs):
        raise AssertionError("HTTP call should not occur in gated modes")

`@pytest.fixture`
def failing_client():
    return FailingHttpClient()

Also, remove the unused # noqa: D401 directives as the D401 rule isn't enabled.


55-59: Test assertion could be more robust.

The assertion record.__dict__.get("result") relies on the internal structure of LogRecord. Consider using getattr(record, "result", None) or accessing record.__dict__ more explicitly.

However, the test correctly validates that:

  1. No HTTP calls occur (via _FailingClient)
  2. Appropriate skip reasons are logged for each gating mode
Minor improvement
-    assert any(record.__dict__.get("result") == expected for record in caplog.records)
+    results = [getattr(r, "result", None) for r in caplog.records]
+    assert expected in results, f"Expected '{expected}' in log results, got {results}"
src/ghdcbot/engine/scoring.py (1)

21-24: Add upper bound check for period_end to match the intended period boundaries.

The current code filters events before period_start but ignores events after period_end, even though period_end is stored in the Score result. Since the Score model explicitly tracks both period boundaries, the filtering should enforce them consistently.

Suggested fix
         for event in contributions:
-            if event.created_at < period_start:
+            if event.created_at < period_start or event.created_at > period_end:
                 continue
             totals[event.github_user] += self._weights.get(event.event_type, 0)

While GitHub API data in practice contains only real event timestamps and won't include future-dated events, this defensive check removes the asymmetry between boundary checks and ensures the period semantics are enforced correctly throughout the codebase.

src/ghdcbot/engine/assignment.py (1)

28-30: Silence the unused scores parameter.

If scores is required by the interface, rename it to _scores (or add _ = scores) to document intentional non‑use and avoid lint noise.

♻️ Suggested tweak
-    def plan_issue_assignments(
-        self, issues: Iterable[dict], scores: Sequence[Score]
-    ) -> Sequence[AssignmentPlan]:
+    def plan_issue_assignments(
+        self, issues: Iterable[dict], _scores: Sequence[Score]
+    ) -> Sequence[AssignmentPlan]:
@@
-    def plan_review_requests(
-        self, pull_requests: Iterable[dict], scores: Sequence[Score]
-    ) -> Sequence[ReviewPlan]:
+    def plan_review_requests(
+        self, pull_requests: Iterable[dict], _scores: Sequence[Score]
+    ) -> Sequence[ReviewPlan]:

Also applies to: 48-50

src/ghdcbot/engine/orchestrator.py (1)

92-100: Tidy audit report handling (unused md_path + exception logging).

Rename md_path to _md_path and use logger.exception to keep tracebacks.

♻️ Suggested tweak
-                json_path, md_path = write_reports(
+                json_path, _md_path = write_reports(
                     discord_plans, github_plans, self.config, repo_count=repo_count
                 )
@@
-            except Exception as exc:  # noqa: BLE001
-                logger.error("Failed to write audit reports", extra={"error": str(exc)})
+            except Exception as exc:  # noqa: BLE001
+                logger.exception("Failed to write audit reports", extra={"error": str(exc)})
src/ghdcbot/core/models.py (1)

1-62: Well-structured domain models with good immutability intent.

The frozen dataclasses provide a clean, immutable interface for the domain models. A few optional improvements to consider:

  1. Mutable dict fields in frozen dataclasses: Fields like payload and source (lines 14, 51, 62) are dicts which remain mutable even in frozen dataclasses. This is a known Python limitation where the field reference is frozen but contents can still be modified. Consider using MappingProxyType or documenting this behavior if true immutability is needed.

  2. Type safety for constrained strings: The action and target_type fields use string comments to indicate valid values. Using Literal types would provide compile-time safety:

from typing import Literal

action: Literal["add", "remove"]
target_type: Literal["issue", "pull_request"]
src/ghdcbot/adapters/github/rest.py (3)

21-33: Consider adding resource cleanup for the httpx.Client.

The httpx.Client is created but never explicitly closed. While Python's garbage collector will eventually clean it up, explicit resource management is preferred for HTTP clients to avoid connection leaks, especially in long-running processes.

♻️ Suggested approaches

Option 1: Add a close method and use it explicitly:

def close(self) -> None:
    self._client.close()

Option 2: Implement context manager protocol:

def __enter__(self) -> GitHubRestAdapter:
    return self

def __exit__(self, *args) -> None:
    self._client.close()

Option 3: Use atexit for cleanup in long-running scenarios.


277-289: Aggressive rate limit cutoff may cause incomplete ingestion.

When remaining <= 1, the method returns None and stops all subsequent requests. This is a safe default but may lead to incomplete data ingestion if the rate limit is low but not fully exhausted.

Consider:

  1. Allowing continuation when remaining == 1 (stop at 0)
  2. Adding a configurable threshold
  3. Implementing a sleep-until-reset strategy for non-urgent ingestion

The current behavior is safe but worth documenting for operators who may be surprised by partial results.


185-194: Assignee timestamp is approximated from updated_at.

Using issue["updated_at"] as assigned_at (line 186) is an approximation—the actual assignment timestamp would require fetching issue timeline events. This trade-off avoids additional API calls but may attribute assignments to incorrect time periods.

Consider adding a code comment to document this approximation for future maintainers.

src/ghdcbot/adapters/discord/api.py (2)

16-24: Same resource cleanup consideration as the GitHub adapter.

The httpx.Client should be explicitly closed when the adapter is no longer needed. Consider adding a close() method or implementing the context manager protocol for consistent resource management.


146-151: Consider implementing retry with retry_after instead of immediate abort.

The retry_after value from Discord's 429 response is logged but not used. For improved resilience, consider:

  1. Sleeping for retry_after seconds and retrying the request
  2. Or exposing retry_after to the caller for upstream handling

Current behavior is safe but may cause unnecessary incomplete operations.

♻️ Suggested retry implementation
if response.status_code == 429:
    retry_after = response.json().get("retry_after", 1)
    self._logger.warning(
        "Discord rate limit reached; retrying after delay",
        extra={"path": path, "retry_after": retry_after},
    )
    time.sleep(retry_after)
    return self._request(method, path, params)  # Retry once

Note: Add recursion limit or use a loop to prevent infinite retries.

src/ghdcbot/engine/planning.py (1)

43-47: Potential StopIteration if role not found in role_thresholds.

The next() call at lines 43-47 (and similarly at lines 61-65) has no default value. If role is not found in role_thresholds, this will raise StopIteration, which in a generator context could silently terminate iteration.

However, since desired_roles is derived from role_thresholds (line 27-31), the role should always exist. Consider adding an assertion or default for defensive coding:

♻️ Defensive alternative
                     "threshold": next(
-                            mapping_cfg.min_score
-                            for mapping_cfg in role_thresholds
-                            if mapping_cfg.discord_role == role
+                            (mapping_cfg.min_score
+                             for mapping_cfg in role_thresholds
+                             if mapping_cfg.discord_role == role),
+                            0,  # Should never happen, but safe default
                     ),

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@github-actions github-actions bot added size/XL and removed size/XL labels Jan 20, 2026
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@github-actions github-actions bot added size/XL and removed size/XL labels Jan 20, 2026
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@github-actions github-actions bot added size/XL and removed size/XL labels Jan 20, 2026
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/ghdcbot/adapters/discord/writer.py`:
- Around line 19-26: The httpx.Client created in __init__ (self._client) is
never closed; add a proper teardown by implementing an explicit close() method
that calls self._client.close() and/or implement context manager methods
__enter__ and __exit__ on the same class so callers can use "with" to auto-close
the client; update any callers to call close() or use the context manager as
needed to release connection pool resources.
🧹 Nitpick comments (3)
src/ghdcbot/adapters/discord/writer.py (3)

28-34: Consider adding deduplication logic like the GitHub writer.

The GitHub writer uses a seen set to skip duplicate plans. Without this, redundant API calls may occur if duplicate plans are generated, wasting rate limit quota.

♻️ Proposed deduplication
     def apply_plans(self, plans: Iterable[DiscordRolePlan], policy: MutationPolicy) -> None:
+        seen: set[tuple[str, str, str]] = set()
         for plan in plans:
             skip_reason = _skip_reason(policy, policy.allow_discord_mutations)
             if skip_reason:
                 self._log_plan(plan, result=skip_reason)
                 continue
+            dedupe_key = (plan.discord_user_id, plan.role, plan.action)
+            if dedupe_key in seen:
+                self._log_plan(plan, result="skipped (duplicate)")
+                continue
+            seen.add(dedupe_key)
             self._apply_plan(plan)

65-88: Role list is fetched on every plan execution, causing redundant API calls.

For N plans, this makes N identical requests to /guilds/{guild_id}/roles. Cache the role mapping to avoid hitting Discord rate limits.

♻️ Proposed caching approach
     def __init__(self, token: str, guild_id: str) -> None:
         self._logger = logging.getLogger(self.__class__.__name__)
         self._guild_id = guild_id
         self._client = httpx.Client(
             base_url="https://discord.com/api/v10",
             headers={"Authorization": f"Bot {token}"},
             timeout=30.0,
         )
+        self._role_cache: dict[str, str] | None = None

+    def _fetch_roles(self) -> dict[str, str] | None:
+        """Fetch and cache role name -> ID mapping."""
+        if self._role_cache is not None:
+            return self._role_cache
+        try:
+            response = self._client.request("GET", f"/guilds/{self._guild_id}/roles")
+        except httpx.HTTPError as exc:
+            self._logger.warning("Role lookup failed", extra={"guild_id": self._guild_id, "error": str(exc)})
+            return None
+        if response.status_code != 200:
+            self._logger.warning("Role lookup failed", extra={"guild_id": self._guild_id, "status": response.status_code})
+            return None
+        self._role_cache = {r["name"]: r["id"] for r in response.json()}
+        return self._role_cache

     def _resolve_role_id(self, role_name: str) -> str | None:
-        """Resolve a role name to a role ID. Returns None if not found."""
-        try:
-            response = self._client.request(
-                "GET", f"/guilds/{self._guild_id}/roles"
-            )
-        except httpx.HTTPError as exc:
-            self._logger.warning(
-                "Role lookup failed",
-                extra={"guild_id": self._guild_id, "error": str(exc)},
-            )
-            return None
-        if response.status_code != 200:
-            self._logger.warning(
-                "Role lookup failed",
-                extra={"guild_id": self._guild_id, "status": response.status_code},
-            )
+        """Resolve a role name to a role ID. Returns None if not found."""
+        roles = self._fetch_roles()
+        if roles is None:
             return None
-
-        roles = response.json()
-        for role in roles:
-            if role.get("name") == role_name:
-                return role.get("id")
-        return None
+        return roles.get(role_name)

111-118: Duplicate of _skip_reason in github/writer.py.

This helper is identical to the one in the GitHub writer. Consider extracting it to a shared location (e.g., core/modes.py) to avoid duplication.

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@github-actions github-actions bot added size/XL and removed size/XL labels Jan 20, 2026
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@github-actions github-actions bot added size/XL and removed size/XL labels Feb 10, 2026
shubham5080 and others added 2 commits February 10, 2026 23:28
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@github-actions github-actions bot added size/XL and removed size/XL labels Feb 10, 2026
…yping

- _ingest_helpful_comments: accept pre-fetched comments to avoid duplicate API pagination
- _ingest_issue_comments/_ingest_pr_comments: return (events, comments_by_number) for reuse
- _issue_assignment_events: log exceptions with debug (owner, repo, issue_number)
- _detect_reverted_pr/_check_pr_ci_status: log exceptions with context instead of bare pass
- sqlite: add Any to typing imports for mark_notification_sent
- orchestrator: preserve merge-based roles on removal (score_desired | merge_desired)
- _send_role_congratulation: generic message 'You have earned the role'
- pr_context: return 'Waiting on contributor' when approved but not mergeable

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions github-actions bot added size/XL and removed size/XL labels Feb 10, 2026
- Notifications: improved DM messages (issue assign, changes requested, PR approved/merged); dedupe key includes review_id for pr_reviewed; logging for notification flow
- Issue assignment: skip already-assigned issues in sync (assignment.py, planning.py); rest.py list_open_issues includes assignees
- Bot: mentor-only /assign-issue, /issue-requests, /sync with mentor_check; friendly permission error; issue request approve/replace DM messages with issue title
- Add AUDIT_TESTING_POINTS.txt (1060 manual audit test points) and mentor_features_summary.txt

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions github-actions bot added size/XL and removed size/XL labels Feb 12, 2026
@shubham5080
Copy link
Author

- Add comprehensive Discord bot commands section with all 12 slash commands
- Add instructions for running the Discord bot
- Add Gitcord logo alongside AOSSIE logo in README header
- Organize commands into Identity Linking, Contribution & Metrics, and Issue Management categories

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions github-actions bot added size/XL and removed size/XL labels Feb 16, 2026
Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions github-actions bot added size/XL and removed size/XL labels Feb 16, 2026
- Remove all .md files from root and docs/ directory
- Keep only README.md as requested
- Remove CONTRIBUTING.md, all docs/*.md files, and other documentation files

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions github-actions bot added size/XL and removed size/XL labels Feb 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants