Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions devtools/command_catalog.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
"lab probe pipeline",
"lab probe turso",
"lab projections",
"lab run",
"lab smoke",
"lab schema audit",
"lab schema commit",
Expand Down Expand Up @@ -2149,6 +2150,18 @@ class CatalogBypassSite:
use_when="Assert memory budgets around a concrete query or archive-facing command.",
examples=("devtools bench memory --max-rss-mb 1536 -- polylogue --plain analyze",),
),
CommandSpec(
"lab run",
"verification lab",
"Run a named archive verification scenario.",
"devtools.lab_scenario",
entrypoint="run_main",
use_when="Run a scenario such as rebuild-safety through the direct lab command path.",
examples=(
"devtools lab run rebuild-safety",
"devtools lab run rebuild-safety --report-dir .cache/rebuild-safety-report --json",
),
),
Comment on lines +2153 to +2164

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check how lab smoke and lab run are distinguished, and confirm docs coverage resolves both.
set -euo pipefail

rg -nP -C 3 '"lab smoke"|"lab run"' devtools/command_catalog.py

rg -nP -C 3 'lab smoke|lab run' docs/ -g '*.md' | head -40

Repository: Sinity/polylogue

Length of output: 3367


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== catalog command specs =="
sed -n '2145,2185p' devtools/command_catalog.py

echo "== catalog command list context =="
sed -n '24,38p' devtools/command_catalog.py

echo "== find lab_scenario and command invocation paths =="
rg -n "def run_main|def main|_SCENARIO_NAMES|ArchiveSmoke|RebuildSafety|ArchiveScenario|lab_scenario|CommandSpec" devtools -S

echo "== file sizes =="
wc -l devtools/command_catalog.py devtools/lab_scenario.py

Repository: Sinity/polylogue

Length of output: 29123


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== devtools/lab_scenario.py relevant implementation =="
sed -n '1,120p' devtools/lab_scenario.py
sed -n '220,310p' devtools/lab_scenario.py
sed -n '460,510p' devtools/lab_scenario.py

echo "== devtools command entrypoint resolution =="
sed -n '1,240p' devtools/click_dispatch.py

echo "== docs coverage command name extraction =="
sed -n '180,230p' devtools/verify_docs_coverage.py
rg -n "coverage.*lab|lab run|lab smoke|devtools docs coverage|devtools verify docs-coverage" devtools docs -S -g '*.py' -g '*.md' | head -120

echo "== focused text search for run Main/main mapping in catalog =="
python3 - <<'PY'
from pathlib import Path
p = Path('devtools/lab_scenario.py')
text = p.read_text()
for marker in ['_SCENARIO_NAMES', 'run_parser.add_argument', 'def run_main', 'def main']:
    print(f'-- {marker} --')
    for i,line in enumerate(text.splitlines(),1):
        if marker in line:
            print(f'{i}:{line}')
PY

Repository: Sinity/polylogue

Length of output: 24271


Differentiate lab run from lab smoke in the catalog.

devtools lab run reconstruct-safety ... and devtools lab smoke run archive-smoke ... both resolve to devtools.lab_scenario and accept the same _SCENARIO_NAMES. Update use_when to choose one route, or limit the new command to a narrower owned set. Also update docs/devtools.md so the new description is not duplicated next to lab smoke.

🤖 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 `@devtools/command_catalog.py` around lines 2153 - 2164, Differentiate the
catalog entry for “lab run” from the existing “lab smoke” route by updating the
`CommandSpec` use_when guidance to select only one command path, or constrain
its scenarios to a distinct owned set rather than the shared `_SCENARIO_NAMES`.
Update the corresponding `docs/devtools.md` description so the new command
guidance is not duplicated alongside `lab smoke`.

