Skip to content

[rescue] rescue/2026-08-09-review-599 — preserve the PR #599 backfill review snapshot - #691

Draft
EtanHey wants to merge 12 commits into
mainfrom
rescue/2026-08-09-review-599
Draft

[rescue] rescue/2026-08-09-review-599 — preserve the PR #599 backfill review snapshot#691
EtanHey wants to merge 12 commits into
mainfrom
rescue/2026-08-09-review-599

Conversation

@EtanHey

@EtanHey EtanHey commented Aug 9, 2026

Copy link
Copy Markdown
Owner

Rescue context

This preserves the earlier review-stage snapshot of transcript-window backfill: idempotent replay, watcher/denylist integration, CLI and test-runner mapping, and the first integrity fixes.

The work came from the retained review-599 worktree. Raw ahead and verified-unlanded counts are both 12 against main; original PR #599 was CLOSED without merge. This snapshot overlaps bl-599 but records the distinct review-stage head that was rescued.

It was rescued instead of merged because the original PR closed without landing and Etan's ruling requires each real rescued survivor to remain visible for later disposition.

— maintenanceCodex (worker) · codex/gpt-5.6-sol

Note

Preserve backfill review snapshot with time-windowed, idempotent watch-backfill replays

  • Adds --since/--until options to the watch-backfill CLI command to scope backfills to a half-open UTC window, with per-window registries named via window_registry_suffix.
  • Adds --legacy-excluded-only to restrict backfills to transcript roots previously blocked by a retired blanket denylist, using is_legacy_excluded_path and is_legacy_backfill_denylisted.
  • Introduces WindowedFlush in backfill.py as an on_flush adapter that filters entries by time window and source predicate, advances watermarks for excluded/malformed lines, and rejects synthesized timestamps.
  • Extends JSONLWatcher and JSONLTailer in watcher.py to support configurable denylist policies, custom predicates, and watermark-only progress markers for blank or malformed lines.
  • Behavioral Change: polling now terminates when no tailer offset advances (not when zero entries are returned); denylisting a file no longer deletes its registry entry.
📊 Macroscope summarized 4dfcc6b. 5 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted

🗂️ Filtered Issues

No issues evaluated.

@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c5cd1feb-9178-4d90-aa81-03d9ea9cfb60

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Comment thread scripts/run_tests.sh
Comment on lines +146 to +152
for test_path in "$TEST_ROOT"/test_cli*.py "$TEST_ROOT"/test_watch_backfill_cli.py; do
if [ -f "$test_path" ] && ! is_real_db_test_file "$test_path"; then
append_unique "$test_path"
mapped=1
fi
done
;;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Medium scripts/run_tests.sh:146

A change to src/brainlayer/cli/__init__.py marks the file as fully mapped after selecting only test_cli*.py and test_watch_backfill_cli.py, so a changed-only pre-push run no longer falls back to the full suite. Other tests that import brainlayer.cli — e.g. tests/test_agent_profiles.py, tests/test_doctor.py, tests/test_runtime_store.py, and tests/test_status_truthfulness.py — are skipped on such a change, so edits to their command implementations can pass changed-only checks without running the relevant tests. Either map every test that exercises this module or set changed_source_unmapped=1 (instead of mapped=1) so the full-suite fallback still applies.

      src/brainlayer/cli/__init__.py)
-        for test_path in "$TEST_ROOT"/test_cli*.py "$TEST_ROOT"/test_watch_backfill_cli.py; do
-          if [ -f "$test_path" ] && ! is_real_db_test_file "$test_path"; then
-            append_unique "$test_path"
-            mapped=1
-          fi
-        done
+        for test_path in "$TEST_ROOT"/test_cli*.py "$TEST_ROOT"/test_watch_backfill_cli.py; do
+          if [ -f "$test_path" ] && ! is_real_db_test_file "$test_path"; then
+            append_unique "$test_path"
+          fi
+        done
+        changed_source_unmapped=1
        ;;
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @scripts/run_tests.sh around lines 146-152:

A change to `src/brainlayer/cli/__init__.py` marks the file as fully mapped after selecting only `test_cli*.py` and `test_watch_backfill_cli.py`, so a changed-only pre-push run no longer falls back to the full suite. Other tests that import `brainlayer.cli` — e.g. `tests/test_agent_profiles.py`, `tests/test_doctor.py`, `tests/test_runtime_store.py`, and `tests/test_status_truthfulness.py` — are skipped on such a change, so edits to their command implementations can pass changed-only checks without running the relevant tests. Either map every test that exercises this module or set `changed_source_unmapped=1` (instead of `mapped=1`) so the full-suite fallback still applies.

Comment on lines +54 to +65
def is_legacy_excluded_path(path: str | Path) -> bool:
"""Identify roots blocked by the blanket denylist retired in July 2026."""
parts = Path(path).expanduser().parts
return any(
_contains_ordered(parts, expected)
for expected in (
(".claude", "projects", "subagents"),
(".codex", "sessions"),
(".cursor", "agent-transcripts"),
(".gemini", "sessions"),
)
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 High brainlayer/backfill.py:54

is_legacy_excluded_path matches paths that contain the expected segments as an ordered subsequence anywhere in the path, so it returns True for paths like ~/.codex/other/sessions/file.jsonl or ~/.gemini/other/sessions/file.jsonl where sessions is not directly under .codex/.gemini. The retired denylist required .codex/sessions, .gemini/sessions, and .claude/projects/.../subagents, so --legacy-excluded-only ingests transcripts that were never excluded by the legacy policy. Use consecutive path segment matching (e.g., a sliding window over parts) instead of _contains_ordered.

Suggested change
def is_legacy_excluded_path(path: str | Path) -> bool:
"""Identify roots blocked by the blanket denylist retired in July 2026."""
parts = Path(path).expanduser().parts
return any(
_contains_ordered(parts, expected)
for expected in (
(".claude", "projects", "subagents"),
(".codex", "sessions"),
(".cursor", "agent-transcripts"),
(".gemini", "sessions"),
)
)
def is_legacy_excluded_path(path: str | Path) -> bool:
"""Identify roots blocked by the blanket denylist retired in July 2026."""
parts = Path(path).expanduser().parts
expected_patterns = (
(".claude", "projects", "subagents"),
(".codex", "sessions"),
(".cursor", "agent-transcripts"),
(".gemini", "sessions"),
)
for expected in expected_patterns:
window_size = len(expected)
for i in range(len(parts) - window_size + 1):
if parts[i:i + window_size] == expected:
return True
return False
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/brainlayer/backfill.py around lines 54-65:

`is_legacy_excluded_path` matches paths that contain the expected segments as an ordered subsequence anywhere in the path, so it returns `True` for paths like `~/.codex/other/sessions/file.jsonl` or `~/.gemini/other/sessions/file.jsonl` where `sessions` is not directly under `.codex`/`.gemini`. The retired denylist required `.codex/sessions`, `.gemini/sessions`, and `.claude/projects/.../subagents`, so `--legacy-excluded-only` ingests transcripts that were never excluded by the legacy policy. Use consecutive path segment matching (e.g., a sliding window over `parts`) instead of `_contains_ordered`.

@EtanHey

EtanHey commented Aug 10, 2026

Copy link
Copy Markdown
Owner Author

Lead disposition (Etan delegated): holding open as REFERENCE alongside #690 (its review snapshot). Closes together with it when Wave 4 supersedes. — brainlayerClaude lead (Fable 5)

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant