Skip to content

Commit c48b4c9

Browse files
committed
chore: enforce import placement via pre-commit, pre-push, and CI
AGENTS.md has required all imports at the top of the file for a long time, but nothing checked it, so function-level imports accumulated. check_import_placement.py fails a nested import unless a comment explains why (>=4 words; noqa-style directives do not count), and treats a try/except ImportError guard as self-explanatory. Also fixes every existing violation, so the repository starts clean and any future failure is something the change introduced.
1 parent 7eff235 commit c48b4c9

25 files changed

Lines changed: 356 additions & 40 deletions

File tree

.github/workflows/code-style.yml

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
name: Code Style
2+
3+
# Enforces the Chronicle-specific style rules that pre-commit runs locally, so
4+
# a `git push --no-verify` cannot land them.
5+
6+
on:
7+
pull_request:
8+
paths:
9+
- "**/*.py"
10+
- ".github/workflows/code-style.yml"
11+
push:
12+
branches: [dev, main]
13+
workflow_dispatch:
14+
15+
permissions:
16+
contents: read
17+
18+
jobs:
19+
import-placement:
20+
name: Imports at top of file
21+
runs-on: ubuntu-latest
22+
timeout-minutes: 5
23+
steps:
24+
- uses: actions/checkout@v4
25+
- uses: actions/setup-python@v5
26+
with:
27+
python-version: "3.12"
28+
- name: Check nested imports are explained
29+
run: python3 scripts/check_import_placement.py

.pre-commit-config.yaml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,17 @@
1+
default_install_hook_types: [pre-commit, pre-push]
2+
13
repos:
4+
# Code style rules that are Chronicle-specific (AGENTS.md → Code Style)
5+
- repo: local
6+
hooks:
7+
- id: import-placement
8+
name: imports at top of file (or explained)
9+
entry: python3 scripts/check_import_placement.py
10+
language: system
11+
types: [python]
12+
exclude: \.venv/
13+
stages: [pre-commit, pre-push]
14+
215
# Code formatting
316
- repo: https://github.com/psf/black
417
rev: 26.5.1

