refactor: drain no_magic_numbers baseline to zero via Final hoists (#1856 phase 2)#1872
Conversation
- Remove 55 # lint-allow: magic-numbers markers (now 0 in src/) - Phase 1's named-constant gate already allowlists module-level : int|float|Final|Final[int]|Final[float] = literal, making most markers redundant - Add Final annotation to three previously-untyped module constants (_DEFAULT_WINDOW_SECONDS in loop_prevention/rate_limit.py, _STATE_TOKEN_PREFIX_LENGTH in oauth/state_service.py, _SUBWORKFLOW_KEYSET_ARITY in workflow/subworkflow_registry.py) - Annotate _DEFAULT_TIMEOUT_CHECK_INTERVAL_SECONDS in api/app.py - Convert function-parameter literal defaults to named-Final references (_DEFAULT_TOP_N, _DEFAULT_MIN_DEPLOYMENTS, _DEFAULT_RECENT_OUTCOMES_LIMIT, _DEFAULT_MAX_RESULTS, _DEFAULT_PAGE_SIZE, _DEFAULT_CHUNK_SIZE_WORDS, _DEFAULT_MAX_WORKERS_PER_QUERY, _DEFAULT_MAX_RETRY_COUNT, _DEFAULT_FORCE_FLUSH_TIMEOUT_SECONDS) - Reuse persistence._shared.DEFAULT_LIST_LIMIT for the two approval repos' list_items default - Baseline auto-shrinks 513 to 499 (Final annotations + named-reference defaults satisfy the gate without needing a baseline entry) Refs #1856 (phase 2)
Section 3 (meta + adjacent) of the magic-numbers drain. Per the user's
'default to Final invariant unless clearly operator-tunable' rule,
converts module-level untyped constants to the Final[int|float] shape
the gate already accepts, and hoists function-default literals to
named module-level Final references.
Files touched:
- meta/rules/builtin.py: 10 rule thresholds become _DEFAULT_<NAME>
module constants referenced from constructor defaults
- meta/rollout/{ab_test,ab_comparator,regression/welch}.py:
significance level / sample minimums / improvement threshold
become module-level Final
- meta/chief_of_staff/learning.py: EMA + Bayesian adjuster defaults
become module-level Final
- meta/telemetry/emitter.py: retry / backoff / HTTP-status bounds
become Final[int|float]
- core/personality.py: Big Five compatibility weights become
Final[float] (sum to 1.0 invariant preserved)
- memory/consolidation/density.py: signal weights + tolerance become
Final[float]
- memory/consolidation/service.py: enforce-batch bounds Final[int]
- memory/retrieval_config.py: 7 retrieval defaults Final
- memory/embedding/fine_tune.py: 7 stage defaults hoisted to
module-level Final
- communication/event_stream/stream.py: hub queue + dedup +
janitor defaults Final
- communication/bus/_nats_history.py: history scan batch + timeout
hoisted to two shared module Finals (used by 3 functions)
- engine/classification/detectors.py: minimum-text + drift
thresholds Final
- providers/health.py: health window + thresholds Final
- persistence/ontology_protocol.py: Protocol method limit defaults
reuse persistence._shared.DEFAULT_LIST_LIMIT
- observability/http_handler.py: HttpBatchHandler ctor defaults
hoisted to module-level Finals
Refs #1856 (phase 2)
Section 3 continued: integrations/health, hr/training/curation, hr/performance/composite_quality_strategy, engine/trajectory, engine/shutdown_strategies, api/controllers/oauth, security/llm_evaluator. All literal-RHS module-level constants now carry the Final[int|float] annotation the magic-numbers gate accepts; structural_erosion's four window_size function defaults reference a single shared module-level _DEFAULT_WINDOW_SIZE constant. Refs #1856 (phase 2)
…onstants Replaces every 'limit: int = N' default in persistence/ Protocol and repository methods with a named reference: - limit=100 sites (~59) reference persistence._shared.DEFAULT_LIST_LIMIT - limit=50 / =200 / =10 / =5 sites use a per-file _DEFAULT_LIST_LIMIT_<N> module-level Final constant Affects ~50 files across persistence/, persistence/postgres/, and persistence/sqlite/. Behaviour is unchanged: the same numeric value reaches the SQL LIMIT clause; the literal just lives in a named slot the magic-numbers gate accepts. Refs #1856 (phase 2)
Phase 2 final cleanup. The baseline shrinks from 499 stale + live entries to a single entry at memory/fine_tune_plan.py:40 where the constant uses Literal[2] = 2 (value-encoded-in-type, deliberate). The gate accepts Final[int|float] but not Literal[N]; keeping the Literal form preserves the type-level singleton-value guarantee and leaves a single line in the baseline. Drains: 513 to 1 (99.8% shrinkage). Refs #1856 (phase 2)
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (18)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
🧰 Additional context used📓 Path-based instructions (4)src/synthorg/!(persistence)/**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/synthorg/**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/**/*.py⚙️ CodeRabbit configuration file
Files:
src/synthorg/**/provider*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (1)📚 Learning: 2026-05-05T09:04:46.195ZApplied to files:
🔇 Additional comments (33)
WalkthroughThis update eliminates all magic-number-style inline configuration defaults by introducing named, typed module-level constants using |
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
Merging this PR will not alter performance
Comparing 
Footnotes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/synthorg/persistence/connection_protocol.py (1)
35-43:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDocstring default is now stale relative to the updated signature.
limitnow defaults toDEFAULT_LIST_LIMIT, but the docstring still sayslimit=None (default)with legacy fetch-all semantics. Please update the docstring (or type/default) so the protocol contract is self-consistent.🤖 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 `@src/synthorg/persistence/connection_protocol.py` around lines 35 - 43, The docstring for the function with signature "(limit: int = DEFAULT_LIST_LIMIT, offset: int = 0) -> tuple[Connection, ...]" is inconsistent: it still claims "limit=None (default)" while the code now defaults to DEFAULT_LIST_LIMIT. Update the docstring to reflect the current contract (e.g., state that limit defaults to DEFAULT_LIST_LIMIT and therefore returns at most that many connections unless callers pass a different positive int; keep the notes that limit <= 0 returns () and negative offset is treated as 0), or alternatively change the signature to accept Optional[int] and default limit=None if you want to retain legacy fetch-all semantics—make the docstring and the signature/type consistent with whichever behavior you choose.src/synthorg/persistence/task_protocol.py (1)
46-57:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlign
list_tasksdocs with the non-optionalintlimit contract.
limitis typed asintwith a concrete default, but the docstring saysNoneis a supported meaning. That contract is contradictory and can cause incorrect caller assumptions.Suggested doc fix
- limit: Maximum rows to return. ``None`` means "no - repository-level cap" (the caller remains free to - impose a safety cap above). + limit: Maximum rows to return. + Callers may pass a larger value when needed.🤖 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 `@src/synthorg/persistence/task_protocol.py` around lines 46 - 57, The docstring for list_tasks is inconsistent with its signature: limit is a non-optional int with default DEFAULT_LIST_LIMIT but the text says None means "no repository-level cap"; fix by updating the docstring to remove the "None" semantics and instead describe that limit is an int defaulting to DEFAULT_LIST_LIMIT (i.e., maximum rows to return, with callers free to impose higher-level caps), referencing the limit parameter and DEFAULT_LIST_LIMIT in the explanation so the contract matches the list_tasks signature.src/synthorg/persistence/cost_record_protocol.py (1)
30-41:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix
query()docstring:Noneis no longer the default (or typed).The method now declares
limit: int = DEFAULT_LIST_LIMIT, but docs still describeNone (default)semantics. Please update the contract text to match the callable signature.Suggested doc fix
- limit: Maximum rows to return; ``None`` (default) preserves - fetch-all semantics. - offset: Rows to skip before applying *limit*; ignored when - *limit* is ``None``. + limit: Maximum rows to return. + offset: Rows to skip before applying ``limit``.🤖 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 `@src/synthorg/persistence/cost_record_protocol.py` around lines 30 - 41, The docstring for the query() method no longer matches its signature: limit is typed as int with default DEFAULT_LIST_LIMIT (not None). Update the Args text for the query() method in CostRecordProtocol/CostRecord to remove any reference to "None (default)/fetch-all semantics" and instead state that limit is an integer defaulting to DEFAULT_LIST_LIMIT which caps the number of rows returned, and that offset is the number of rows to skip before applying that limit (i.e., offset is ignored only if limit is not used or set to a value that indicates "no rows", adjust wording to match actual implementation).
🤖 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 `@src/synthorg/engine/loop_selector.py`:
- Around line 228-237: AutoLoopConfig currently hardcodes 80 for
budget_tight_threshold while select_loop_type uses
_DEFAULT_BUDGET_TIGHT_THRESHOLD; update AutoLoopConfig.budget_tight_threshold to
reference the module-level constant _DEFAULT_BUDGET_TIGHT_THRESHOLD instead of
the literal 80 so both call sites share the same named constant (keep the
constant's annotation and value as-is and import/qualify it if needed).
In `@src/synthorg/integrations/oauth/flows/device_flow.py`:
- Around line 26-27: The response parser fallback currently uses the hardcoded
literal 600; change that fallback to use the module constant _DEFAULT_EXPIRES_IN
instead so there is a single source of truth (keep _DEFAULT_MAX_WAIT_SECONDS
as-is). Locate the device flow response parsing code that sets the expires_in
fallback (the branch that uses the literal 600) and replace the literal with
_DEFAULT_EXPIRES_IN, ensuring the constant is in scope for that function.
---
Outside diff comments:
In `@src/synthorg/persistence/connection_protocol.py`:
- Around line 35-43: The docstring for the function with signature "(limit: int
= DEFAULT_LIST_LIMIT, offset: int = 0) -> tuple[Connection, ...]" is
inconsistent: it still claims "limit=None (default)" while the code now defaults
to DEFAULT_LIST_LIMIT. Update the docstring to reflect the current contract
(e.g., state that limit defaults to DEFAULT_LIST_LIMIT and therefore returns at
most that many connections unless callers pass a different positive int; keep
the notes that limit <= 0 returns () and negative offset is treated as 0), or
alternatively change the signature to accept Optional[int] and default
limit=None if you want to retain legacy fetch-all semantics—make the docstring
and the signature/type consistent with whichever behavior you choose.
In `@src/synthorg/persistence/cost_record_protocol.py`:
- Around line 30-41: The docstring for the query() method no longer matches its
signature: limit is typed as int with default DEFAULT_LIST_LIMIT (not None).
Update the Args text for the query() method in CostRecordProtocol/CostRecord to
remove any reference to "None (default)/fetch-all semantics" and instead state
that limit is an integer defaulting to DEFAULT_LIST_LIMIT which caps the number
of rows returned, and that offset is the number of rows to skip before applying
that limit (i.e., offset is ignored only if limit is not used or set to a value
that indicates "no rows", adjust wording to match actual implementation).
In `@src/synthorg/persistence/task_protocol.py`:
- Around line 46-57: The docstring for list_tasks is inconsistent with its
signature: limit is a non-optional int with default DEFAULT_LIST_LIMIT but the
text says None means "no repository-level cap"; fix by updating the docstring to
remove the "None" semantics and instead describe that limit is an int defaulting
to DEFAULT_LIST_LIMIT (i.e., maximum rows to return, with callers free to impose
higher-level caps), referencing the limit parameter and DEFAULT_LIST_LIMIT in
the explanation so the contract matches the list_tasks signature.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 507c6bce-0334-4a95-b7f2-86b04351f782
📒 Files selected for processing (300)
scripts/no_magic_numbers_baseline.txtsrc/synthorg/a2a/connection_types/a2a_peer.pysrc/synthorg/a2a/gateway.pysrc/synthorg/a2a/push_verifier.pysrc/synthorg/api/app.pysrc/synthorg/api/auth/middleware.pysrc/synthorg/api/auth/ticket_store.pysrc/synthorg/api/boundary.pysrc/synthorg/api/bus_bridge.pysrc/synthorg/api/controllers/activities.pysrc/synthorg/api/controllers/agent_identity_versions.pysrc/synthorg/api/controllers/agents.pysrc/synthorg/api/controllers/analytics.pysrc/synthorg/api/controllers/approvals.pysrc/synthorg/api/controllers/artifacts.pysrc/synthorg/api/controllers/audit.pysrc/synthorg/api/controllers/backup.pysrc/synthorg/api/controllers/budget.pysrc/synthorg/api/controllers/budget_config_versions.pysrc/synthorg/api/controllers/clients.pysrc/synthorg/api/controllers/company_versions.pysrc/synthorg/api/controllers/connections.pysrc/synthorg/api/controllers/coordination_metrics.pysrc/synthorg/api/controllers/custom_rules.pysrc/synthorg/api/controllers/departments.pysrc/synthorg/api/controllers/escalations.pysrc/synthorg/api/controllers/evaluation_config_versions.pysrc/synthorg/api/controllers/events.pysrc/synthorg/api/controllers/integration_health.pysrc/synthorg/api/controllers/meetings.pysrc/synthorg/api/controllers/messages.pysrc/synthorg/api/controllers/meta.pysrc/synthorg/api/controllers/meta_analytics.pysrc/synthorg/api/controllers/oauth.pysrc/synthorg/api/controllers/ontology.pysrc/synthorg/api/controllers/personalities.pysrc/synthorg/api/controllers/projects.pysrc/synthorg/api/controllers/reports.pysrc/synthorg/api/controllers/requests.pysrc/synthorg/api/controllers/role_versions.pysrc/synthorg/api/controllers/simulations.pysrc/synthorg/api/controllers/tasks.pysrc/synthorg/api/controllers/users.pysrc/synthorg/api/controllers/workflow_executions.pysrc/synthorg/api/controllers/workflow_versions.pysrc/synthorg/api/cursor.pysrc/synthorg/api/dto_discovery.pysrc/synthorg/api/exception_handlers.pysrc/synthorg/api/lifecycle.pysrc/synthorg/api/lifecycle_builder.pysrc/synthorg/api/rate_limits/in_memory.pysrc/synthorg/api/rate_limits/in_memory_inflight.pysrc/synthorg/api/services/org_mutations.pysrc/synthorg/api/services/ssrf_violation_service.pysrc/synthorg/backup/service_archive.pysrc/synthorg/budget/automated_reports.pysrc/synthorg/budget/coordination_metrics.pysrc/synthorg/budget/coordination_store.pysrc/synthorg/budget/enforcer.pysrc/synthorg/budget/optimizer.pysrc/synthorg/budget/quota_tracker.pysrc/synthorg/budget/rebalance.pysrc/synthorg/budget/reports.pysrc/synthorg/budget/risk_tracker.pysrc/synthorg/budget/tracker.pysrc/synthorg/client/feedback/adversarial.pysrc/synthorg/client/feedback/scored.pysrc/synthorg/client/generators/llm.pysrc/synthorg/client/generators/procedural.pysrc/synthorg/communication/bus/_nats_history.pysrc/synthorg/communication/conflict_resolution/escalation/config.pysrc/synthorg/communication/conflict_resolution/escalation/in_memory_store.pysrc/synthorg/communication/conflict_resolution/escalation/notify.pysrc/synthorg/communication/conflict_resolution/escalation/protocol.pysrc/synthorg/communication/conflict_resolution/escalation/sweeper.pysrc/synthorg/communication/conflict_resolution/models.pysrc/synthorg/communication/delegation/record_store.pysrc/synthorg/communication/event_stream/stream.pysrc/synthorg/communication/loop_prevention/dedup.pysrc/synthorg/communication/loop_prevention/rate_limit.pysrc/synthorg/config/rate_limits.pysrc/synthorg/constants.pysrc/synthorg/core/auth/config.pysrc/synthorg/core/concurrency/cas_retry.pysrc/synthorg/core/personality.pysrc/synthorg/engine/agent_engine_post_exec.pysrc/synthorg/engine/approval_gate.pysrc/synthorg/engine/classification/detectors.pysrc/synthorg/engine/classification/loaders.pysrc/synthorg/engine/classification/semantic_detectors.pysrc/synthorg/engine/classification/sinks.pysrc/synthorg/engine/classification/taxonomy_store.pysrc/synthorg/engine/compaction/epistemic.pysrc/synthorg/engine/decomposition/classifier.pysrc/synthorg/engine/evolution/guards/rate_limit.pysrc/synthorg/engine/evolution/guards/rollback.pysrc/synthorg/engine/evolution/proposers/composite.pysrc/synthorg/engine/evolution/proposers/self_report.pysrc/synthorg/engine/evolution/proposers/separate_analyzer.pysrc/synthorg/engine/intake/strategies/agent_intake.pysrc/synthorg/engine/loop_selector.pysrc/synthorg/engine/middleware/behavior_tagger.pysrc/synthorg/engine/middleware/coordination_constraints.pysrc/synthorg/engine/middleware/semantic_drift.pysrc/synthorg/engine/plan_helpers.pysrc/synthorg/engine/quality/decomposers/llm.pysrc/synthorg/engine/routing/scorer.pysrc/synthorg/engine/sanitization.pysrc/synthorg/engine/shutdown.pysrc/synthorg/engine/shutdown_strategies.pysrc/synthorg/engine/stagnation/quality_erosion_detector.pysrc/synthorg/engine/strategy/consensus.pysrc/synthorg/engine/trajectory/budget_guard.pysrc/synthorg/engine/trajectory/structural_erosion.pysrc/synthorg/engine/workflow/condition_eval.pysrc/synthorg/engine/workflow/sprint_velocity.pysrc/synthorg/engine/workflow/subworkflow_registry.pysrc/synthorg/engine/workflow/validate_edges.pysrc/synthorg/engine/workflow/validation_types.pysrc/synthorg/engine/workflow/version_service.pysrc/synthorg/engine/workflow/webhook_bridge.pysrc/synthorg/engine/workspace/disk_quota.pysrc/synthorg/engine/workspace/semantic_checks.pysrc/synthorg/engine/workspace/semantic_llm.pysrc/synthorg/hr/performance/ci_quality_strategy.pysrc/synthorg/hr/performance/composite_quality_strategy.pysrc/synthorg/hr/performance/llm_calibration_sampler.pysrc/synthorg/hr/performance/multi_window_strategy.pysrc/synthorg/hr/performance/quality_override_store.pysrc/synthorg/hr/performance/theil_sen_strategy.pysrc/synthorg/hr/scaling/guards/approval_gate.pysrc/synthorg/hr/scaling/guards/conflict_resolver.pysrc/synthorg/hr/scaling/guards/cooldown.pysrc/synthorg/hr/scaling/guards/rate_limit.pysrc/synthorg/hr/scaling/service.pysrc/synthorg/hr/scaling/signals/workload.pysrc/synthorg/hr/scaling/strategies/budget_cap.pysrc/synthorg/hr/scaling/strategies/workload.pysrc/synthorg/hr/scaling/triggers/batched.pysrc/synthorg/hr/training/curation/llm_curated.pysrc/synthorg/hr/training/curation/relevance.pysrc/synthorg/hr/training/extractors/procedural.pysrc/synthorg/hr/training/extractors/semantic.pysrc/synthorg/hr/training/extractors/tool_patterns.pysrc/synthorg/hr/training/guards/review_gate.pysrc/synthorg/hr/training/guards/sanitization.pysrc/synthorg/hr/training/source_selectors/department_diversity.pysrc/synthorg/hr/training/source_selectors/role_top_performers.pysrc/synthorg/infrastructure/services.pysrc/synthorg/integrations/health/checks/generic_http.pysrc/synthorg/integrations/health/checks/github.pysrc/synthorg/integrations/health/checks/slack.pysrc/synthorg/integrations/health/checks/smtp.pysrc/synthorg/integrations/health/prober.pysrc/synthorg/integrations/mcp_catalog/in_memory_installations.pysrc/synthorg/integrations/mcp_catalog/installations.pysrc/synthorg/integrations/oauth/flows/device_flow.pysrc/synthorg/integrations/oauth/pkce.pysrc/synthorg/integrations/oauth/state_service.pysrc/synthorg/integrations/oauth/token_manager.pysrc/synthorg/integrations/rate_limiting/shared_state.pysrc/synthorg/integrations/tunnel/ngrok_adapter.pysrc/synthorg/integrations/webhooks/replay_protection.pysrc/synthorg/integrations/webhooks/verifiers/slack_signing.pysrc/synthorg/memory/backends/inmemory/adapter.pysrc/synthorg/memory/backends/mem0/adapter.pysrc/synthorg/memory/consolidation/abstractive.pysrc/synthorg/memory/consolidation/config.pysrc/synthorg/memory/consolidation/density.pysrc/synthorg/memory/consolidation/dual_mode_strategy.pysrc/synthorg/memory/consolidation/extractive.pysrc/synthorg/memory/consolidation/service.pysrc/synthorg/memory/consolidation/simple_strategy.pysrc/synthorg/memory/consolidation/two_tier_strategy.pysrc/synthorg/memory/embedding/fine_tune.pysrc/synthorg/memory/fine_tune_plan.pysrc/synthorg/memory/procedural/capture/success_capture.pysrc/synthorg/memory/procedural/propagation/department_scoped.pysrc/synthorg/memory/procedural/propagation/role_scoped.pysrc/synthorg/memory/procedural/pruning/pareto_strategy.pysrc/synthorg/memory/procedural/pruning/ttl_strategy.pysrc/synthorg/memory/procedural/trajectory_aggregator.pysrc/synthorg/memory/ranking.pysrc/synthorg/memory/retrieval/hierarchical/supervisor.pysrc/synthorg/memory/retrieval/hierarchical/workers.pysrc/synthorg/memory/retrieval/reranking/cache.pysrc/synthorg/memory/retrieval/reranking/llm_reranker.pysrc/synthorg/memory/retrieval_config.pysrc/synthorg/memory/sparse.pysrc/synthorg/memory/tools/_args.pysrc/synthorg/meta/analytics/service.pysrc/synthorg/meta/appliers/github_client.pysrc/synthorg/meta/appliers/prompt_applier.pysrc/synthorg/meta/chief_of_staff/inflection.pysrc/synthorg/meta/chief_of_staff/learning.pysrc/synthorg/meta/chief_of_staff/monitor.pysrc/synthorg/meta/chief_of_staff/outcome_store.pysrc/synthorg/meta/chief_of_staff/protocol.pysrc/synthorg/meta/evolution/outcome_store.pysrc/synthorg/meta/evolution/outcome_store_protocol.pysrc/synthorg/meta/guards/approval_gate.pysrc/synthorg/meta/guards/rate_limit.pysrc/synthorg/meta/mcp/handlers/analytics.pysrc/synthorg/meta/mcp/handlers/common_args.pysrc/synthorg/meta/reports/service.pysrc/synthorg/meta/rollout/ab_comparator.pysrc/synthorg/meta/rollout/ab_models.pysrc/synthorg/meta/rollout/ab_test.pysrc/synthorg/meta/rollout/before_after.pysrc/synthorg/meta/rollout/canary.pysrc/synthorg/meta/rollout/regression/statistical.pysrc/synthorg/meta/rollout/regression/welch.pysrc/synthorg/meta/rules/builtin.pysrc/synthorg/meta/strategies/code_modification.pysrc/synthorg/meta/telemetry/aggregator.pysrc/synthorg/meta/telemetry/collector.pysrc/synthorg/meta/telemetry/emitter.pysrc/synthorg/meta/telemetry/protocol.pysrc/synthorg/meta/telemetry/recommender.pysrc/synthorg/meta/validation/ci_validator.pysrc/synthorg/notifications/adapters/email.pysrc/synthorg/notifications/adapters/ntfy.pysrc/synthorg/notifications/adapters/slack.pysrc/synthorg/observability/audit_chain/tsa_client.pysrc/synthorg/observability/background_tasks.pysrc/synthorg/observability/http_handler.pysrc/synthorg/observability/otlp_handler.pysrc/synthorg/observability/otlp_trace_handler.pysrc/synthorg/observability/tracing/protocol.pysrc/synthorg/ontology/drift/active.pysrc/synthorg/ontology/drift/passive.pysrc/synthorg/ontology/injection/hybrid.pysrc/synthorg/ontology/injection/prompt.pysrc/synthorg/ontology/service.pysrc/synthorg/persistence/approval_protocol.pysrc/synthorg/persistence/artifact_protocol.pysrc/synthorg/persistence/atlas.pysrc/synthorg/persistence/audit_protocol.pysrc/synthorg/persistence/auth_protocol.pysrc/synthorg/persistence/connection_protocol.pysrc/synthorg/persistence/cost_record_protocol.pysrc/synthorg/persistence/decision_protocol.pysrc/synthorg/persistence/fine_tune_protocol.pysrc/synthorg/persistence/integration_stubs.pysrc/synthorg/persistence/jsonb_capability.pysrc/synthorg/persistence/mcp_protocol.pysrc/synthorg/persistence/memory_protocol.pysrc/synthorg/persistence/message_protocol.pysrc/synthorg/persistence/ontology_protocol.pysrc/synthorg/persistence/postgres/approval_repo.pysrc/synthorg/persistence/postgres/artifact_repo.pysrc/synthorg/persistence/postgres/audit_repository.pysrc/synthorg/persistence/postgres/connection_repo.pysrc/synthorg/persistence/postgres/decision_repo.pysrc/synthorg/persistence/postgres/escalation_repo.pysrc/synthorg/persistence/postgres/fine_tune_repo.pysrc/synthorg/persistence/postgres/mcp_installation_repo.pysrc/synthorg/persistence/postgres/ontology_drift_repo.pysrc/synthorg/persistence/postgres/ontology_entity_repo.pysrc/synthorg/persistence/postgres/org_fact_repo.pysrc/synthorg/persistence/postgres/provider_audit_repo.pysrc/synthorg/persistence/postgres/repositories.pysrc/synthorg/persistence/postgres/session_repo.pysrc/synthorg/persistence/postgres/settings_repo.pysrc/synthorg/persistence/postgres/ssrf_violation_repo.pysrc/synthorg/persistence/postgres/version_repo.pysrc/synthorg/persistence/postgres/webhook_receipt_repo.pysrc/synthorg/persistence/provider_audit_protocol.pysrc/synthorg/persistence/settings_protocol.pysrc/synthorg/persistence/sqlite/approval_repo.pysrc/synthorg/persistence/sqlite/artifact_repo.pysrc/synthorg/persistence/sqlite/audit_repository.pysrc/synthorg/persistence/sqlite/connection_repo.pysrc/synthorg/persistence/sqlite/decision_repo.pysrc/synthorg/persistence/sqlite/escalation_repo.pysrc/synthorg/persistence/sqlite/fine_tune_repo.pysrc/synthorg/persistence/sqlite/mcp_installation_repo.pysrc/synthorg/persistence/sqlite/ontology_drift_repo.pysrc/synthorg/persistence/sqlite/ontology_entity_repo.pysrc/synthorg/persistence/sqlite/org_fact_repo.pysrc/synthorg/persistence/sqlite/provider_audit_repo.pysrc/synthorg/persistence/sqlite/repositories.pysrc/synthorg/persistence/sqlite/session_repo.pysrc/synthorg/persistence/sqlite/settings_repo.pysrc/synthorg/persistence/sqlite/ssrf_violation_repo.pysrc/synthorg/persistence/sqlite/version_repo.pysrc/synthorg/persistence/sqlite/webhook_receipt_repo.pysrc/synthorg/persistence/ssrf_violation_protocol.pysrc/synthorg/persistence/task_protocol.pysrc/synthorg/persistence/version_protocol.pysrc/synthorg/providers/drivers/litellm_driver.pysrc/synthorg/providers/health.pysrc/synthorg/providers/health_prober.pysrc/synthorg/providers/management/_capability_helpers.pysrc/synthorg/providers/management/audit_service.pysrc/synthorg/providers/management/local_models.pysrc/synthorg/security/audit.pysrc/synthorg/security/llm_evaluator.pysrc/synthorg/security/timeout/factory.pysrc/synthorg/security/timeout/policies.py
💤 Files with no reviewable changes (1)
- scripts/no_magic_numbers_baseline.txt
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request systematically replaces magic numbers with named constants and Final type annotations across the entire codebase, significantly reducing the baseline of linting exclusions. The changes improve code readability and maintainability by centralizing default values for timeouts, limits, and thresholds. Feedback from the reviewer highlights multiple opportunities to further improve the implementation by importing shared constants from protocol and configuration files rather than redefining them in repository-specific modules, adhering more strictly to the DRY principle.
| from synthorg.engine.workflow.definition import WorkflowDefinition | ||
|
|
||
| _MIN_SPLIT_BRANCHES = 2 | ||
| _MIN_SPLIT_BRANCHES: Final[int] = 2 |
There was a problem hiding this comment.
The constant _MIN_SPLIT_BRANCHES is also defined in src/synthorg/engine/workflow/validation_types.py. To avoid duplication and improve maintainability, you could import it from there instead of redefining it.
For example:
from synthorg.engine.workflow.validation_types import (
ValidationFinding,
ValidationContext,
_validate_node_exists,
_MIN_SPLIT_BRANCHES,
)| logger = get_logger(__name__) | ||
|
|
||
| _DEFAULT_LIMIT = 50 | ||
| _DEFAULT_LIMIT: Final[int] = 50 |
There was a problem hiding this comment.
The constant _DEFAULT_LIMIT is also defined in src/synthorg/communication/conflict_resolution/escalation/protocol.py. To avoid duplication, you could import it from the protocol file. The same applies to _DEFAULT_OFFSET.
|
|
||
| logger = get_logger(__name__) | ||
|
|
||
| _DEFAULT_LIST_LIMIT_50: Final[int] = 50 |
There was a problem hiding this comment.
The constant _DEFAULT_LIST_LIMIT_50 is also defined in src/synthorg/persistence/fine_tune_protocol.py. To avoid duplication, you could import it from the protocol file.
|
|
||
| logger = get_logger(__name__) | ||
|
|
||
| _DEFAULT_LIST_LIMIT_5: Final[int] = 5 |
There was a problem hiding this comment.
The constant _DEFAULT_LIST_LIMIT_5 is also defined in src/synthorg/persistence/memory_protocol.py. To avoid duplication, you could import it from there.
|
|
||
| logger = get_logger(__name__) | ||
|
|
||
| _DEFAULT_LIST_LIMIT_50: Final[int] = 50 |
There was a problem hiding this comment.
The constant _DEFAULT_LIST_LIMIT_50 is also defined in src/synthorg/persistence/provider_audit_protocol.py. To avoid duplication, you could import it from the protocol file.
|
|
||
| logger = get_logger(__name__) | ||
|
|
||
| _DEFAULT_LIST_LIMIT_50: Final[int] = 50 |
There was a problem hiding this comment.
The constant _DEFAULT_LIST_LIMIT_50 is also defined in src/synthorg/persistence/fine_tune_protocol.py. To avoid duplication, you could import it from the protocol file.
|
|
||
| logger = get_logger(__name__) | ||
|
|
||
| _DEFAULT_LIST_LIMIT_5: Final[int] = 5 |
There was a problem hiding this comment.
The constant _DEFAULT_LIST_LIMIT_5 is also defined in src/synthorg/persistence/memory_protocol.py. To avoid duplication, you could import it from there.
|
|
||
| logger = get_logger(__name__) | ||
|
|
||
| _DEFAULT_LIST_LIMIT_50: Final[int] = 50 |
There was a problem hiding this comment.
The constant _DEFAULT_LIST_LIMIT_50 is also defined in src/synthorg/persistence/provider_audit_protocol.py. To avoid duplication, you could import it from the protocol file.
|
|
||
| logger = get_logger(__name__) | ||
|
|
||
| _DEFAULT_LIST_LIMIT_200: Final[int] = 200 |
There was a problem hiding this comment.
The constant _DEFAULT_LIST_LIMIT_200 is also defined in src/synthorg/persistence/settings_protocol.py. To avoid duplication, you could import it from there.
|
|
||
| logger = get_logger(__name__) | ||
|
|
||
| _DEFAULT_LIST_LIMIT_50: Final[int] = 50 |
There was a problem hiding this comment.
The constant _DEFAULT_LIST_LIMIT_50 is also defined in src/synthorg/persistence/version_protocol.py. To avoid duplication, you could import it from there.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1872 +/- ##
==========================================
+ Coverage 84.89% 84.94% +0.04%
==========================================
Files 1804 1804
Lines 105233 105557 +324
Branches 9214 9214
==========================================
+ Hits 89342 89669 +327
+ Misses 13656 13654 -2
+ Partials 2235 2234 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- loop_selector.py: route AutoLoopConfig.budget_tight_threshold through
_DEFAULT_BUDGET_TIGHT_THRESHOLD (move constant above the class).
- device_flow.py: use _DEFAULT_EXPIRES_IN for the response parser
fallback instead of literal 600.
- connection_protocol.py / task_protocol.py / cost_record_protocol.py:
align list/query docstrings with the int = DEFAULT_LIST_LIMIT signature
(drop stale 'None default' / fetch-all semantics).
- engine/workflow/validate_edges.py: import _MIN_SPLIT_BRANCHES from
validation_types instead of redefining locally.
- persistence/{postgres,sqlite}/{escalation,fine_tune,org_fact,
provider_audit,settings,version}_repo.py: import the
_DEFAULT_LIMIT/_DEFAULT_OFFSET/_DEFAULT_LIST_LIMIT_* constants from
their respective protocol files instead of redefining.
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
<!-- HIGHLIGHTS_START --> ## Highlights > _AI-generated summary (model: `openai/gpt-4.1-mini` via GitHub Models). Commit-based changelog below._ ### What you'll notice - Password and secret fields now include an eye-toggle for easier visibility control. - Containers running without probes are shown as healthy in the doctor command. - Unloaded and missing PR-review agents are restored and available again. ### What's new - Gate baseline protection is enhanced to block em-dashes during writing. ### Under the hood - Replaced Atlas with yoyo-migrations for persistence management. - Refactored codebase extensively, including context-bound user authentication and registry pattern for enums. - Improved linting by draining magic number usages and tightening mock and constant checks. - Updated CI to retry Docker pushes on network timeout errors. - Updated apko lockfiles for dependency management. <!-- HIGHLIGHTS_END --> :robot: I have created a release *beep* *boop* --- ## [0.8.3](v0.8.2...v0.8.3) (2026-05-12) ### Features * harden gate baseline protection + block em-dashes at write time ([#1860](#1860)) ([b41f151](b41f151)) * **web:** eye-toggle on every password / secret field ([#1873](#1873)) ([9070387](9070387)) ### Bug Fixes * **ci:** retry Docker push on Go net/http deadline + cancellation errors ([#1877](#1877)) ([23a0bfa](23a0bfa)) * **cli:** render running-no-probe containers as healthy in doctor ([#1870](#1870)) ([6263795](6263795)) * restore unloaded and missing PR-review agents ([#1875](#1875)) ([db004fd](db004fd)), closes [#1871](#1871) ### Refactoring * bind authenticated user via ContextVar ([#1858](#1858)) ([57ed0b4](57ed0b4)) * code-structure cleanup (sub-tasks D + F + G + H + I) ([#1859](#1859)) ([362e5c8](362e5c8)) * convert enum dispatch to registry pattern ([#1854](#1854)) ([e90550e](e90550e)) * drain no_magic_numbers baseline to zero via Final hoists ([#1856](#1856) phase 2) ([#1872](#1872)) ([ec8109e](ec8109e)) * drain pagination + loop-init + kill-switch baselines ([#1857](#1857)) ([#1868](#1868)) ([115c3c2](115c3c2)) * **persistence:** replace Atlas with yoyo-migrations ([#1876](#1876)) ([1b7e975](1b7e975)), closes [#1874](#1874) * protocols audit follow-up (REVIEW + fold pass) ([#1869](#1869)) ([af33ddb](af33ddb)) * protocols audit follow-up REMOVE pass ([#1867](#1867)) ([dd1eebc](dd1eebc)) * tighten check_mock_spec gate, add mock_of[T], drain baseline ([#1862](#1862)) ([240a253](240a253)) * tighten check_no_magic_numbers for named module constants ([#1856](#1856)) ([#1866](#1866)) ([90c933b](90c933b)) ### CI/CD * update apko lockfiles ([#1863](#1863)) ([2bd32e6](2bd32e6)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: synthorg-repo-bot[bot] <279117679+synthorg-repo-bot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 participant
Summary
Finishes off the work tracked in (closed) issue #1856 by draining
scripts/no_magic_numbers_baseline.txtfrom 499 entries down to 0, exceeding the issue's stretch goal.Phase 1 (#1866) tightened
scripts/check_no_magic_numbers.pyto recognise typed module-level constants as already-named. This phase migrates the remaining real literals into either:Finalconstants (NAME: Final[int|float] = literal) for code-level invariants and bootstrap defaults, orInline
# lint-allow: magic-numbersmarkers insrc/: 50 → 0.What changed
.pyfiles undersrc/synthorg/: inline numeric literals hoisted to module-levelFinalconstantsscripts/no_magic_numbers_baseline.txt: 499 entries → header-only (0 entries)tests/evals/prompt/test_memory_consolidation_prompt.py: contract test relaxed to accept Final references for the consolidation temperature pinNo behavioural changes. No SQL / schema / migration / API / config changes. Every hoisted value was preserved exactly (verified by sampling 40+ files across the diff).
Test plan
uv run ruff check src/ tests/: cleanuv run ruff format src/ tests/: 3712 files unchangeduv run mypy src/ tests/(strict): clean across 3712 filesuv run python -m pytest tests/ -m unit: 28400 passed, 18 skipped (platform-only)uv run python scripts/check_no_magic_numbers.py: exit 0 (no violations against the now-empty baseline)Review coverage
Pre-reviewed by 10 parallel agents. 8 returned clean. 2 surfaced nomenclature suggestions (
_DEFAULT_LIMITrepetition across controllers;_DEFAULT_LIST_LIMIT_50value-in-name in 9 persistence files); reviewed with the author and consciously deferred since the constants are module-private and the patterns are deliberate.Verified clean: value preservation across 40+ files, security-sensitive constants (JWT TTLs, PKCE lengths, OAuth thresholds, webhook clock-skew, rate-limit caps, sandbox config) across 18 files, API contracts across 36 controllers, SQLite/Postgres parity in persistence, docs consistency (CLAUDE.md, convention-gates.md, design pages).