test(devtools): publish rebuild safety differential lane - #3915
Conversation
📝 WalkthroughWalkthroughAdds ChangesRebuild verification scenarios
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Developer
participant LabCLI
participant RebuildSafetyResult
participant Archive
participant ComparisonEngine
Developer->>LabCLI: run rebuild-safety
LabCLI->>RebuildSafetyResult: execute scenario
RebuildSafetyResult->>Archive: seed evidence and rebuild indexes
RebuildSafetyResult->>ComparisonEngine: compare derived tables and FTS results
ComparisonEngine-->>RebuildSafetyResult: return diffs and status
RebuildSafetyResult-->>LabCLI: return report and exit status
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@circleci run |
|
@coderabbitai review |
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb64dfebf7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _DEMO_STRATA: tuple[Stratum, ...] = ( | ||
| Stratum("rebuild-safety/codex-session", Provider.CODEX, count=4, total_bytes=4 * 4_000), | ||
| Stratum("rebuild-safety/claude-code-session", Provider.CLAUDE_CODE, count=4, total_bytes=4 * 3_000), | ||
| ) |
There was a problem hiding this comment.
Seed rows for the content-bearing tables this differential claims to cover. In this corpus, tests.infra.rebuild_cost_model._codex_payload emits only one user text message and _claude_code_payload emits only two text turns, so actions, attachments, session links, refs, file edits, provider usage, and several other derived relations remain empty in both databases. A regression that drops or mis-materializes any of those rows therefore still reports success because two empty dumps compare equal.
AGENTS.md reference: AGENTS.md:L118-L122
Useful? React with 👍 / 👎.
| "messages_fts", | ||
| "messages_fts_config", | ||
| "messages_fts_data", | ||
| "messages_fts_docsize", | ||
| "messages_fts_idx", | ||
| "messages_fts_identity", | ||
| "blocks_command_trigram", |
There was a problem hiding this comment.
Do not exclude the logical FTS surfaces along with their opaque shadow tables. If incremental trigger maintenance stops indexing blocks while the relational rows remain correct, this differential still passes because messages_fts, messages_fts_identity, blocks_command_trigram, and session_work_events_fts are never queried; notably, messages_fts_identity is an ordinary exact-comparison ledger rather than an opaque FTS segment. Keep the binary shadow tables allowlisted, but compare the ledger and execute representative MATCH/LIKE parity checks for the logical indexes.
AGENTS.md reference: AGENTS.md:L74-L78
Useful? React with 👍 / 👎.
| row = conn.execute( | ||
| "SELECT assertion_id, target_ref, kind, body_text FROM assertions WHERE assertion_id = ?", | ||
| (assertion_id,), | ||
| ).fetchone() |
There was a problem hiding this comment.
Expand the preservation check beyond four columns of one assertion. A rebuild regression that changes context_policy_json, timestamps, provenance fields, annotation schemas/batches, settings, or any other durable user row will still satisfy user_db_untouched, even though the scenario advertises that the durable tier was never touched. Snapshot all relevant user.db tables and columns before and after the rebuild instead.
AGENTS.md reference: AGENTS.md:L119-L122
Useful? React with 👍 / 👎.
| if self.report_dir is not None: | ||
| self.report_dir.mkdir(parents=True, exist_ok=True) | ||
| (self.report_dir / "rebuild-safety.txt").write_text( | ||
| f"{self.safety.format_report()}\n\n{self.differential.format_report()}\n", | ||
| encoding="utf-8", |
There was a problem hiding this comment.
Write the report during scenario execution rather than from extra_payload(). That method is invoked only by _scenario_payload, so devtools lab smoke run rebuild-safety --report-dir <dir> without --json prints an Artifacts: path but never creates the directory or rebuild-safety.txt; the other scenarios honor --report-dir independently of the display format.
Useful? React with 👍 / 👎.
| "raw_revision_applications", | ||
| "raw_revision_heads", | ||
| "insight_materialization", | ||
| } |
There was a problem hiding this comment.
Remove insight_materialization from the whole-table allowlist and exclude only its wall-clock stamp. This ledger contains materializer versions, source high-water marks, row counts, and provenance used by readiness and staleness queries, so an incremental path that writes correct-looking profile rows but omits or corrupts its ledger can still be considered perpetually stale—or incorrectly fresh—in production while this differential reports success.
AGENTS.md reference: AGENTS.md:L168-L177
Useful? React with 👍 / 👎.
| insights_stage = make_insights_stage(index_db) | ||
| execute_sessions = insights_stage.execute_sessions | ||
| if execute_sessions is None: | ||
| raise RuntimeError("insights convergence stage does not expose session-scoped execution") | ||
| execute_sessions(session_ids) |
There was a problem hiding this comment.
Check the boolean returned by execute_sessions instead of discarding it. This stage deliberately returns False when work is deferred, SQLite is transiently busy, or stale sessions remain; if the currently compared rows happen to match—especially for allowlisted bookkeeping—the scenario reports the incremental path as converged even though the production stage explicitly says it is still pending. Retry through the stage contract or fail the differential when it does not settle.
AGENTS.md reference: AGENTS.md:L168-L177
Useful? React with 👍 / 👎.
| _SCENARIO_NAMES = ( | ||
| "archive-smoke", | ||
| "reader-visual-smoke", | ||
| STORAGE_CORRECTNESS_SCENARIO_NAME, | ||
| REBUILD_SAFETY_SCENARIO_NAME, | ||
| ) |
There was a problem hiding this comment.
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 👍 / 👎.
| def __init__(self, *, report_dir: Path | None) -> None: | ||
| self.report_dir = report_dir | ||
| self.safety = run_rebuild_safety() | ||
| self.differential = run_rebuild_differential() |
There was a problem hiding this comment.
Catch failures from each rebuild sub-scenario and represent them as failed stages. Any admission, replay, parser, DDL, or acceptance-check regression currently raises out of this constructor, so a --json run emits a traceback instead of its promised machine-readable result, skips the differential when safety fails first, and cannot write diagnostic artifacts—the exact cases where this validation lane is most needed.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🤖 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 `@devtools/command_catalog.py`:
- Around line 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`.
In `@devtools/lab_scenario.py`:
- Around line 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.
- Around line 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.
- Around line 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.
- Around line 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.
- Around line 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.
- Around line 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.
- Around line 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.
- Around line 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.
In `@devtools/rebuild_safety_scenario.py`:
- Around line 340-346: Update _diff_table to compare row multiplicity with
collections.Counter rather than sets, so duplicate rows produce a differential
failure. Change _dump_table and its callers so tables with only volatile columns
are recorded as unverifiable or cause the comparison to fail, rather than
returning an empty dump that is counted in covered_tables as passing. Leave the
existing locally derived SQL identifiers unchanged.
- Around line 163-177: Fix coverage accounting in ScenarioResult.format_report
and the related all_passed logic so covered_tables contains only tables actually
compared by the diff engine; retain allowlisted tables separately, report any
remaining uncovered census tables, and make all_passed fail when that set is
non-empty. Update the stale docstring reference to _assert_every_table_covered
to name the real implementation. In devtools/rebuild_safety_scenario.py lines
103-108, add threads_fts to _LOGICAL_FTS_SURFACES with a corresponding
_logical_fts_rows branch, or explicitly document why the threads search surface
is excluded from comparison.
- Around line 358-428: Update _fts_probe_term and _logical_fts_rows to compare a
representative set or full (term, doc, cnt) vocabulary projection so partial FTS
loss is detected, including multiple probes for blocks_command_trigram rather
than only the first tool-use block. Escape LIKE wildcards in the trigram probe
and add an ESCAPE clause. Ensure divergence reporting clearly identifies term
mismatches when databases select different probe terms.
- Around line 602-613: Update the raw_sessions lookup in the loop over raw_ids
so a missing row raises a RuntimeError identifying the affected raw_id, instead
of continuing. Keep the existing payload decoding and undetectable-payload error
path unchanged.
- Around line 647-667: Preserve the full determinism comparison details in the
RebuildComparisonResult instead of reducing determinism to only
determinism.all_passed in extra_checks. Update the result construction around
_diff_index_databases and the extra_checks field so per-table diffs, including
diverging table names and sample rows, remain available while retaining the
pass/fail status.
- Around line 436-441: Update _diff_index_databases so both sqlite3.connect
calls occur inside a guarded resource-management block; use contextlib.closing
(or ExitStack) to ensure conn_a is released if connecting conn_b fails, and
remove or adapt the existing trailing finally block to avoid duplicate cleanup.
In `@tests/unit/devtools/test_rebuild_safety_scenario.py`:
- Around line 120-132: The test
test_incremental_path_writes_every_session_from_a_multi_session_raw does not
validate its multi-session equivalence claim. Update it to run both
_full_rebuild and _incremental_ingest_and_converge on equivalent seeded
archives, collect and compare their resulting session sets, and retain the
comparison as the assertion; alternatively, revise the docstring to accurately
describe the single native_id count check.
- Line 85: Remove the unused tmp_path parameter from
test_rebuild_safety_rejects_complete_user_tier_mutation, leaving the test’s
behavior and other fixtures unchanged.
- Around line 34-45: Import closing from contextlib and wrap every
sqlite3.connect call in this test, including the blocks around counts, actual
writes, and the connections near the other referenced sections, with closing so
each connection is explicitly closed after commit or rollback.
- Around line 26-32: Add the repository’s slow-test marker and an explicit
extended timeout to the full rebuild and incremental ingest tests in
test_rebuild_safety_scenario.py, including
test_seeded_corpus_populates_content_bearing_derived_relations and any related
cases invoking scenario._full_rebuild or incremental ingest. Keep the existing
coverage and test behavior unchanged while ensuring normal unit runs can exclude
these slow scenarios.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2644810c-1936-43f4-b45f-2366cd719085
📒 Files selected for processing (6)
devtools/command_catalog.pydevtools/lab_scenario.pydevtools/rebuild_safety_scenario.pydocs/devtools.mdtests/unit/devtools/test_lab_scenario.pytests/unit/devtools/test_rebuild_safety_scenario.py
| 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", | ||
| ), | ||
| ), |
There was a problem hiding this comment.
📐 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 -40Repository: 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.pyRepository: 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}')
PYRepository: 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`.
| from devtools.rebuild_safety_scenario import ( | ||
| REBUILD_DIFFERENTIAL_SCENARIO_NAME, | ||
| REBUILD_SAFETY_SCENARIO_NAME, | ||
| RebuildComparisonResult, | ||
| run_rebuild_differential, | ||
| run_rebuild_safety, | ||
| ) |
There was a problem hiding this comment.
🚀 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 -60Repository: 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 -200Repository: 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 || trueRepository: 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 -350Repository: 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.
| 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() |
There was a problem hiding this comment.
📐 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}" |
There was a problem hiding this comment.
📐 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.
| @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() |
There was a problem hiding this comment.
🩺 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.
| @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.
| determinism = _diff_index_databases(full_pass, full_rerun_pass, scenario_name="rebuild-determinism") | ||
|
|
||
| incremental_pass = Path(tmp) / "index-incremental.db" | ||
| _incremental_ingest_and_converge(archive_root, raw_ids) | ||
| incremental_witness_blobs = _write_attachment_witness(archive_root) | ||
| (archive_root / "index.db").rename(incremental_pass) | ||
| _discard_attachment_witness_blobs(archive_root, incremental_witness_blobs) | ||
|
|
||
| differential = _diff_index_databases( | ||
| full_pass, incremental_pass, scenario_name=REBUILD_DIFFERENTIAL_SCENARIO_NAME | ||
| ) | ||
| return RebuildComparisonResult( | ||
| scenario_name=REBUILD_DIFFERENTIAL_SCENARIO_NAME, | ||
| diffs=differential.diffs, | ||
| covered_tables=differential.covered_tables, | ||
| census_tables=differential.census_tables, | ||
| extra_checks={ | ||
| **differential.extra_checks, | ||
| "full_rebuild_is_deterministic": determinism.all_passed, | ||
| }, | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
The determinism failure loses all diagnostic detail.
determinism is a full RebuildComparisonResult with per-table diffs. Line 665 collapses it to a single boolean in extra_checks. When two consecutive full rebuilds disagree, the operator sees only full_rebuild_is_deterministic: FAILED. The diverging table names and sample rows are discarded, and the report cannot answer which table drifted.
Determinism failure is the most valuable signal this lane produces. Preserve its diffs.
🔧 Proposed fix: carry the determinism diffs into the result
return RebuildComparisonResult(
scenario_name=REBUILD_DIFFERENTIAL_SCENARIO_NAME,
- diffs=differential.diffs,
+ diffs=(
+ *differential.diffs,
+ *(
+ TableDiff(
+ table=f"determinism: {diff.table}",
+ only_in_a=diff.only_in_a,
+ only_in_b=diff.only_in_b,
+ )
+ for diff in determinism.diverging_tables
+ ),
+ ),
covered_tables=differential.covered_tables,
census_tables=differential.census_tables,
extra_checks={
**differential.extra_checks,
"full_rebuild_is_deterministic": determinism.all_passed,
},
)🤖 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/rebuild_safety_scenario.py` around lines 647 - 667, Preserve the
full determinism comparison details in the RebuildComparisonResult instead of
reducing determinism to only determinism.all_passed in extra_checks. Update the
result construction around _diff_index_databases and the extra_checks field so
per-table diffs, including diverging table names and sample rows, remain
available while retaining the pass/fail status.
| def test_seeded_corpus_populates_content_bearing_derived_relations(tmp_path: Path) -> None: | ||
| """The differential must compare non-empty rows from real parser input.""" | ||
| from devtools import rebuild_safety_scenario as scenario | ||
|
|
||
| archive_root, _raw_ids = _seeded_archive(tmp_path) | ||
| scenario._full_rebuild(archive_root) | ||
| scenario._write_attachment_witness(archive_root) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check the repository's test tiering conventions and any slow-test markers.
set -euo pipefail
rg -nP -C 3 'markers\s*=|slow|integration|timeout' pyproject.toml | head -40
fd -t d -d 2 . tests/ | head -20
rg -nP '`@pytest`\.mark\.(slow|integration|timeout)' tests/unit/devtools/ | head -20Repository: Sinity/polylogue
Length of output: 3420
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file outline =="
ast-grep outline tests/unit/devtools/test_rebuild_safety_scenario.py --view expanded || true
echo "== file lines =="
cat -n tests/unit/devtools/test_rebuild_safety_scenario.py
echo "== related devtools test markers =="
rg -n '`@pytest`\.mark\.(slow|integration|timeout|frozen_clock|uses_real_clock)' tests/unit/devtools -g '*.py' | head -120
echo "== pytest config markers relevant =="
sed -n '155,182p' pyproject.tomlRepository: Sinity/polylogue
Length of output: 12514
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import ast
from pathlib import Path
path = Path("tests/unit/devtools/test_rebuild_safety_scenario.py")
tree = ast.parse(path.read_text())
def marker_names(node):
names = []
for dec in node.decorator_list:
call = dec if isinstance(dec, ast.Call) else None
expr = call.func if call else dec
if isinstance(expr, ast.Name):
names.append(expr.id)
elif isinstance(expr, ast.Attribute):
names.append(expr.attr)
elif call and isinstance(call.func, ast.Attribute) and call.func.attr == "timeout":
names.append("timeout")
return names
markerless_tests = []
marked_tests = {}
for node in tree.body:
if isinstance(node, ast.FunctionDef) and node.name.startswith("test_"):
markerless = marker_names(node) == []
markerless_tests.append({"name": node.name, "lineno": node.lineno})
marked_tests[node.name] = marker_names(node)
print({"unmarked_tests": markerless_tests})
print({"marked_tests": marked_tests})
PY
rg -n 'pytest\.mark\.(unit|slow|integration|timeout)' tests pyproject.toml pytest.ini setup.cfg tox.ini Makefile 2>/dev/null || trueRepository: Sinity/polylogue
Length of output: 10355
Move full rebuild coverage out of the fast unit tier.
These tests call real full archive rebuild and incremental ingest paths, but all tests in tests/unit/devtools/test_rebuild_safety_scenario.py are unmarked. Because pytest selects tests under tool.pytest.ini_options, these slow rebuild cases can run in normal unit runs and increase devtools test/testmon cost. Preserve coverage by adding @pytest.mark.slow with an explicit longer timeout where needed, or move these tests to integration tier.
🤖 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 `@tests/unit/devtools/test_rebuild_safety_scenario.py` around lines 26 - 32,
Add the repository’s slow-test marker and an explicit extended timeout to the
full rebuild and incremental ingest tests in test_rebuild_safety_scenario.py,
including test_seeded_corpus_populates_content_bearing_derived_relations and any
related cases invoking scenario._full_rebuild or incremental ingest. Keep the
existing coverage and test behavior unchanged while ensuring normal unit runs
can exclude these slow scenarios.
| with sqlite3.connect(archive_root / "index.db") as conn: | ||
| counts = { | ||
| table: int(conn.execute(f"SELECT COUNT(*) FROM {table}").fetchone()[0]) | ||
| for table in ( | ||
| "actions", | ||
| "attachments", | ||
| "session_links", | ||
| "session_model_usage", | ||
| "session_provider_usage_events", | ||
| "blocks_command_trigram_docsize", | ||
| ) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
with sqlite3.connect(...) does not close the connection.
The sqlite3.Connection context manager commits or rolls back the transaction on exit. It does not close the connection. Every one of these blocks leaks an open handle for the remainder of the test session.
This matters here beyond hygiene. At Lines 62-72 the test writes to actual, and at Line 74 _diff_index_databases reopens the same file while the first handle is still open. The test works today, but the pattern is fragile against file-locking behavior.
Wrap with contextlib.closing.
🐛 Proposed fix pattern
- with sqlite3.connect(actual) as conn:
+ with closing(sqlite3.connect(actual)) as conn:
conn.execute("DELETE FROM messages_fts")Add from contextlib import closing and apply the same change at Lines 34, 93, and 127.
Also applies to: 62-72, 93-98, 127-130
🧰 Tools
🪛 OpenGrep (1.26.0)
[ERROR] 36-36: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
🤖 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 `@tests/unit/devtools/test_rebuild_safety_scenario.py` around lines 34 - 45,
Import closing from contextlib and wrap every sqlite3.connect call in this test,
including the blocks around counts, actual writes, and the connections near the
other referenced sections, with closing so each connection is explicitly closed
after commit or rollback.
| assert result.extra_checks["messages_fts_identity_b_is_consistent"] is False | ||
|
|
||
|
|
||
| def test_rebuild_safety_rejects_complete_user_tier_mutation(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
tmp_path is unused in this test.
run_rebuild_safety creates its own TemporaryDirectory. The tmp_path parameter is never referenced in the body. Remove it.
♻️ Proposed refactor
-def test_rebuild_safety_rejects_complete_user_tier_mutation(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
+def test_rebuild_safety_rejects_complete_user_tier_mutation(monkeypatch: pytest.MonkeyPatch) -> None:📝 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.
| def test_rebuild_safety_rejects_complete_user_tier_mutation(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: | |
| def test_rebuild_safety_rejects_complete_user_tier_mutation(monkeypatch: pytest.MonkeyPatch) -> None: |
🤖 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 `@tests/unit/devtools/test_rebuild_safety_scenario.py` at line 85, Remove the
unused tmp_path parameter from
test_rebuild_safety_rejects_complete_user_tier_mutation, leaving the test’s
behavior and other fixtures unchanged.
| def test_incremental_path_writes_every_session_from_a_multi_session_raw(tmp_path: Path) -> None: | ||
| """Incremental parsing preserves the same multi-session raw expansion as replay.""" | ||
| from devtools import rebuild_safety_scenario as scenario | ||
|
|
||
| archive_root, raw_ids = _seeded_archive(tmp_path) | ||
| scenario._incremental_ingest_and_converge(archive_root, raw_ids) | ||
|
|
||
| with sqlite3.connect(archive_root / "index.db") as conn: | ||
| count = int( | ||
| conn.execute("SELECT COUNT(*) FROM sessions WHERE native_id = 'claude-normalization-other'").fetchone()[0] | ||
| ) | ||
|
|
||
| assert count == 1 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The assertion does not verify the claim in the docstring.
The docstring states "Incremental parsing preserves the same multi-session raw expansion as replay." The test never runs the replay path and never compares the two. It asserts that one hardcoded native_id appears exactly once after incremental ingest.
That assertion cannot detect the failure the docstring describes. If incremental ingest expanded a multi-session raw into fewer sessions than replay, but still produced claude-normalization-other once, the test passes.
Either compare the session set produced by _full_rebuild against the set produced by _incremental_ingest_and_converge, or change the docstring to describe what is actually asserted.
🤖 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 `@tests/unit/devtools/test_rebuild_safety_scenario.py` around lines 120 - 132,
The test test_incremental_path_writes_every_session_from_a_multi_session_raw
does not validate its multi-session equivalence claim. Update it to run both
_full_rebuild and _incremental_ingest_and_converge on equivalent seeded
archives, collect and compare their resulting session sets, and retain the
comparison as the assertion; alternatively, revise the docstring to accurately
describe the single native_id count check.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a321b886c6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| determinism = _diff_index_databases(full_pass, full_rerun_pass, scenario_name="rebuild-determinism") | ||
|
|
||
| incremental_pass = Path(tmp) / "index-incremental.db" | ||
| _incremental_ingest_and_converge(archive_root, raw_ids) |
There was a problem hiding this comment.
The incremental comparand always starts from a deleted index.db, writes each session once, and immediately runs the insights stage, so every selected session merely lacks a profile. It never creates an existing projection and then re-ingests, changes, or deletes its source session; consequently, a regression in the stale-profile predicates—the motivating a7xr.2 failure class described by this module—still passes as long as first-time materialization works. Add an update/delete cycle and converge that stale state before declaring the paths equivalent.
AGENTS.md reference: AGENTS.md:L168-L177
Useful? React with 👍 / 👎.
| before_user_tier = _user_tier_snapshot(archive_root) | ||
|
|
||
| first_pass = Path(tmp) / "index-first.db" | ||
| _full_rebuild(archive_root) |
There was a problem hiding this comment.
This safety scenario only invokes _full_rebuild on index.db; it never runs an additive durable-tier migration or verifies its required backup manifest. Therefore the command can report the schema rebuild-safety lane green even if a user.db migration loses assertions or bypasses the backup gate, leaving the durable-migration half of the advertised safety proof untested. Exercise a numbered user-tier migration and round-trip its data through the production backup-gated path.
AGENTS.md reference: AGENTS.md:L185-L193
Useful? React with 👍 / 👎.
|
|
||
| full_pass = Path(tmp) / "index-full.db" | ||
| _full_rebuild(archive_root) | ||
| full_witness_blobs = _write_attachment_witness(archive_root) |
There was a problem hiding this comment.
Fresh evidence after the earlier corpus-coverage comment is that attachment rows are now populated only by calling _write_attachment_witness after each full rebuild and again after incremental convergence. Because all three databases receive the same fixture through the same direct writer outside the paths under test—and the seeded raw corpus contains only Codex and Claude fixtures—a defect that drops or changes attachments during source replay or incremental ingest still produces equal attachment dumps and passes. Seed attachment evidence before the paths diverge and let each path materialize it.
AGENTS.md reference: AGENTS.md:L118-L121
Useful? React with 👍 / 👎.
| differential = _diff_index_databases( | ||
| full_pass, incremental_pass, scenario_name=REBUILD_DIFFERENTIAL_SCENARIO_NAME |
There was a problem hiding this comment.
The differential compares only a fresh full replay with a fresh incremental build, while the repository also upgrades existing derived generations in place through index_fast_forward_plan() and apply_index_fast_forward(). A fast-forward operation that drops, miscopies, or fails to backfill derived rows can therefore ship while this lane remains green, even though that is a supported production schema-evolution route. Build a prior-version generation, fast-forward it, and compare it with the same canonical full-rebuild result.
AGENTS.md reference: AGENTS.md:L193-L200
Useful? React with 👍 / 👎.
| (archive_root / "index.db").rename(full_rerun_pass) | ||
| _discard_attachment_witness_blobs(archive_root, rerun_witness_blobs) | ||
|
|
||
| determinism = _diff_index_databases(full_pass, full_rerun_pass, scenario_name="rebuild-determinism") |
There was a problem hiding this comment.
The full-rebuild determinism check reuses _diff_index_databases, whose global allowlist omits raw_revision_heads and raw_revision_applications solely because the incremental path does not recreate them. That rationale does not apply when both operands are full rebuilds: nondeterministic accepted heads or application receipts are ignored even though these relations drive authority, retention, readiness, and cursor reconciliation. Use a narrower allowlist for the full/full comparison and exclude only genuinely volatile receipt columns.
Useful? React with 👍 / 👎.
| "name": REBUILD_SAFETY_SCENARIO_NAME, | ||
| "kind": "derived-tier-differential", | ||
| "checks": [REBUILD_SAFETY_SCENARIO_NAME, REBUILD_DIFFERENTIAL_SCENARIO_NAME], | ||
| }, |
There was a problem hiding this comment.
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 👍 / 👎.
| index_db = archive_root / "index.db" | ||
| if index_db.exists(): | ||
| index_db.unlink() |
There was a problem hiding this comment.
The shared rebuild helper deletes, initializes, and compares only index.db; neither scenario seeds nonempty embedding metadata/status, resets embeddings.db, nor runs embedding catch-up. The lane can therefore report derived-tier rebuild safety even if the other rebuildable tier cannot be recreated or loses its source-to-vector status during reset. Add a deterministic embeddings witness and rebuild/catch-up comparison, or explicitly narrow the scenario and coverage claims to index-only safety.
AGENTS.md reference: AGENTS.md:L118-L121
Useful? React with 👍 / 👎.
a321b88 to
b0e243c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0e243cb3b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| vocab_name = f"rebuild_safety_{table}_vocab" | ||
| conn.execute(f"CREATE VIRTUAL TABLE temp.{vocab_name} USING fts5vocab(main, {table}, row)") | ||
| try: | ||
| row = conn.execute(f"SELECT term FROM temp.{vocab_name} WHERE doc > 0 ORDER BY term LIMIT 1").fetchone() |
There was a problem hiding this comment.
Fresh evidence after the prior logical-FTS allowlist comment is that the new parity check selects only the lexicographically first vocabulary term. If incremental trigger maintenance drops a block whose tokens do not affect that term, the selected MATCH results remain equal and the populated check stays true even though the contentless index has lost searchable content. Compare postings for all seeded terms/blocks, or otherwise verify the complete indexed identity set.
AGENTS.md reference: AGENTS.md:L76-L78
Useful? React with 👍 / 👎.
| elif args.scenario == REBUILD_SAFETY_SCENARIO_NAME: | ||
| result = RebuildSafetyResult(report_dir=args.report_dir) |
There was a problem hiding this comment.
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 👍 / 👎.
| # Pure convergence/refresh bookkeeping: run-scoped progress markers, | ||
| # not content derived from source raws. | ||
| "fts_freshness_state", | ||
| "derived_refresh_guard", |
There was a problem hiding this comment.
Do not completely allowlist derived_refresh_guard: a leaked session-write or bulk-FTS guard row is persistent behavioral state, because multiple triggers run only when the corresponding guard is absent. If either rebuild or incremental convergence leaves a guard behind after producing otherwise equal current rows, this comparison still passes, but later session writes silently skip FTS and derived refresh work. Compare the table or add an explicit final COUNT(*) = 0 check while still ignoring genuinely run-local counters.
Useful? React with 👍 / 👎.
| "price_catalogs", | ||
| # Pure convergence/refresh bookkeeping: run-scoped progress markers, | ||
| # not content derived from source raws. | ||
| "fts_freshness_state", |
There was a problem hiding this comment.
Do not treat fts_freshness_state as disposable run bookkeeping. Its state and row-count fields drive daemon readiness, status reporting, and metrics, so a rebuild path that leaves the ledger empty or stale can make /healthz/ready fail even when the underlying FTS rows happen to match. Compare the semantic state/count/detail columns between paths while normalizing only the run-local checked_at timestamp.
Useful? React with 👍 / 👎.
## Summary Close the Beads whose implementation obligations are now satisfied by merged PRs #3914 and #3915. ## Problem The tracker still represented merged documentation and rebuild-proof work as open, obscuring the remaining partial and production-phase obligations. ## Solution - Close `polylogue-cybpg` for merged PR #3914. - Close `polylogue-1xc.8` and `polylogue-hjwr` for merged PR #3915. - Preserve the parent derived-tier transition epic and all production/reindex successors as open. ## Verification - `bd show polylogue-cybpg --json`, `bd show polylogue-1xc.8 --json`, and `bd show polylogue-hjwr --json` report `status=closed` with evidence-bearing close reasons. - `bd export -o .beads/issues.jsonl` exported 1766 issues. - Pre-push Beads validation: zero unhandled findings across 1766 issues; active leaf set 26. <!-- polylogue-pr-scope:v1 { "assigned_beads": [ "polylogue-cybpg", "polylogue-1xc.8", "polylogue-hjwr" ], "beads_digest": "5b2031ef7877df277a5a254e1e9ec332f08534b151df263a9255373a47bb4f49", "dispositions": [ { "bead_id": "polylogue-cybpg", "disposition": "satisfied", "evidence": [ { "kind": "commit", "ref": "c75a5e2c4" }, { "kind": "command", "ref": "merged PR #3914; focused documentation checks and quick verification passed" } ], "successors": [] }, { "bead_id": "polylogue-1xc.8", "disposition": "satisfied", "evidence": [ { "kind": "commit", "ref": "cb0de952a" }, { "kind": "command", "ref": "merged PR #3915; rebuild-safety and rebuild-differential passed; 7 focused tests and all 24 quick checks passed" } ], "successors": [] }, { "bead_id": "polylogue-hjwr", "disposition": "satisfied", "evidence": [ { "kind": "commit", "ref": "cb0de952a" }, { "kind": "command", "ref": "merged PR #3915; deterministic/incremental differential covered 58 tables with zero uncovered" } ], "successors": [] } ], "head_sha": "76a34b8aabc315163482965b2617a3771c9fb914", "scope_digest": "773128c6a7c940b8858009b8f2907eaed210b2c733ec312f603a13e4f3485913", "version": 1 } -->
1 participant
Summary
Publish the recovered derived-tier rebuild-safety and rebuild-differential lab lane and expose it through
devtools lab run rebuild-safety.Problem
The repository had no executable scenario proving that a derived-index reset is lossless/idempotent or that full replay and incremental ingest plus convergence produce equivalent content. The recovered WIP also predated the explicit schema-inference and frozen-source gates, so it could not run against current master.
Solution
user.dbassertion preservation and deterministic full rebuild output.Verification
devtools test tests/unit/devtools/test_lab_scenario.py— 11 passed.python -m devtools.lab_scenario run rebuild-safety --json --report-dir .cache/rebuild-safety-report— bothrebuild-safetyandrebuild-differentialstagesok; 58 tables censused, 0 uncovered.devtools verify --quick— all 24 steps exit 0.Scope and residuals
This is implementation and proof infrastructure only. It does not claim a live production rebuild, candidate promotion, or closure of the parent Beads.
raw_revision_applicationsandraw_revision_headsare explicitly excluded from the content differential because they are replay-only source-authority receipts; source authority is prepared and frozen before both compared paths.Ref #1xc.8
Ref #hjwr
Summary by CodeRabbit
New Features
lab runcommand for executing named archive verification scenarios.Documentation
Tests