feat: add GitHub to Logseq TODO generator - #125
Closed
theinterneti wants to merge 5 commits into
Closed
theinterneti wants to merge 5 commits into
theinterneti wants to merge 5 commits into
Conversation
- Add comprehensive AlertManager setup (804 lines) - Include SLO, performance, cost, and cache alerts - Production-ready with routing and notification configs - Extracted from feature/keploy-framework PR #26 Files: - README.md (355 lines): Documentation and alert catalog - alertmanager.yaml (223 lines): Routing and notification config - tta-alerts.yaml (226 lines): Prometheus alert rules This is genuinely new content - we had NO existing AlertManager setup.
…26 - Add comprehensive cost tracking dashboard (413 lines, 5 panels) - Panels: Total Cost, Total Savings, Savings Rate, Cost by Primitive, Savings by Primitive - Tracks tta_workflow_cost_total and tta_workflow_cost_saved metrics - Extracted from feature/keploy-framework PR #26 This is genuinely new content - we had NO existing cost tracking dashboard.
…t files - Added worktree-specific files like workspace.code-workspace, .COORDINATION_NOTICE, and .AGENT_ID, generated by setup-worktrees.sh - Included agent-specific temp directories (.copilot-temp/, .cline-temp/, .augment-temp/) and configuration caches (.cline/sessions/, .augment/cache/) to prevent accidental commits of temporary or configuration data in development workflows.
👋 Welcome @theinterneti!Thanks for your contribution! Here's how to work with this repo the lazy way: 🤖 Quick Setup# Make script executable
chmod +x scripts/lazy_dev.py
# Run interactive mode
./scripts/lazy_dev.py🚀 Common Tasks# Check status
./scripts/lazy_dev.py status
# Start new work
./scripts/lazy_dev.py work-on "your feature"
# Create PR (AI-powered!)
./scripts/lazy_dev.py pr🤝 Agent Collaboration
📚 Resources
🤖 This is an automated message. The lazy_dev.py script was created to make your life easier! |
|
💡 Pro Tip: This PR could have been created automatically! Try using the lazy dev manager next time: ./scripts/lazy_dev.py prIt will:
See LAZY_DEV_GUIDE.md for details. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
🎯 Lazy Dev Pro TipsAdd these aliases to your # Quick commands
alias work='./scripts/lazy_dev.py work-on'
alias pr='./scripts/lazy_dev.py pr'
alias status='./scripts/lazy_dev.py status'
alias ship='git add . && git commit -m "update" && ./scripts/lazy_dev.py pr'Then you can just type:
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds a GitHub-to-Logseq TODO generator by implementing IssueManager.generate_logseq_todo() to sync GitHub issues into Logseq-formatted TODO blocks. However, the PR contains significant scope issues: approximately 90% of the changed files are unrelated to the stated purpose and should be in separate pull requests.
Key changes related to the PR title:
- Adds
generate_logseq_todo()method toIssueManagerfor converting GitHub issues to Logseq TODO format - Implements label-to-priority and label-to-type mapping logic
- Adds CLI command
logseq-todo <issue-number>to the issue manager - Includes test suite with 3 test cases covering basic functionality
Critical issues identified:
- Two bugs where invalid TODO types ("bugfix" and "observability") are generated, violating Logseq conventions
- Missing test coverage for multiple edge cases (multiple assignees, priority precedence, empty tags)
- Maintainability concerns with order-dependent label processing
Reviewed Changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/issue_manager.py |
Adds generate_logseq_todo() method (60 lines) with label-to-property mapping; includes 2 bugs with invalid TODO types |
tests/test_issue_manager_logseq.py |
Test suite with 3 test cases; missing coverage for several edge cases |
scripts/intelligent_extraction.py |
❌ Out of scope - PR #26 analysis script (133 lines); should be separate PR |
scripts/git-commit-tracker.py |
❌ Out of scope - Complete Python→Bash rewrite (158→66 lines); should be separate PR |
scripts/analyze_pr_26.py |
❌ Out of scope - PR #26 analysis script (262 lines); should be separate PR |
pr_26_analysis.json |
❌ Out of scope - Generated analysis data (475 lines); should be separate PR |
platform/primitives/dashboards/grafana/cost-tracking.json |
❌ Out of scope - Grafana dashboard (413 lines); should be separate PR |
platform/primitives/dashboards/alertmanager/*.yaml |
❌ Out of scope - AlertManager configs (449 lines); should be separate PR |
platform/primitives/dashboards/alertmanager/README.md |
❌ Out of scope - AlertManager docs (355 lines); should be separate PR |
docs/development/MULTI_AGENT_OVERSIGHT.md |
❌ Out of scope - Git collaboration docs (+177 lines); should be separate PR |
PR_26_EXTRACTION_PLAN.md |
❌ Out of scope - PR planning document (363 lines); should be separate PR |
AGENTS.md |
❌ Out of scope - Agent git docs (+26 lines); should be separate PR |
.gitignore |
❌ Out of scope - Worktree ignores (+15 lines); should be separate PR |
Comment on lines
+1
to
+133
| #!/usr/bin/env python3 | ||
| """ | ||
| Intelligent PR #26 Content Extraction | ||
|
|
||
| Compares PR #26 content against current workspace and extracts only improvements. | ||
| """ | ||
|
|
||
| import json | ||
| import os | ||
| from pathlib import Path | ||
|
|
||
| # Current state analysis | ||
| CURRENT_STATE = { | ||
| "grafana_dashboards": [ | ||
| "platform/primitives/dashboards/grafana/orchestration-metrics.json", | ||
| "monitoring/grafana/dashboards/adaptive-primitives.json", | ||
| ], | ||
| "observability_primitives": [ | ||
| "platform/observability/src/observability_integration/primitives/cache.py", | ||
| "platform/observability/src/observability_integration/primitives/router.py", | ||
| "platform/observability/src/observability_integration/primitives/timeout.py", | ||
| ], | ||
| "alertmanager": [], # No AlertManager configs currently | ||
| } | ||
|
|
||
| # PR #26 offerings | ||
| PR_26_OFFERINGS = { | ||
| "grafana_dashboards": [ | ||
| { | ||
| "path": "packages/tta-dev-primitives/dashboards/grafana/cost-tracking.json", | ||
| "size": 413, | ||
| "value": "⭐⭐⭐ High - Detailed cost tracking dashboard", | ||
| }, | ||
| { | ||
| "path": "packages/tta-dev-primitives/dashboards/grafana/slo-tracking.json", | ||
| "size": 0, # Need to fetch | ||
| "value": "⭐⭐⭐ High - SLO monitoring", | ||
| }, | ||
| { | ||
| "path": "packages/tta-dev-primitives/dashboards/grafana/workflow-overview.json", | ||
| "size": 0, # Need to fetch | ||
| "value": "⭐⭐ Medium - Workflow visualization", | ||
| }, | ||
| ], | ||
| "alertmanager": [ | ||
| { | ||
| "path": "packages/tta-dev-primitives/dashboards/alertmanager/README.md", | ||
| "size": 355, | ||
| "value": "⭐⭐⭐ High - Complete AlertManager setup guide", | ||
| }, | ||
| { | ||
| "path": "packages/tta-dev-primitives/dashboards/alertmanager/alertmanager.yaml", | ||
| "size": 223, | ||
| "value": "⭐⭐⭐ High - AlertManager config", | ||
| }, | ||
| { | ||
| "path": "packages/tta-dev-primitives/dashboards/alertmanager/tta-alerts.yaml", | ||
| "size": 226, | ||
| "value": "⭐⭐⭐ High - 20+ alert rules for primitives", | ||
| }, | ||
| ], | ||
| "observability_code": [ | ||
| { | ||
| "path": "packages/tta-dev-primitives/src/tta_dev_primitives/observability/enhanced_metrics.py", | ||
| "value": "⭐⭐ Medium - Check if better than current", | ||
| }, | ||
| { | ||
| "path": "packages/tta-dev-primitives/src/tta_dev_primitives/observability/instrumented_primitive.py", | ||
| "value": "⭐⭐⭐ High - Enhanced instrumentation", | ||
| }, | ||
| { | ||
| "path": "packages/tta-dev-primitives/src/tta_dev_primitives/observability/prometheus_exporter.py", | ||
| "value": "⭐⭐ Medium - May duplicate existing", | ||
| }, | ||
| ], | ||
| } | ||
|
|
||
|
|
||
| def analyze_extraction_value(): | ||
| """Analyze what from PR #26 should be extracted.""" | ||
| print("=" * 80) | ||
| print("INTELLIGENT EXTRACTION ANALYSIS") | ||
| print("=" * 80) | ||
| print() | ||
|
|
||
| # 1. AlertManager - Clear win (we have NONE currently) | ||
| print("1️⃣ ALERTMANAGER (EXTRACT ALL)") | ||
| print(" Current state: ❌ No AlertManager configs") | ||
| print(" PR #26 offers: ✅ Complete AlertManager setup (804 lines)") | ||
| print(" Decision: ⭐⭐⭐ EXTRACT - Adds entirely new capability") | ||
| print() | ||
|
|
||
| # 2. Grafana Dashboards - Need comparison | ||
| print("2️⃣ GRAFANA DASHBOARDS (SELECTIVE EXTRACTION)") | ||
| print(" Current state: ✅ 2 dashboards (orchestration, adaptive)") | ||
| print(" PR #26 offers: ✅ 3 dashboards (cost, SLO, workflow)") | ||
| print(" Overlap: 📊 orchestration-metrics.json vs workflow-overview.json") | ||
| print(" Decision: ⭐⭐ COMPARE - Extract non-duplicates") | ||
| print() | ||
|
|
||
| # 3. Observability code - Need detailed comparison | ||
| print("3️⃣ OBSERVABILITY PYTHON CODE (NEEDS REVIEW)") | ||
| print(" Current state: ✅ 3 enhanced primitives (cache, router, timeout)") | ||
| print(" PR #26 offers: ✅ 5 modules (enhanced_metrics, instrumented, exporter, etc)") | ||
| print(" Decision: ⚠️ REVIEW - May have overlapping functionality") | ||
| print() | ||
|
|
||
| # Extraction recommendations | ||
| print("=" * 80) | ||
| print("RECOMMENDED EXTRACTION ORDER") | ||
| print("=" * 80) | ||
| print() | ||
| print("Phase 1: IMMEDIATE EXTRACTION (No conflicts)") | ||
| print(" ✅ AlertManager configs (3 files, 804 lines)") | ||
| print(" → NEW capability, no existing configs") | ||
| print() | ||
| print("Phase 2: SMART DASHBOARD EXTRACTION (After comparison)") | ||
| print(" 📊 cost-tracking.json (413 lines)") | ||
| print(" → NEW dashboard, focused on cost metrics") | ||
| print(" 📊 slo-tracking.json (unknown size)") | ||
| print(" → NEW dashboard, SLO monitoring") | ||
| print(" ⚠️ workflow-overview.json (unknown size)") | ||
| print(" → COMPARE with orchestration-metrics.json first") | ||
| print() | ||
| print("Phase 3: CODE REVIEW (Careful analysis)") | ||
| print(" 🔍 Review observability/*.py modules") | ||
| print(" → Compare functionality with current primitives") | ||
| print(" → Extract only genuine improvements") | ||
| print() | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| analyze_extraction_value() |
There was a problem hiding this comment.
This PR contains numerous unrelated changes that should be in separate pull requests. The PR title and description indicate this is for "GitHub to Logseq TODO generator," but 9 out of 10+ files are completely unrelated:
Unrelated changes that should be separate PRs:
- PR feat: Phase 1 Workflow Enhancements - Observability, API Testing & Validation #26 analysis scripts (
scripts/intelligent_extraction.py,scripts/analyze_pr_26.py,pr_26_analysis.json) - Complete rewrite of
git-commit-tracker.pyfrom Python to Bash - Grafana dashboard (
cost-tracking.json) - AlertManager configurations (3 files, ~800 lines)
- Documentation updates (
MULTI_AGENT_OVERSIGHT.md,PR_26_EXTRACTION_PLAN.md,AGENTS.md) .gitignoreupdates for worktrees
Only in-scope files:
scripts/issue_manager.py(addsgenerate_logseq_todo()method)tests/test_issue_manager_logseq.py
This violates best practices for focused, reviewable PRs. Each category above should be its own PR with appropriate title, description, and context.
Comment on lines
+29
to
+90
| def test_generate_logseq_todo_basic(monkeypatch) -> None: | ||
| manager = IssueManager() | ||
|
|
||
| issue = _make_issue(labels=["P2"], state="OPEN") | ||
| monkeypatch.setattr(manager, "get_issue", lambda n: issue) | ||
|
|
||
| block = manager.generate_logseq_todo(issue.number) | ||
| assert block is not None | ||
|
|
||
| lines = block.splitlines() | ||
|
|
||
| # First line is a TODO with dev tag | ||
| assert lines[0].startswith("- TODO Implement CachePrimitive metrics") | ||
| assert "#dev-todo" in lines[0] | ||
|
|
||
| # Default type and mapped priority | ||
| assert "type:: implementation" in block | ||
| assert "priority:: medium" in block # from P2 | ||
|
|
||
| # Status is lowercased | ||
| assert "status:: open" in block | ||
|
|
||
| # URL includes repo and issue number | ||
| assert re.search(r"url:: https://github.com/.+/issues/42", block) | ||
|
|
||
|
|
||
| def test_generate_logseq_todo_with_labels_and_milestone(monkeypatch) -> None: | ||
| manager = IssueManager() | ||
|
|
||
| issue = _make_issue( | ||
| labels=["P0", "observability", "testing"], | ||
| state="OPEN", | ||
| milestone="Phase 2: Observability Integration", | ||
| assignees=["theinterneti"], | ||
| ) | ||
| monkeypatch.setattr(manager, "get_issue", lambda n: issue) | ||
|
|
||
| block = manager.generate_logseq_todo(issue.number) | ||
| assert block is not None | ||
|
|
||
| lines = block.splitlines() | ||
|
|
||
| # Tags include non-priority labels only | ||
| assert "#observability" in lines[0] | ||
| assert "#testing" in lines[0] | ||
| assert "#P0" not in lines[0] | ||
|
|
||
| # Priority and type reflect labels | ||
| assert "priority:: high" in block # P0 | ||
| assert "type:: observability" in block | ||
|
|
||
| # Milestone and assignee lines present | ||
| assert "milestone:: [[Phase 2: Observability Integration]]" in block | ||
| assert "assigned:: [[@theinterneti]]" in block | ||
|
|
||
|
|
||
| def test_generate_logseq_todo_returns_none_for_missing_issue(monkeypatch) -> None: | ||
| manager = IssueManager() | ||
| monkeypatch.setattr(manager, "get_issue", lambda n: None) | ||
|
|
||
| block = manager.generate_logseq_todo(9999) | ||
| assert block is None |
There was a problem hiding this comment.
Missing test coverage for edge cases with the priority mapping logic. The implementation has fallback behavior when multiple priority labels exist (lines 356-361), using the first match. Consider adding test cases for:
- An issue with multiple priority labels (e.g., both "P0" and "P2") to verify the first-match behavior
- An issue with no priority labels to verify the default "medium" priority
- An issue with both "critical" and "P3" labels to verify precedence
Comment on lines
+358
to
+361
| for label in issue.labels: | ||
| if label in priority_map: | ||
| priority = priority_map[label] | ||
| break |
There was a problem hiding this comment.
[nitpick] The priority mapping logic (lines 357-361) uses a break after finding the first match, but the iteration order through issue.labels is not guaranteed to be deterministic across Python versions or label orderings. If an issue has multiple priority labels (e.g., ["P2", "P0"]), the result depends on label order. Consider either:
- Documenting that the first priority label in the list wins
- Implementing explicit precedence (e.g., always choose the highest priority if multiple exist)
- Adding validation to prevent multiple priority labels
Suggested change
| for label in issue.labels: | |
| if label in priority_map: | |
| priority = priority_map[label] | |
| break | |
| # Explicit precedence: highest priority wins if multiple present | |
| priority_order = ["P0", "critical", "urgent", "P1", "P2", "P3"] | |
| found_priority = None | |
| for p_label in priority_order: | |
| if p_label in issue.labels: | |
| found_priority = priority_map[p_label] | |
| break | |
| if found_priority: | |
| priority = found_priority |
Comment on lines
+380
to
+381
| lines = [ | ||
| f"- TODO {issue.title} {tags_str} #dev-todo", |
There was a problem hiding this comment.
Potential issue with tag formatting: When constructing the TODO line (line 381), if there are no non-priority labels, tags_str will be an empty string, resulting in two consecutive spaces before #dev-todo (e.g., "- TODO Title #dev-todo"). Consider handling this edge case by only adding tags_str if it's non-empty:
tags_str = " ".join(tags)
title_line = f"- TODO {issue.title} {tags_str} #dev-todo" if tags_str else f"- TODO {issue.title} #dev-todo"or
title_line = f"- TODO {issue.title} {tags_str + ' ' if tags_str else ''}#dev-todo"
Suggested change
| lines = [ | |
| f"- TODO {issue.title} {tags_str} #dev-todo", | |
| # Only include tags_str if non-empty to avoid extra spaces | |
| title_line = ( | |
| f"- TODO {issue.title} {tags_str} #dev-todo" | |
| if tags_str | |
| else f"- TODO {issue.title} #dev-todo" | |
| ) | |
| lines = [ | |
| title_line, |
| if "documentation" in issue.labels: | ||
| todo_type = "documentation" | ||
| elif "bug" in issue.labels: | ||
| todo_type = "bugfix" |
There was a problem hiding this comment.
Invalid TODO type "bugfix": According to the Logseq guidelines (.github/instructions/logseq-knowledge-base.instructions.md line 78), valid type:: values for #dev-todo are: implementation | testing | documentation | infrastructure | mcp-integration | examples. The value "bugfix" (line 368) is not in this list.
For issues with a "bug" label, consider mapping to "implementation" instead, or document "bugfix" as a new valid type if it's intentionally being added to the TTA.dev Logseq conventions.
Suggested change
| todo_type = "bugfix" | |
| todo_type = "implementation" |
| """ | ||
|
|
||
| import json | ||
| import subprocess |
There was a problem hiding this comment.
Import of 'subprocess' is not used.
Suggested change
| import subprocess |
| import json | ||
| import subprocess | ||
| from pathlib import Path | ||
| from typing import Dict, List, Tuple |
There was a problem hiding this comment.
Import of 'List' is not used.
Suggested change
| from typing import Dict, List, Tuple | |
| from typing import Dict, Tuple |
| Compares PR #26 content against current workspace and extracts only improvements. | ||
| """ | ||
|
|
||
| import json |
There was a problem hiding this comment.
Import of 'json' is not used.
Suggested change
| import json |
| """ | ||
|
|
||
| import json | ||
| import os |
There was a problem hiding this comment.
Import of 'os' is not used.
Suggested change
| import os |
|
|
||
| import json | ||
| import os | ||
| from pathlib import Path |
There was a problem hiding this comment.
Import of 'Path' is not used.
Suggested change
| from pathlib import Path |
4 tasks
Owner
Author
|
Superseded by #131 which contains the clean feature implementation without the infrastructure pollution. |
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
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.
Implements IssueManager.generate_logseq_todo for GitHub→Logseq TODO sync and adds tests in tests/test_issue_manager_logseq.py. Validated with 'uv run ruff format .', 'uv run ruff check . --fix', 'uvx pyright packages/', and 'uv run pytest -v'.