CommandSpec(
"lab smoke",
"verification lab",
Expand Down
101 changes: 100 additions & 1 deletion devtools/lab_scenario.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,20 @@
import subprocess
import sys
import time
from collections.abc import Callable
from dataclasses import dataclass
from pathlib import Path
from typing import Protocol, TextIO

from devtools import repo_root as _get_root
from devtools.cli_boundary import invoke_polylogue_cli
from devtools.rebuild_safety_scenario import (
REBUILD_DIFFERENTIAL_SCENARIO_NAME,
REBUILD_SAFETY_SCENARIO_NAME,
RebuildComparisonResult,
run_rebuild_differential,
run_rebuild_safety,
)
Comment on lines +17 to +23

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check the import weight of the lab scenario module and the help-latency budget targets.
set -euo pipefail

rg -nP '^\s*(from|import)\s+' devtools/lab_scenario.py | head -40

rg -nP -C 4 '700|cold|budget|required|informational' devtools/help_latency_probe.py | head -60

Repository: Sinity/polylogue

Length of output: 4110


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '== lab_scenario outline/importers ==\n'
ast-grep outline devtools/lab_scenario.py || true
printf '\n== lab_scenario imports and relevant top section ==\n'
sed -n '1,220p' devtools/lab_scenario.py | cat -n | sed -n '/^[[:space:]]*[0-9]\{1,4\}[[:space:]]/p'

printf '\n== rebuild_safety_scenario module outline ==\n'
ast-grep outline devtools/rebuild_safety_scenario.py || true
printf '\n== rebuild_safety_scenario imports/top section ==\n'
sed -n '1,120p' devtools/rebuild_safety_scenario.py | cat -n

printf '\n== help-latency targets mentioning lab or rebuild ==\n'
rg -n 'TARGETS|HelpLatencyTarget|lab|rebuild' devtools/help_latency_probe.py || true

printf '\n== usages of lab_scenario and rebuild runners ==\n'
rg -n 'devtools\.lab_scenario|from devtools import lab_scenario|lab_scenario\.|run_rebuild_(safety|differential)|REBUILD_.*SCENARIO_NAME|list_scenarios|smoke list' . -g '!tests' -g '!docs' -g '!**/.git/**' | head -200

Repository: Sinity/polylogue

Length of output: 24169


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '== command_catalog lab entries ==\n'
sed -n '2130,2195p' devtools/command_catalog.py | cat -n

printf '\n== command_catalog top imports relevant to devtools ==\n'
sed -n '1,220p' devtools/command_catalog.py | cat -n

printf '\n== exact module scope import summary for devtools/*.py that affect command catalog import entry ==\n'
python3 - <<'PY'
import ast, pathlib
for path in sorted(pathlib.Path("devtools").glob("*.py")):
    import_count = import_module_count = submodule_names = 0
    with open(path, encoding="utf-8") as f:
        tree = ast.parse(f.read(), filename=str(path))
    for node in tree.body:
        if isinstance(node, ast.Import):
            import_count += len(node.names)
            for name in node.names:
                if name.name.startswith("devtools."):
                    import_module_count += 1
                    submodule_names.append(name.name)
        elif isinstance(node, ast.ImportFrom):
            if node.module and node.module.startswith("devtools."):
                import_count += len(node.names)
                import_module_count += 1
                submodule_names.append(node.module)
    if import_module_count:
        print(f"{path}: imports={import_count}, named_devtools_imports={import_module_count}")
        for name in submodule_names[:40]:
            print(f"  {name}")
PY

printf '\n== test monkeypatches of lab scenario runners/results ==\n'
rg -n 'lab_scenario|run_rebuild_(safety|differential)|RebuildSafetyResult|run_storage_correctness|storage_correctness_scenario_entry' tests -g '*.py' | head -200 || true

Repository: Sinity/polylogue

Length of output: 12363


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '== monkeypatches in tests for lab scenario runners/results ==\n'
rg -n 'lab_scenario\.|monkeypatch|patch\.|run_rebuild_(safety|differential)|RebuildSafetyResult|list_scenarios' tests -g '*.py' | head -250

printf '\n== command catalog relevant code around importlib usage ==\n'
rg -n -C 5 'def COMMAND_SPECS|CommandMain =|resolve_main|import_module|run_main|lab labs|command_catalog|resolve|devtools lab|--help|VERIFICATION_LAB_COMMAND_NAMES' devtools polylogue -g '*.py' | head -350

Repository: Sinity/polylogue

Length of output: 50374


Defer the rebuild-scenario imports to the lab runner path.

devtools/lab_scenario.py imports devtools.rebuild_safety_scenario at module scope, and the command catalog imports this module for devtools lab run and devtools lab smoke. That imports ArchiveStore, rebuild_index, revision_backfill, convergence_stages, BlobStore, and tests.infra even when only the archive smoke, schema listing, or storage-correctness path runs. Import the runners and constants only when RebuildSafetyResult or the rebuild dispatch is resolved instead, and keep the existing patch points stable by patching the deferred runner names/attributes.

🤖 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 `@devtools/lab_scenario.py` around lines 17 - 23, Move the rebuild-scenario
imports out of module scope in devtools/lab_scenario.py and defer them until
RebuildSafetyResult or the rebuild dispatch path is resolved. Update those paths
to access the runners and constants through the deferred module or equivalent
stable attributes, while preserving the existing patch points for the deferred
runner names; non-rebuild lab commands must not import
devtools.rebuild_safety_scenario or its transitive dependencies.

from devtools.storage_correctness_scenario import (
STORAGE_CORRECTNESS_SCENARIO_NAME,
run_storage_correctness,
Expand All @@ -25,7 +33,12 @@
from polylogue.core.outcomes import OutcomeStatus
from polylogue.scenarios import AssertionSpec, ExecutionSpec, polylogue_execution

_SCENARIO_NAMES = ("archive-smoke", "reader-visual-smoke", STORAGE_CORRECTNESS_SCENARIO_NAME)
_SCENARIO_NAMES = (
"archive-smoke",
"reader-visual-smoke",
STORAGE_CORRECTNESS_SCENARIO_NAME,
REBUILD_SAFETY_SCENARIO_NAME,
)
Comment on lines +36 to +41

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Register the advertised devtools command path

Expose this scenario at the advertised devtools lab run rebuild-safety path. Adding it to _SCENARIO_NAMES only makes it an argument of the existing lab smoke command, whose sole catalog entry is CommandSpec("lab smoke", ...); repo-wide command-catalog inspection finds no lab run registration, so the documented invocation is rejected before reaching this parser.

AGENTS.md reference: AGENTS.md:L554-L555

Useful? React with 👍 / 👎.

_ARCHIVE_SMOKE_TIER = 0


Expand Down Expand Up @@ -113,6 +126,77 @@ def failed_stages(self) -> tuple[str, ...]:
return tuple(name for name, status in self.stage_statuses().items() if status is OutcomeStatus.ERROR)


class RebuildSafetyResult:
"""Direct result wrapper for the derived-tier rebuild lab lane."""

def __init__(self, *, report_dir: Path | None) -> None:
self.report_dir = report_dir
self.safety, self.safety_error = self._run("rebuild-safety", run_rebuild_safety)
self.differential, self.differential_error = self._run("rebuild-differential", run_rebuild_differential)
self._write_report()
Comment on lines +129 to +136

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

Move the scenario execution out of __init__.

Constructing RebuildSafetyResult runs two full archive rebuild scenarios and writes a report file. A constructor that performs minutes of work and filesystem writes is surprising, and it prevents any consumer from building the result type without executing the work.

The sibling scenario at Line 473 uses a function, run_storage_correctness(report_dir=args.report_dir). Match that shape: keep __init__ as a plain data assignment and add a run_rebuild_safety_scenario(*, report_dir) factory that performs the runs and returns the populated result.

🤖 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 `@devtools/lab_scenario.py` around lines 129 - 136, Move the rebuild execution
and report writing out of RebuildSafetyResult.__init__, leaving it as plain data
assignment only. Add run_rebuild_safety_scenario(*, report_dir) to run both
scenarios, populate the result fields, and write the report before returning the
result, matching the run_storage_correctness factory pattern.


@staticmethod
def _run(
name: str, runner: Callable[[], RebuildComparisonResult]
) -> tuple[RebuildComparisonResult | None, str | None]:
try:
return runner(), None
except Exception as exc:
return None, f"{name} failed: {type(exc).__name__}: {exc}"
Comment on lines +138 to +145

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Retain the traceback for a failed scenario run.

_run catches every Exception and reduces it to f"{name} failed: {type(exc).__name__}: {exc}". The traceback is discarded. This lane exists to produce evidence about rebuild failures; when a runner raises, the stack is the evidence. Capture it with traceback.format_exc() and include it in the written report.

♻️ Proposed refactor
+import traceback
+
     `@staticmethod`
     def _run(
         name: str, runner: Callable[[], RebuildComparisonResult]
     ) -> tuple[RebuildComparisonResult | None, str | None]:
         try:
             return runner(), None
         except Exception as exc:
-            return None, f"{name} failed: {type(exc).__name__}: {exc}"
+            return None, f"{name} failed: {type(exc).__name__}: {exc}\n{traceback.format_exc()}"

The existing test asserts "safety boom" in payload["safety_report"], which still holds.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@staticmethod
def _run(
name: str, runner: Callable[[], RebuildComparisonResult]
) -> tuple[RebuildComparisonResult | None, str | None]:
try:
return runner(), None
except Exception as exc:
return None, f"{name} failed: {type(exc).__name__}: {exc}"
import traceback
`@staticmethod`
def _run(
name: str, runner: Callable[[], RebuildComparisonResult]
) -> tuple[RebuildComparisonResult | None, str | None]:
try:
return runner(), None
except Exception as exc:
return None, f"{name} failed: {type(exc).__name__}: {exc}\n{traceback.format_exc()}"
🤖 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 `@devtools/lab_scenario.py` around lines 138 - 145, Update the exception
handling in _run to capture traceback.format_exc() and include the full
traceback in the returned failure message, while preserving the scenario name
and exception details.


@staticmethod
def _report(value: RebuildComparisonResult | None, error: str | None) -> str:
if error is not None:
return error
assert value is not None
return value.format_report()
Comment on lines +147 to +152

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Replace the assert with an explicit check.

python -O removes assert statements. With optimizations enabled and error is None while value is None, Line 152 calls format_report() on None and raises AttributeError instead of producing a report.

🐛 Proposed fix
     `@staticmethod`
     def _report(value: RebuildComparisonResult | None, error: str | None) -> str:
         if error is not None:
             return error
-        assert value is not None
+        if value is None:
+            return "no result and no error recorded"
         return value.format_report()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@staticmethod
def _report(value: RebuildComparisonResult | None, error: str | None) -> str:
if error is not None:
return error
assert value is not None
return value.format_report()
`@staticmethod`
def _report(value: RebuildComparisonResult | None, error: str | None) -> str:
if error is not None:
return error
if value is None:
return "no result and no error recorded"
return value.format_report()
🤖 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 `@devtools/lab_scenario.py` around lines 147 - 152, Update the static method
_report to replace the assert value is not None with an explicit runtime check,
ensuring the None/None case is handled safely instead of calling format_report
on None; preserve the existing error return and normal value.format_report
behavior.


def _write_report(self) -> None:
if self.report_dir is None:
return
self.report_dir.mkdir(parents=True, exist_ok=True)
(self.report_dir / "rebuild-safety.txt").write_text(
f"{self._report(self.safety, self.safety_error)}\n\n"
f"{self._report(self.differential, self.differential_error)}\n",
encoding="utf-8",
)
Comment on lines +154 to +162

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

The report filename does not distinguish the two stages.

_write_report writes both stage reports into one file named rebuild-safety.txt, with no header separating them. The scenario name is also the name of one of the two stages, so a reader cannot tell whether the file holds the safety report, the differential report, or both. RebuildComparisonResult.format_report does emit a scenario: first line, which partly mitigates this, but the file name remains ambiguous.

Consider writing rebuild-safety.txt and rebuild-differential.txt separately, or adding explicit section headers.

🤖 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 `@devtools/lab_scenario.py` around lines 154 - 162, Update _write_report so the
safety and differential stage reports are distinguishable: write them to
separate rebuild-safety.txt and rebuild-differential.txt files, or add explicit
section headers identifying each stage while retaining both reports.


@property
def scenario_name(self) -> str:
return REBUILD_SAFETY_SCENARIO_NAME

@property
def all_passed(self) -> bool:
return (
self.safety_error is None
and self.differential_error is None
and self.safety is not None
and self.differential is not None
and self.safety.all_passed
and self.differential.all_passed
)

def stage_statuses(self) -> dict[str, OutcomeStatus]:
return {
REBUILD_SAFETY_SCENARIO_NAME: OutcomeStatus.OK
if self.safety_error is None and self.safety is not None and self.safety.all_passed
else OutcomeStatus.ERROR,
REBUILD_DIFFERENTIAL_SCENARIO_NAME: OutcomeStatus.OK
if self.differential_error is None and self.differential is not None and self.differential.all_passed
else OutcomeStatus.ERROR,
}

def failed_stages(self) -> tuple[str, ...]:
return tuple(name for name, status in self.stage_statuses().items() if status is OutcomeStatus.ERROR)
Comment on lines +164 to +190

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Derive all_passed from stage_statuses to remove the duplicated predicate.

all_passed at Lines 169-177 and stage_statuses at Lines 179-187 encode the same success condition twice, once per stage and once combined. If one is changed later, the CLI exit code and the reported per-stage status can disagree. main uses result.all_passed for the exit code at Line 489, while _scenario_payload uses failed_stages() for the ok field at Line 369. Those two must not drift.

♻️ Proposed refactor
+    `@staticmethod`
+    def _stage_status(result: RebuildComparisonResult | None, error: str | None) -> OutcomeStatus:
+        if error is None and result is not None and result.all_passed:
+            return OutcomeStatus.OK
+        return OutcomeStatus.ERROR
+
     `@property`
     def all_passed(self) -> bool:
-        return (
-            self.safety_error is None
-            and self.differential_error is None
-            and self.safety is not None
-            and self.differential is not None
-            and self.safety.all_passed
-            and self.differential.all_passed
-        )
+        return not self.failed_stages()
 
     def stage_statuses(self) -> dict[str, OutcomeStatus]:
         return {
-            REBUILD_SAFETY_SCENARIO_NAME: OutcomeStatus.OK
-            if self.safety_error is None and self.safety is not None and self.safety.all_passed
-            else OutcomeStatus.ERROR,
-            REBUILD_DIFFERENTIAL_SCENARIO_NAME: OutcomeStatus.OK
-            if self.differential_error is None and self.differential is not None and self.differential.all_passed
-            else OutcomeStatus.ERROR,
+            REBUILD_SAFETY_SCENARIO_NAME: self._stage_status(self.safety, self.safety_error),
+            REBUILD_DIFFERENTIAL_SCENARIO_NAME: self._stage_status(self.differential, self.differential_error),
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@property
def scenario_name(self) -> str:
return REBUILD_SAFETY_SCENARIO_NAME
@property
def all_passed(self) -> bool:
return (
self.safety_error is None
and self.differential_error is None
and self.safety is not None
and self.differential is not None
and self.safety.all_passed
and self.differential.all_passed
)
def stage_statuses(self) -> dict[str, OutcomeStatus]:
return {
REBUILD_SAFETY_SCENARIO_NAME: OutcomeStatus.OK
if self.safety_error is None and self.safety is not None and self.safety.all_passed
else OutcomeStatus.ERROR,
REBUILD_DIFFERENTIAL_SCENARIO_NAME: OutcomeStatus.OK
if self.differential_error is None and self.differential is not None and self.differential.all_passed
else OutcomeStatus.ERROR,
}
def failed_stages(self) -> tuple[str, ...]:
return tuple(name for name, status in self.stage_statuses().items() if status is OutcomeStatus.ERROR)
`@property`
def scenario_name(self) -> str:
return REBUILD_SAFETY_SCENARIO_NAME
`@staticmethod`
def _stage_status(result: RebuildComparisonResult | None, error: str | None) -> OutcomeStatus:
if error is None and result is not None and result.all_passed:
return OutcomeStatus.OK
return OutcomeStatus.ERROR
`@property`
def all_passed(self) -> bool:
return not self.failed_stages()
def stage_statuses(self) -> dict[str, OutcomeStatus]:
return {
REBUILD_SAFETY_SCENARIO_NAME: self._stage_status(self.safety, self.safety_error),
REBUILD_DIFFERENTIAL_SCENARIO_NAME: self._stage_status(self.differential, self.differential_error),
}
def failed_stages(self) -> tuple[str, ...]:
return tuple(name for name, status in self.stage_statuses().items() if status is OutcomeStatus.ERROR)
🤖 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 `@devtools/lab_scenario.py` around lines 164 - 190, Update the all_passed
property to derive its result from stage_statuses(), returning true only when
every stage status is OutcomeStatus.OK. Remove the duplicated safety and
differential success predicates from all_passed while preserving
stage_statuses() as the single source of truth for both exit status and reported
stage results.


def extra_payload(self) -> dict[str, object]:
payload: dict[str, object] = {
"safety_report": self._report(self.safety, self.safety_error),
"differential_report": self._report(self.differential, self.differential_error),
}
return payload
Comment on lines +192 to +197

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Simplify extra_payload.

The local payload variable is assigned and immediately returned with no intervening mutation. Return the dictionary directly.

♻️ Proposed refactor
     def extra_payload(self) -> dict[str, object]:
-        payload: dict[str, object] = {
+        return {
             "safety_report": self._report(self.safety, self.safety_error),
             "differential_report": self._report(self.differential, self.differential_error),
         }
-        return payload
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def extra_payload(self) -> dict[str, object]:
payload: dict[str, object] = {
"safety_report": self._report(self.safety, self.safety_error),
"differential_report": self._report(self.differential, self.differential_error),
}
return payload
def extra_payload(self) -> dict[str, object]:
return {
"safety_report": self._report(self.safety, self.safety_error),
"differential_report": self._report(self.differential, self.differential_error),
}
🤖 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 `@devtools/lab_scenario.py` around lines 192 - 197, Update extra_payload to
return the report dictionary directly, removing the unnecessary local payload
variable while preserving both safety_report and differential_report entries
unchanged.



def get_archive_smoke_checks() -> tuple[ArchiveSmokeCheck, ...]:
"""Return direct CLI checks for the archive-smoke lab smoke."""
return _ARCHIVE_SMOKE_CHECKS
Expand Down Expand Up @@ -171,6 +255,11 @@ def list_scenarios(*, as_json: bool) -> int:
"artifact_count": len(reader_visual_artifact_payloads()),
},
storage_correctness_scenario_entry(),
{
"name": REBUILD_SAFETY_SCENARIO_NAME,
"kind": "derived-tier-differential",
"checks": [REBUILD_SAFETY_SCENARIO_NAME, REBUILD_DIFFERENTIAL_SCENARIO_NAME],
},
Comment on lines +259 to +262

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Register the scenario in the coverage control plane

Adding this entry makes the scenario visible to lab run/list, but docs/plans/scenario-coverage.yaml still declares scenario.schema-rebuild-safety as a major gap with “No scenario for schema rebuild safety verification,” and no scenario family or projection references this implementation. Consequently, devtools lab projections and readiness artifacts continue reporting the gap, and the new lane remains outside the repository's scenario-coverage inventory. Add the realized family/projection and retire the stale gap when registering the scenario.

AGENTS.md reference: AGENTS.md:L535-L551

Useful? React with 👍 / 👎.

]
payload = {"scenarios": scenarios}
if as_json:
Expand All @@ -184,6 +273,9 @@ def list_scenarios(*, as_json: bool) -> int:
if name == "storage-correctness":
print(f"{name:<20s} checks: {entry['check_count']}")
continue
if name == REBUILD_SAFETY_SCENARIO_NAME:
print(f"{name:<20s} checks: rebuild safety + differential")
continue
Comment on lines +276 to +278

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Render the listing line from the entry data.

The printed text "rebuild safety + differential" is hardcoded and duplicates entry["checks"] set at Line 261. If a third check is added to the scenario, the JSON output updates and the human-readable output does not.

♻️ Proposed refactor
         if name == REBUILD_SAFETY_SCENARIO_NAME:
-            print(f"{name:<20s}  checks: rebuild safety + differential")
+            checks = ", ".join(str(check) for check in entry["checks"])  # type: ignore[union-attr]
+            print(f"{name:<20s}  checks: {checks}")
             continue
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if name == REBUILD_SAFETY_SCENARIO_NAME:
print(f"{name:<20s} checks: rebuild safety + differential")
continue
if name == REBUILD_SAFETY_SCENARIO_NAME:
checks = ", ".join(str(check) for check in entry["checks"]) # type: ignore[union-attr]
print(f"{name:<20s} checks: {checks}")
continue
🤖 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 `@devtools/lab_scenario.py` around lines 276 - 278, Update the listing branch
for REBUILD_SAFETY_SCENARIO_NAME to render the checks value from the
corresponding entry data rather than hardcoding "rebuild safety + differential";
preserve the existing name formatting and ensure added checks appear in the
human-readable output.

print(f"{name:<20s} tier-0 checks: {entry['tier_0_check_count']}")
return 0

Expand Down Expand Up @@ -379,6 +471,8 @@ def main(argv: list[str] | None = None) -> int:
result: _ScenarioResult
if args.scenario == "storage-correctness":
result = run_storage_correctness(report_dir=args.report_dir)
elif args.scenario == REBUILD_SAFETY_SCENARIO_NAME:
result = RebuildSafetyResult(report_dir=args.report_dir)
Comment on lines +474 to +475

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject --live for the scratch-only rebuild scenario

The parser documents --live as running against the active archive, but devtools lab run rebuild-safety --live reaches this branch without inspecting the flag, and both rebuild runners unconditionally create a TemporaryDirectory. The command can therefore report green while testing only synthetic scratch data despite the requested context; reject this unsupported option for rebuild-safety or explicitly route it to the requested archive.

Useful? React with 👍 / 👎.

else:
result = run_archive_smoke(
live=bool(args.live),
Expand All @@ -395,5 +489,10 @@ def main(argv: list[str] | None = None) -> int:
return 0 if result.all_passed else 1


def run_main(argv: list[str] | None = None) -> int:
"""Run a named lab scenario through the advertised ``devtools lab run`` route."""
return main(["run", *(argv or [])])


if __name__ == "__main__":
raise SystemExit(main())
Loading