feat: config-driven contract + concurrent-writer safety#189
Open
labgadget015-dotcom wants to merge 2 commits into
Open
feat: config-driven contract + concurrent-writer safety#189labgadget015-dotcom wants to merge 2 commits into
labgadget015-dotcom wants to merge 2 commits into
Conversation
Addresses the deferred review findings from PR #180/#188: 1. [MAJOR] config.yaml blocks are now read (were declared but never used). New config_loader.py reads config.yaml via PyYAML with stdlib fallback, so debounce windows, status tags, max_step_len, and require_* sets are config-driven instead of hardcoded. Silent config drift eliminated. 2. [MAJOR] Concurrent-writer safety: ledger _append_with_seq() now holds an exclusive fcntl.flock across seq-assignment + write, preventing the _next_seq race and large-entry interleaving between the recommender (record) and executor (transition) agents. fsync per write. POSIX-only; no-op fallback on non-fcntl platforms. 3. [MINOR] Status validation: transition() now raises ValueError on unknown statuses (e.g. 'assiged' typo) via VALID_STATUSES, so a typo can't silently break de-dup. Tests: 14/14 passing (added 4: config-overrides-max_step_len, config-overrides-debounce, transition-rejects-unknown-status, concurrent-writers-do-not-lose-entries with 8x25 parallel writes).
Contributor
📊 Code Complexity AnalysisSummary:
|
| File | Function | Complexity | Line |
|---|---|---|---|
core/risk_scorer.py |
score_pull_request |
35 | 141 |
autopilot/autopilot.py |
generate_summary |
24 | 195 |
autopilot/staleness_engine.py |
process_stale_prs |
16 | 281 |
autopilot/ai_optimization/performance_monitor.py |
get_benchmark_stats |
15 | 184 |
.github/scripts/weekly_digest.py |
build_blocks |
15 | 38 |
autopilot/recommendation_contract.py |
validate |
14 | 53 |
.github/scripts/metrics_collector.py |
parse_workflow_metrics |
14 | 148 |
.github/scripts/setup_branch_protection.py |
main |
14 | 240 |
.github/scripts/self_healing_system.py |
analyze_failure_patterns |
14 | 256 |
.github/scripts/ai_code_suggestor.py |
_check_import_organization |
14 | 113 |
... and 17 more
Recommendations:
- Break down large functions into smaller, focused units
- Extract complex conditional logic into separate functions
- Use early returns to reduce nesting
🔧 Low Maintainability Files
These files have low maintainability scores and may need refactoring:
| File | Score | Status |
|---|---|---|
.github/scripts/health_dashboard_generator.py |
28.14 | 🔴 |
.github/scripts/workflow_monitor.py |
33.73 | 🔴 |
.github/scripts/ai_code_suggestor.py |
33.76 | 🔴 |
.github/scripts/ai_workflow_optimizer.py |
35.51 | 🔴 |
.github/scripts/performance_benchmark.py |
39.46 | 🔴 |
.github/scripts/self_healing_system.py |
40.27 | 🔴 |
.github/scripts/threshold_monitor.py |
41.13 | 🔴 |
.github/scripts/parallel_code_analyzer_optimized.py |
41.16 | 🔴 |
autopilot/tests/test_recommendation_contract.py |
42.02 | 🔴 |
autopilot/autopilot.py |
42.45 | 🔴 |
autopilot/ai_optimization/anomaly_detector.py |
42.56 | 🔴 |
agents/triage_agent.py |
42.79 | 🔴 |
.github/scripts/refactoring_assistant.py |
43.03 | 🔴 |
autopilot/ai_optimization/intelligent_cache.py |
43.28 | 🔴 |
autopilot/ai_optimization/commit_summarizer.py |
44.05 | 🔴 |
.github/scripts/async_parallel_analyzer.py |
44.47 | 🔴 |
autopilot/ai_optimization/performance_monitor.py |
44.69 | 🔴 |
.github/scripts/badge_generator.py |
45.28 | 🔴 |
.github/scripts/copilot_integration.py |
45.37 | 🔴 |
.github/scripts/distributed_monitoring.py |
45.53 | 🔴 |
.github/scripts/elite_copilot.py |
45.69 | 🔴 |
agents/dependency_agent.py |
45.76 | 🔴 |
.github/scripts/issue_auto_creator.py |
46.39 | 🔴 |
.github/scripts/cost_calculator.py |
46.4 | 🔴 |
.github/scripts/inline_pr_commenter.py |
46.63 | 🔴 |
.github/scripts/complexity_reporter.py |
46.78 | 🔴 |
.github/scripts/pr_triage.py |
47.13 | 🔴 |
core/risk_scorer.py |
48.15 | 🔴 |
autopilot/ai_optimization/nlp_relevance_filter.py |
48.43 | 🔴 |
.github/scripts/pr_inline_commenter.py |
48.47 | 🔴 |
autopilot/staleness_engine.py |
48.73 | 🔴 |
.github/scripts/metrics_collector.py |
48.91 | 🔴 |
.github/scripts/dependency_updater.py |
48.91 | 🔴 |
autopilot/ai_optimization/ml_priority_scorer.py |
49.53 | 🔴 |
.github/scripts/changelog_generator.py |
49.75 | 🔴 |
.github/scripts/parallel_code_analyzer.py |
49.96 | 🔴 |
autopilot/ai_optimization/api_optimizer.py |
50.46 | 🟡 |
agents/security_scan_agent.py |
51.04 | 🟡 |
.github/scripts/workflow_optimizer.py |
51.67 | 🟡 |
.github/scripts/cot_selector.py |
51.73 | 🟡 |
.github/scripts/release_manager.py |
51.92 | 🟡 |
.github/scripts/llm_router.py |
52.35 | 🟡 |
.github/scripts/auto_pr.py |
52.72 | 🟡 |
.github/scripts/notification_manager.py |
53.58 | 🟡 |
.github/scripts/prometheus_exporter.py |
54.96 | 🟡 |
.github/scripts/weekly_digest.py |
55.02 | 🟡 |
core/audit_logger.py |
55.6 | 🟡 |
.github/scripts/gather_context.py |
56.0 | 🟡 |
core/llm_provider.py |
56.32 | 🟡 |
.github/scripts/streaming_results.py |
56.64 | 🟡 |
.github/scripts/setup_branch_protection.py |
57.0 | 🟡 |
.github/scripts/optimized_github_client.py |
58.27 | 🟡 |
agents/orchestrator_agent.py |
59.02 | 🟡 |
agents/code_review_agent.py |
60.45 | 🟡 |
core/github_client.py |
61.96 | 🟡 |
autopilot/decisions/ledger.py |
62.87 | 🟡 |
core/message_queue.py |
63.22 | 🟡 |
core/agent_config.py |
63.86 | 🟡 |
core/idempotency.py |
64.45 | 🟡 |
Maintainability Index Guide:
- 🟢 85-100: Excellent maintainability
- 🟡 65-84: Good maintainability
- 🟠 50-64: Moderate maintainability (consider refactoring)
- 🔴 0-49: Poor maintainability (needs refactoring)
Contributor
🟢 Risk Assessment: LOW (1.5/10)Analysed 6 files, 346+ / 70− lines. Test coverage unchanged or improved. Scoring breakdown
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| try: | ||
| r = _good() | ||
| assert should_post(r, path)[0] is True # first raise allowed | ||
| assert should_post(r, path)[0] is True # first raise allowed |
| r = _good() | ||
| r.steps[0] = "x" * 25 # over the custom 20 limit, under default 120 | ||
| ok, errs = validate(r) | ||
| assert not ok |
| r.steps[0] = "x" * 25 # over the custom 20 limit, under default 120 | ||
| ok, errs = validate(r) | ||
| assert not ok | ||
| assert any("step 1" in e and "20" in e for e in errs) |
| from decisions.ledger import record, should_post | ||
| record(r, ledger.name) | ||
| allow, reason = should_post(r, ledger.name) | ||
| assert allow is False, reason # 100000h window always suppresses |
| record(r, ledger.name) | ||
| try: | ||
| transition(r.signature(), "assiged", path=ledger.name) # typo | ||
| assert False, "expected ValueError for unknown status" |
| transition(r.signature(), "assiged", path=ledger.name) # typo | ||
| assert False, "expected ValueError for unknown status" | ||
| except ValueError as e: | ||
| assert "assiged" in str(e) |
| for t in threads: | ||
| t.join() | ||
|
|
||
| assert errors == [], errors |
| assert errors == [], errors | ||
| from decisions.ledger import _load | ||
| entries = _load(ledger.name) | ||
| assert len(entries) == n_writers * n_each, ( |
| ) | ||
| # seq values should be unique 0..N-1 (no collisions from the race) | ||
| seqs = sorted(e.get("seq", -1) for e in entries) | ||
| assert seqs == list(range(n_writers * n_each)), "seq collisions detected" |
Contributor
🤖 Elite AI Copilot AnalysisElite AI Copilot Analysis ReportGenerated: 2026-07-06 04:04:48 🎯 Health Score: 100.0/100🚀 Top Recommendations
📊 Detailed InsightsCode Quality Baseline Established
Security Scan Initiated
Repository Structure Analyzed
Performance Baseline Captured
Documentation Structure Good
Powered by Elite AI Copilot v1.0 |
Contributor
Code Quality Analysis ❌ FAILEDDuration: 0.02s Tool Results
View detailed results{
"timestamp": "2026-07-06 04:04:50",
"elapsed_seconds": 0.02,
"summary": {
"total_issues": 10,
"critical": 0,
"high": 0,
"medium": 0,
"low": 0
},
"tools": {
"pylint": {
"status": "failed",
"output": "",
"errors": "Pylint error: [Errno 2] No such file or directory: 'pylint'"
},
"flake8": {
"status": "failed",
"output": "",
"errors": "Flake8 error: [Errno 2] No such file or directory: 'flake8'"
},
"bandit": {
"status": "failed",
"output": "",
"errors": "Bandit error: [Errno 2] No such file or directory: 'bandit'"
},
"radon_cc": {
"status": "failed",
"output": "",
"errors": "Radon error: [Errno 2] No such file or directory: 'radon'"
},
"radon_mi": {
"status": "failed",
"output": "",
"errors": "Radon MI error: [Errno 2] No such file or directory: 'radon'"
}
},
"passed": false
} |
Contributor
🔍 Pre-commit Checks🔧 Pre-commit issues were automatically fixed and committed. Please pull the latest changes before pushing again: git pull origin feat/config-driven-contractPre-commit hooks help maintain code quality and consistency. |
Contributor
🔒 Security Scan Results🛡️ Bandit Security Scan
📦 Dependency Vulnerabilities
Vulnerable Dependencies:
Security scans run automatically on every PR. View detailed reports in the Actions tab. |
Owner
Author
🤖 DRC Agent AnalysisRecommendation: 🔴 P0 CRITICAL Summary: Realist recommended Solution 1: Merge As-Is with Hardened CI Gate Next steps:
Strategic fit: Consulting: high · Product: medium · Tech debt: reduces Analysed by GadgetLab DRC Agent (Dreamer → Realist → Critic) · Run |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR follows up on #180/#188 to make the DRC recommendation contract + ledger behavior config-driven and to harden the ledger against concurrent writers by adding file locking and atomic seq assignment.
Changes:
- Introduces
autopilot/config_loader.pyto loadautopilot/config.yaml(with defaults when PyYAML/config are unavailable) and wires contract/debounce/status tags to it. - Updates the decisions ledger to append entries with a lock-held seq assignment +
fsync, and validates lifecycle statuses on transition. - Adds tests for config overrides, unknown-status rejection, and concurrent writers; updates docs.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| autopilot/config_loader.py | New config loader with cached, merged defaults + helpers for debounce/status tags/contract/ledger path. |
| autopilot/decisions/ledger.py | Adds flock-based locking + atomic append-with-seq, config-driven debounce window lookup, and status validation on transitions. |
| autopilot/recommendation_contract.py | Makes validation rules (max step length + required fields by severity) config-driven and exports valid statuses. |
| autopilot/message_formatter.py | Makes status tags config-driven (with fallback). |
| autopilot/tests/test_recommendation_contract.py | Adds tests for config overrides, status validation, and concurrent writers; small formatting tweaks. |
| autopilot/README.md | Updates the autopilot module file list to reflect new config-driven/locked behavior. |
Comments suppressed due to low confidence (1)
autopilot/tests/test_recommendation_contract.py:5
- These tests live under autopilot/tests/, but pytest in this repo is configured with testpaths = ["tests"] (pyproject.toml), so CI/default pytest runs will not collect them. That means the new config-driven + concurrency behavior may not actually be covered in the main test suite unless tests are moved under tests/ (or pytest config is updated).
"""Smoke tests for recommendation_contract + ledger + formatter.
Run: python -m pytest autopilot/tests/test_recommendation_contract.py -q
or: python autopilot/tests/test_recommendation_contract.py
"""
Comment on lines
49
to
+53
| with open(path, "r", encoding="utf-8") as f: | ||
| for line in f: | ||
| line = line.strip() | ||
| if line: | ||
| try: | ||
| out.append(json.loads(line)) | ||
| except json.JSONDecodeError: | ||
| continue | ||
| if not line: | ||
| continue |
Comment on lines
+81
to
+86
| """Append a JSON line under an exclusive file lock. | ||
|
|
||
| Holds the lock across _next_seq + write when called via _append_locked | ||
| so concurrent writers can't collide on sequence numbers or interleave | ||
| large entries. | ||
| """ |
Comment on lines
+146
to
+163
| import config_loader | ||
| config_loader.reset_cache() | ||
| cfg = tempfile.NamedTemporaryFile( | ||
| mode="w", suffix=".yaml", delete=False, encoding="utf-8") | ||
| cfg.write("message_contract:\n max_step_len: 20\n") | ||
| cfg.close() | ||
| try: | ||
| config_loader.DEFAULT_CONFIG_PATH = cfg.name | ||
| config_loader.reset_cache() | ||
| r = _good() | ||
| r.steps[0] = "x" * 25 # over the custom 20 limit, under default 120 | ||
| ok, errs = validate(r) | ||
| assert not ok | ||
| assert any("step 1" in e and "20" in e for e in errs) | ||
| finally: | ||
| config_loader.reset_cache() | ||
| os.unlink(cfg.name) | ||
|
|
Comment on lines
+167
to
+191
| import config_loader | ||
| config_loader.reset_cache() | ||
| cfg = tempfile.NamedTemporaryFile( | ||
| mode="w", suffix=".yaml", delete=False, encoding="utf-8") | ||
| cfg.write( | ||
| "recommendation_debounce:\n" | ||
| " repost_policy:\n" | ||
| " open: {min_hours_between: 100000, never_repost: false}\n" | ||
| ) | ||
| cfg.close() | ||
| ledger = tempfile.NamedTemporaryFile(suffix=".jsonl", delete=False) | ||
| ledger.close() | ||
| try: | ||
| config_loader.DEFAULT_CONFIG_PATH = cfg.name | ||
| config_loader.reset_cache() | ||
| r = _good() | ||
| from decisions.ledger import record, should_post | ||
| record(r, ledger.name) | ||
| allow, reason = should_post(r, ledger.name) | ||
| assert allow is False, reason # 100000h window always suppresses | ||
| finally: | ||
| config_loader.reset_cache() | ||
| os.unlink(cfg.name) | ||
| os.unlink(ledger.name) | ||
|
|
Open
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.
Deferred review fixes from #180 / #188
Closes out the major/minor findings the dual code review flagged but were deferred from #188.
1. Config drift eliminated (major)
config.yamlblocks were declared but never read —ledger.py/recommendation_contract.py/message_formatter.pyhardcoded everything. Newconfig_loader.pyreadsconfig.yamlvia PyYAML (with stdlib fallback if PyYAML isn't installed), so debounce windows, status tags,max_step_len, and therequire_*severity sets are now config-driven. Operators can tune behaviour without code changes.2. Concurrent-writer safety (major)
The recommender (
record) and executor (transition) agents write to the same ledger. Previously_next_seqraced and large appends could interleave, losing entries or colliding seqs. New_append_with_seq()holds an exclusivefcntl.flockacross seq-assignment + write, withfsyncper write. POSIX-only; no-op fallback on non-fcntl platforms.3. Status validation (minor)
transition()now raisesValueErroron unknown statuses (e.g.assigedtypo) viaVALID_STATUSES, so a typo can't silently break de-dup by creating an unmatchable ledger entry.Tests
14/14 passing. New tests:
test_config_overrides_max_step_len— custom config lowers the step captest_config_overrides_debounce_window— custom config widens the suppress windowtest_transition_rejects_unknown_status— typo raises ValueErrortest_concurrent_writers_do_not_lose_entries— 8 threads × 25 writes, no lost entries, no seq collisionsFiles
autopilot/config_loader.py(new) — config reader with stdlib fallbackautopilot/decisions/ledger.py— fcntl locking, config-driven debounce, status validationautopilot/recommendation_contract.py— config-driven validate()autopilot/message_formatter.py— config-driven status tagsautopilot/tests/test_recommendation_contract.py— 14 testsautopilot/README.md— updated file listingRefs #180 #188