AGENTS.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -607,6 +607,19 @@ tailscale ip -4
607607
- ALL imports must be at the top of the file after the docstring
608608
- Use lazy imports sparingly and only when absolutely necessary for circular import issues
609609
- Group imports: standard library, third-party, local imports
610+
- **Enforced** by `scripts/check_import_placement.py`, which runs as a pre-commit
611+
and pre-push hook and in the `Code Style` CI workflow. An import nested inside a
612+
function or class fails the check unless a comment on the same line, or directly
613+
above it, explains why (≥4 words; `# noqa`-style directives don't count). One
614+
comment covers a contiguous run of imports:
615+
```python
616+
def build_router():
617+
# Imported here to break the circular import with plugins.router.
618+
from advanced_omi_backend.plugins.router import PluginRouter
619+
```
620+
An import guarded by `try/except ImportError` needs no comment — the structure
621+
already says "optional dependency". The repository is currently clean, so any
622+
failure is something the change introduced.
610623
- **Error Handling Guidelines**:
611624
- **Always raise errors, never silently ignore**: Use explicit error handling with proper exceptions rather than silent failures
612625
- **Understand data structures**: Research and understand input/response or class structure instead of adding defensive `hasattr()` checks

backends/advanced/scripts/evaluate_memory_executor.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,7 @@ def _frontmatter(text: str) -> tuple[dict[str, Any] | None, str | None]:
442442
if len(boundaries) < 2:
443443
return None, "missing YAML frontmatter"
444444
try:
445+
# Soft dependency: the except below covers hosts without it.
445446
import yaml
446447

447448
value = yaml.safe_load(text[boundaries[0].end() : boundaries[1].start()]) or {}
@@ -668,6 +669,7 @@ def _safe_runtime_metadata(executor: str, modules: Mapping[str, Any]) -> dict[st
668669
metadata: dict[str, Any] = {"operation": "memory_write"}
669670
if executor == "direct":
670671
try:
672+
# Soft dependency: the except below covers hosts without it.
671673
from advanced_omi_backend.model_registry import get_models_registry
672674

673675
registry = get_models_registry()
@@ -693,6 +695,7 @@ def _safe_runtime_metadata(executor: str, modules: Mapping[str, Any]) -> dict[st
693695
{"executor_available": bool(available), "executor_detail": str(detail)}
694696
)
695697
try:
698+
# Soft dependency: the except below covers hosts without it.
696699
from advanced_omi_backend.model_registry import get_models_registry
697700

698701
registry = get_models_registry()
@@ -747,6 +750,7 @@ def _load_agent(executor: str, args: argparse.Namespace) -> tuple[type, dict[str
747750
backend = Path(__file__).resolve().parents[1]
748751
sys.path.insert(0, str(backend / "src"))
749752

753+
# Imported here: backends/advanced/src only joins sys.path at runtime (above).
750754
from advanced_omi_backend.services.memory import vault_lock
751755
from advanced_omi_backend.services.memory.agent import vault_tools
752756
from advanced_omi_backend.services.memory.agent.memory_agent import MemoryAgent
@@ -759,12 +763,14 @@ def _load_agent(executor: str, args: argparse.Namespace) -> tuple[type, dict[str
759763
if executor == "direct":
760764
agent_type = MemoryAgent
761765
elif executor == "pi":
766+
# Imported here: backends/advanced/src only joins sys.path at runtime (above).
762767
from advanced_omi_backend.services.memory.agent import pi_agent
763768

764769
pi_agent.vault_run_lock = _isolated_vault_lock
765770
modules["pi_agent"] = pi_agent
766771
agent_type = pi_agent.PiMemoryAgent
767772
else:
773+
# Imported here: backends/advanced/src only joins sys.path at runtime (above).
768774
from advanced_omi_backend.services.memory.agent import codex_agent
769775

770776
configured = codex_agent._validated_codex_settings()
@@ -796,6 +802,7 @@ async def _run_case(
796802
expected: Mapping[str, BenchmarkCase],
797803
initial_scaffold: Mapping[str, Any],
798804
) -> dict[str, Any]:
805+
# Imported here: backends/advanced/src only joins sys.path at runtime (above).
799806
from advanced_omi_backend.services.memory.conversation_note import (
800807
ConversationNoteError,
801808
canonicalize_conversation_note,
@@ -947,6 +954,7 @@ async def _run(args: argparse.Namespace) -> int:
947954

948955
backend = Path(__file__).resolve().parents[1]
949956
sys.path.insert(0, str(backend / "src"))
957+
# Imported here: backends/advanced/src only joins sys.path at runtime (above).
950958
from advanced_omi_backend.services.memory.vault_scaffold import seed_vault_scaffold
951959

952960
seed_vault_scaffold(vault)

backends/advanced/scripts/evaluate_memory_retrieval.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,7 @@ def _runtime_metadata(executor: str, pi_module: Any = None) -> dict[str, Any]:
317317
metadata: dict[str, Any] = {"operation": "memory_search"}
318318
if executor == "direct":
319319
try:
320+
# Soft dependency: the except below covers hosts without it.
320321
from advanced_omi_backend.model_registry import get_models_registry
321322

322323
registry = get_models_registry()
@@ -342,6 +343,7 @@ def _runtime_metadata(executor: str, pi_module: Any = None) -> dict[str, Any]:
342343
{"executor_available": bool(available), "executor_detail": str(detail)}
343344
)
344345
try:
346+
# Soft dependency: the except below covers hosts without it.
345347
from advanced_omi_backend.model_registry import get_models_registry
346348

347349
registry = get_models_registry()
@@ -375,9 +377,11 @@ def load_search_executor(
375377
backend = Path(__file__).resolve().parents[1]
376378
sys.path.insert(0, str(backend / "src"))
377379
if executor == "direct":
380+
# Imported here: backends/advanced/src only joins sys.path at runtime (above).
378381
from advanced_omi_backend.services.memory.agent.memory_agent import search_vault
379382

380383
return search_vault, _runtime_metadata(executor)
384+
# Imported here: backends/advanced/src only joins sys.path at runtime (above).
381385
from advanced_omi_backend.services.memory.agent import pi_agent
382386

383387
return pi_agent.search_vault_with_pi, _runtime_metadata(executor, pi_agent)

backends/advanced/scripts/tests/test_memory_executor_benchmark.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
import pytest
1313

14+
from advanced_omi_backend import model_registry
15+
1416
SCRIPTS = Path(__file__).resolve().parents[1]
1517
sys.path.insert(0, str(SCRIPTS))
1618
sys.path.insert(0, str(SCRIPTS.parent / "src"))
@@ -392,8 +394,6 @@ async def run(self, _transcript, source_id, **_kwargs):
392394

393395

394396
def test_pi_runtime_metadata_uses_resolved_override_and_effective_limits(monkeypatch):
395-
from advanced_omi_backend import model_registry
396-
397397
ambient_operation = SimpleNamespace(
398398
model_name="ambient-memory-write-model",
399399
model_provider="ambient-provider",
@@ -432,8 +432,6 @@ def test_pi_runtime_metadata_uses_resolved_override_and_effective_limits(monkeyp
432432

433433

434434
def test_codex_runtime_metadata_uses_actual_codex_model(monkeypatch):
435-
from advanced_omi_backend import model_registry
436-
437435
ambient_operation = SimpleNamespace(
438436
model_name="ambient-memory-write-model",
439437
model_provider="ambient-provider",

backends/advanced/scripts/tests/test_memory_retrieval_benchmark.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
import pytest
1313

14+
from advanced_omi_backend import model_registry
15+
1416
SCRIPTS = Path(__file__).resolve().parents[1]
1517
sys.path.insert(0, str(SCRIPTS))
1618

@@ -62,8 +64,6 @@ def test_load_questions_requires_unique_ids(tmp_path):
6264
def test_direct_runtime_metadata_records_effective_operation_without_secrets(
6365
monkeypatch,
6466
):
65-
from advanced_omi_backend import model_registry
66-
6767
operation = SimpleNamespace(
6868
model_name="qwen-upstream",
6969
model_def=SimpleNamespace(
@@ -102,8 +102,6 @@ def test_direct_runtime_metadata_records_effective_operation_without_secrets(
102102

103103

104104
def test_pi_runtime_metadata_records_resolved_override_without_secrets(monkeypatch):
105-
from advanced_omi_backend import model_registry
106-
107105
registry = object()
108106
monkeypatch.setattr(model_registry, "get_models_registry", lambda: registry)
109107
resolved = SimpleNamespace(

backends/advanced/worker_healthcheck.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ def main() -> int:
4040
# 1. Fresh RQ worker registrations. Redis can retain registrations from dead
4141
# containers, so Worker.all() alone is not a liveness signal.
4242
try:
43+
# Soft dependency: this healthcheck must still run without rq installed.
4344
from rq import Worker
4445

4546
rq_count = sum(

clients.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import json
2121
import os
2222
import plistlib
23+
import shlex
2324
import shutil
2425
import socket
2526
import subprocess
@@ -226,7 +227,6 @@ def _install_linux(name: str, extras=()) -> None:
226227
raise RuntimeError(
227228
"no systemd user instance — on WSL set systemd=true in /etc/wsl.conf"
228229
)
229-
import shlex
230230

231231
uv = _find_uv()
232232
project = _component_project(name)

extras/asr-services/tests/test_batching.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
"""
1616

1717
import difflib
18+
import json
1819
import os
1920
import sys
2021
import tempfile
@@ -28,6 +29,8 @@
2829
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
2930

3031
from common.batching import (
32+
_clip_segments,
33+
_clip_words,
3134
extract_context_tail,
3235
split_audio_file,
3336
stitch_transcription_results,
@@ -465,7 +468,6 @@ def test_words_clipped_at_boundary(self):
465468

466469
def test_clip_helpers_drop_degenerate(self):
467470
"""Segments/words that become start >= end after clipping are dropped."""
468-
from common.batching import _clip_segments, _clip_words
469471

470472
segs = [
471473
Segment(text="survives", start=10.0, end=20.0),
@@ -550,6 +552,8 @@ class TestBatchedTranscriptionQuality:
550552
@pytest.fixture(scope="class")
551553
def transcriber(self):
552554
"""Load VibeVoice model once for all tests in this class."""
555+
# Imported here so the module does not require torch: only this GPU-only
556+
# class needs the VibeVoice provider.
553557
from providers.vibevoice.transcriber import VibeVoiceTranscriber
554558

555559
t = VibeVoiceTranscriber()
@@ -693,8 +697,6 @@ def ground_truth(self):
693697
f"Ground truth fixture not found: {self.FIXTURE_PATH}\n"
694698
f"Run: uv run python tests/capture_vibevoice_ground_truth.py"
695699
)
696-
import json
697-
698700
with open(self.FIXTURE_PATH) as f:
699701
return json.load(f)
700702

0 commit comments

Comments
 (0)