From 75122b3ee22883600f59d2bc27b096d79aeb64a0 Mon Sep 17 00:00:00 2001 From: shokollm <270575765+shokollm@users.noreply.github.com> Date: Thu, 9 Apr 2026 09:30:42 +0000 Subject: [PATCH] docs: add audit and category spec from feynman --- docs/AUDIT_AND_SPEC.md | 544 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 544 insertions(+) create mode 100644 docs/AUDIT_AND_SPEC.md diff --git a/docs/AUDIT_AND_SPEC.md b/docs/AUDIT_AND_SPEC.md new file mode 100644 index 0000000..205dec0 --- /dev/null +++ b/docs/AUDIT_AND_SPEC.md @@ -0,0 +1,544 @@ +# JIGAIDO Audit & Feature Specification + +> Document created: 2026-04-09 +> Purpose: Repository audit findings and category feature specification + +--- + +# Part I: Repository Audit + +## 1.1 Current Architecture + +JIGAIDO follows **hexagonal architecture** with clear separation: + +``` +jigaido/ +├── core/ # Domain layer (pure Python, no deps) +│ ├── models.py # Domain dataclasses +│ ├── ports.py # Storage interfaces +│ └── services.py # Business logic +├── adapters/ +│ └── storage/ +│ └── json_file.py # JSON file persistence +├── apps/ +│ └── telegram-bot/ +│ ├── bot.py # Bot entrypoint +│ └── commands.py # Command handlers +├── tests/ # Unit tests (98 tests passing) +├── config.py # Configuration +├── SPEC.md # Original design spec +└── README.md # Overview +``` + +## 1.2 Features Implemented + +| Feature | Status | Location | +|---------|--------|----------| +| Group bounty management | ✅ Done | `BountyService` | +| Personal DM bounties | ✅ Done | Same service, different room_id | +| Admin management | ✅ Done | `add_admin`, `remove_admin`, `list_admins` | +| Soft delete (recoverable) | ✅ Done | `delete_bounty`, `recover_bounty` | +| Due date with timezone | ✅ Done | `dateparser` + `ZoneInfo` | +| Link deduplication | ✅ Done | `check_link_unique` | +| Tracking/untracking | ✅ Done | `TrackingService` | +| `/track` in groups only | ✅ Done | Command handler | +| Expired bounty filtering | ✅ Done | 24h cutoff logic | +| Timezone per room | ✅ Done | `set_timezone`, `get_timezone` | + +## 1.3 Bugs & Issues Found + +### Bug 1: Admin Promotion Logic Edge Case +**Location**: `core/services.py` - `add_admin()` + +**Issue**: The first admin can self-promote, but if the Telegram group creator joins later, they won't be recognized as admin since they're not in `admin_usernames`. + +**Code**: +```python +# core/services.py:44-49 +has_no_admins = room_data is None or not room_data.admin_usernames +is_self_promotion = requesting_username == username + +if not self.is_admin(room_id, requesting_username): + if not (has_no_admins and is_self_promotion): + raise PermissionError("Only admins can add admins.") +``` + +**Recommendation**: Document this behavior or enhance to auto-detect Telegram group creator. + +--- + +### Bug 2: Hard Delete Method in Storage +**Location**: `adapters/storage/json_file.py:113-119` + +**Issue**: `delete_bounty()` in storage does hard delete, while `BountyService.delete_bounty()` does soft delete. The storage method is unused but confusing. + +**Code**: +```python +def delete_bounty(self, room_id: int, bounty_id: int) -> None: + """Delete a bounty from a room.""" + room_data = self.load(room_id) + if room_data is None: + return + room_data.bounties = [b for b in room_data.bounties if b.id != bounty_id] + self.save(room_data) +``` + +**Recommendation**: Remove or mark as deprecated. + +--- + +### Issue 3: Spec vs Code Inconsistency +**Location**: `SPEC.md` vs `commands.py` + +**Issue**: SPEC.md says anyone can `/add` in groups, but code requires admin. + +| Command | SPEC.md | Code | +|---------|---------|------| +| `/add` | anyone | admin only | +| `/edit` | creator only | admin only | +| `/delete` | creator only | admin only | + +**Recommendation**: Update SPEC.md to reflect actual implementation. + +--- + +### Issue 4: No Input Sanitization +**Location**: `commands.py` - `parse_args()` + +**Issue**: Links without proper scheme (e.g., `github.com/user/repo`) are accepted as-is. + +**Recommendation**: Consider normalizing URL scheme or validating format. + +--- + +### Issue 5: No Rate Limiting +**Location**: `/add` command + +**Issue**: No limit on: +- Number of bounties per room +- Text length +- Request rate + +**Recommendation**: Add rate limiting for production use. + +--- + +## 1.4 Test Status + +```bash +$ PYTHONPATH=. python -m pytest tests/ +======================== 98 passed, 1 warning ======================== +``` + +**Note**: Tests require `PYTHONPATH=.` to run. Consider adding `pytest.ini` or `pyproject.toml`. + +--- + +# Part II: Category Feature Specification + +## 2.1 Overview + +Add category support to JIGAIDO to allow filtering and organizing bounties. + +**Goals**: +- Admin-only category management (create, delete) +- Multiple categories per bounty (no duplicates) +- Filter bounties by category +- Show categories on `/show` command +- Backward compatibility (existing bounties work without categories) + +## 2.2 Data Model + +### Category +```python +@dataclass +class Category: + """A category for organizing bounties in a room.""" + id: str # slug: lowercase alphabetic only, e.g., "bug", "feature" + name: str # display name: e.g., "Bug", "Feature" + created_at: int + deleted_at: int | None = None # soft delete +``` + +**Constraints**: +- `id` (slug): lowercase alphabetic only, no symbols, e.g., `^[a-z]+$` +- `name`: human-readable display +- Unique within room (slug must be unique) +- Soft delete preserves data + +### Bounty (Modified) +```python +@dataclass +class Bounty: + # ... existing fields ... + id: int + text: str | None + link: str | None + due_date_ts: int | None + created_at: int + created_by_user_id: int + deleted_at: int | None = None + created_by_username: str | None = None + category_ids: list[str] = field(default_factory=list) # NEW +``` + +### RoomData (Modified) +```python +@dataclass +class RoomData: + room_id: int + bounties: list[Bounty] + next_id: int + timezone: str | None = None + admin_usernames: list[str] | None = None + categories: list[Category] = field(default_factory=list) # NEW +``` + +## 2.3 Category Scope + +- **Per room**: Same as bounties, each room (group/DM) has independent categories +- **Admin only**: Only admins can create/delete categories +- **User access**: Regular users can only filter by category + +## 2.4 Service Layer API + +All methods require admin permission unless specified otherwise. + +### Category Management + +```python +class BountyService: + # ... existing methods ... + + # --- Category Management --- + + def add_category( + self, + room_id: int, + slug: str, + name: str, + username: str | None + ) -> Category: + """Create a new category. Admin only. + + Args: + room_id: Room identifier + slug: Category ID (lowercase alphabetic, e.g., "bug") + name: Display name (e.g., "Bug Report") + username: Requesting admin's username + + Returns: + Created Category + + Raises: + PermissionError: If not admin + ValueError: If slug already exists or invalid + """ + ... + + def delete_category( + self, + room_id: int, + slug: str, + username: str | None + ) -> bool: + """Soft delete a category. Admin only. + + Args: + room_id: Room identifier + slug: Category slug to delete + username: Requesting admin's username + + Returns: + True if deleted, False if not found + """ + ... + + def list_categories(self, room_id: int) -> list[Category]: + """List active categories (excludes soft-deleted). + + Args: + room_id: Room identifier + + Returns: + List of active categories + """ + ... + + def get_category(self, room_id: int, slug: str) -> Category | None: + """Get a category by slug (excludes soft-deleted). + + Args: + room_id: Room identifier + slug: Category slug + + Returns: + Category or None if not found + """ + ... +``` + +### Category-to-Bounty Association + +```python + def add_category_to_bounty( + self, + room_id: int, + bounty_id: int, + category_slug: str, + username: str | None + ) -> bool: + """Add category to a bounty. Admin only. + + Args: + room_id: Room identifier + bounty_id: Bounty ID + category_slug: Category slug to add + username: Requesting admin's username + + Returns: + True if newly added, False if already exists + + Raises: + PermissionError: If not admin + ValueError: If bounty or category not found + """ + ... + + def remove_category_from_bounty( + self, + room_id: int, + bounty_id: int, + category_slug: str, + username: str | None + ) -> bool: + """Remove category from a bounty. Admin only. + + Args: + room_id: Room identifier + bounty_id: Bounty ID + category_slug: Category slug to remove + username: Requesting admin's username + + Returns: + True if removed, False if not found + """ + ... + + def update_bounty_categories( + self, + room_id: int, + bounty_id: int, + category_slugs: list[str], + username: str | None + ) -> bool: + """Replace all categories on a bounty. Admin only. + + Args: + room_id: Room identifier + bounty_id: Bounty ID + category_slugs: New list of category slugs + username: Requesting admin's username + + Returns: + True if updated + + Raises: + PermissionError: If not admin + ValueError: If bounty or any category not found + """ + ... +``` + +### Bounty Listing with Category Filter + +```python + def list_bounties( + self, + room_id: int, + category_slugs: list[str] | None = None, + include_expired: bool = False + ) -> list[Bounty]: + """List bounties with optional category filtering. + + Args: + room_id: Room identifier + category_slugs: If provided, filter by ANY of these categories (OR) + include_expired: If True, include bounties past due date + + Returns: + List of non-deleted bounties, sorted by due date + """ + ... +``` + +## 2.5 Filtering Logic + +- **Single category**: `/bounty -c bug` → bounties with "bug" +- **Multiple categories (OR)**: `/bounty -c bug,feature` → bounties with "bug" OR "feature" +- **No filter**: `/bounty` → all bounties (current behavior) + +## 2.6 Command Syntax + +### Category Management +``` +/category - list categories +/category add - create category (admin) +/category delete - soft delete category (admin) +``` + +### Bounty with Category +``` +/add [link] [date] -cat - add with category +/add [link] [date] -cat , - add with multiple categories + +/update -cat - add category to bounty +/update -cat , - set categories (replace all) +/update -cat - - clear all categories +/update -remove-cat - remove specific category +``` + +### Bounty Listing with Filter +``` +/bounty - all bounties (current) +/bounty -c - filter by category +/bounty -c , - filter by multiple categories (OR) +/bounty all - show expired (current) +/bounty all -c - show expired + filter by category +``` + +### Show Bounty +``` +/show - show bounty details with categories +``` + +## 2.7 Display Format + +### `/category` output +``` +Categories: +- bug → Bug Report +- feature → Feature Request +- docs → Documentation +``` + +### `/show ` output (with categories) +``` +[#1] Fix login bug +🔗 https://github.com/... +📅 15 April 2026 14:30 (Asia/Jakarta) +📂 Categories: bug | feature +👤 @username +📌 Created: 2026-04-01 10:00 +``` + +### `/bounty -c bug` output +``` +Filtering with 🐛 bug category: +Showing 3 of 10 bounties: +[#5] ... +[#1] ... +[#3] ... +``` + +### `/bounty -c bug,feature` output +``` +Filtering with 🐛 bug, ✨ feature categories: +Showing 5 of 10 bounties: +[#5] ... +[#1] ... +``` + +## 2.8 Edge Cases + +| Scenario | Behavior | +|----------|----------| +| Delete category | Soft delete - existing bounties keep category in data, but filter won't find it | +| Filter by deleted category | Show "No bounties with this category" or error | +| Add duplicate category to bounty | No-op, return False | +| Add invalid slug (uppercase/symbols) | Reject with validation error | +| Category slug conflict | Reject with "Category already exists" | +| Bounty without categories | `category_ids = []` (backward compatible) | + +## 2.9 Test Cases to Add + +```python +# Category Management +def test_add_category_requires_admin(): +def test_add_category_duplicate_slug_fails(): +def test_add_category_invalid_slug_fails(): +def test_add_category_valid(): +def test_delete_category_soft_deletes(): +def test_deleted_category_not_listed(): +def test_deleted_category_still_in_bounty_data(): +def test_list_categories_empty(): +def test_list_categories_returns_active(): +def test_get_category_not_found(): +def test_get_category_deleted_returns_none(): + +# Category-to-Bounty +def test_add_category_to_bounty(): +def test_add_duplicate_category_to_bounty_noop(): +def test_add_category_to_bounty_invalid_category(): +def test_remove_category_from_bounty(): +def test_remove_category_not_on_bounty_returns_false(): +def test_update_bounty_categories_replace_all(): +def test_update_bounty_categories_validates(): + +# Bounty Listing with Filter +def test_list_bounties_filter_by_single_category(): +def test_list_bounties_filter_by_multiple_categories_or(): +def test_list_bounties_no_category_returns_all(): +def test_list_bounties_category_excludes_deleted_bounties(): +``` + +--- + +# Part III: Implementation Checklist + +## Models Layer +- [ ] Add `Category` dataclass to `core/models.py` +- [ ] Add `category_ids` field to `Bounty` dataclass +- [ ] Add `categories` field to `RoomData` dataclass + +## Storage Layer +- [ ] Update `JsonFileRoomStorage.load()` to deserialize categories +- [ ] Update `JsonFileRoomStorage.save()` to serialize categories + +## Service Layer +- [ ] Implement `add_category()` +- [ ] Implement `delete_category()` +- [ ] Implement `list_categories()` +- [ ] Implement `get_category()` +- [ ] Implement `add_category_to_bounty()` +- [ ] Implement `remove_category_from_bounty()` +- [ ] Implement `update_bounty_categories()` +- [ ] Update `list_bounties()` to support category filter + +## Command Layer +- [ ] Add `/category` command handler +- [ ] Add `-cat` flag parsing to `/add` +- [ ] Add `-cat` and `-remove-cat` flags to `/update` +- [ ] Add `-c` flag to `/bounty` for category filter +- [ ] Update `/show` to display categories + +## Tests +- [ ] Add category management tests +- [ ] Add category-to-bounty tests +- [ ] Add category filter tests + +## Documentation +- [ ] Update README.md with category feature +- [ ] Update command help text + +--- + +# Part IV: Open Questions + +1. **Category icon/emoji**: Should categories have optional emoji? (Not in initial spec, can add later) +2. **Category reactivation**: Should soft-deleted categories be reactable? (Not in initial spec) +3. **Bulk category operations**: Should we support `/category add bulk`? (Not in initial spec) + +--- + +*End of Audit & Specification Document*