Skip to content

feat: redirect ignore-pattern drops into the lossless sidecar (WS1.5 PR-2) - #163

Closed
100yenadmin wants to merge 11 commits into
mainfrom
upstream/pr-363
Closed

100yenadmin wants to merge 11 commits into
mainfrom
upstream/pr-363

Conversation

@100yenadmin

Copy link
Copy Markdown
Member

Important

This executable LCM-X PR was recreated by the migration operator from the exact upstream commit head. GitHub did not transfer the original PR actor, dates, review objects, or approval state.

Source and attribution

@coderabbitai ignore

Original commit authorship and history remain in the commits. Historical discussion and review text are imported below as attributed ordinary comments; they are not new approvals or change requests.

Original upstream PR description

Summary

Second WS1.5 step, stacked on #362: preserve messages matched by ignore_message_patterns in the lossless ignored_messages sidecar while keeping them out of the active LCM path.

This update keeps lossless_ignored_enabled default-off and covers both raw drop sites. It also reconciles restart replay against the protected/normalized stored identity: only a contiguous, identity-proven ignored prefix is advanced, while unrelated or newly arrived ignored rows remain available for capture. Ignored-only and mixed ignored/active restarts therefore avoid duplicates without feeding ignored rows back into active context.

Why

Previously, matching messages were dropped permanently. The sidecar provides an audit-only durable path, but restart reconciliation must distinguish already persisted ignored rows from fresh input. Comparing raw input or advancing an unmatched tail can either duplicate protected rows or skip fresh rows.

The repaired flow protects the candidate row before identity comparison, advances only the matched ignored prefix, preserves fresh/unrelated rows, and retains the existing pattern timeout/error fail-safe. With the feature disabled, active behavior remains byte-identical; with it enabled, ignored rows are captured once and never enter replay, FTS, active token counts, or cursor feedback.

Validation

  • PYTHONDONTWRITEBYTECODE=1 PYTHONPATH=/tmp/t_a1dc5f84-generated python3 -m pytest -q -p no:cacheprovider tests/test_lossless_ignored_capture.py tests/test_ignored_sidecar.py tests/test_lcm_command.py tests/test_lcm_core.py — 393 passed in 9.83s
  • PYTHONPATH=/tmp/t_a1dc5f84-generated python3 -m pytest -q — 1638 passed, 12 xfailed in 32.15s
  • python3 -m py_compile reconcile.py tests/test_lossless_ignored_capture.py — passed
  • git diff --check HEAD^ HEAD — passed
  • Independent exact-head review of 482cd905d7e47adf5c56b175b74d57470c78d601 — ACCEPT, no blocking findings

Notes

  • Focused TDD reproduction before the reconciliation repair produced 2 failed, 9 passed: protected stored identity duplicated, and mixed restart skipped a fresh ignored row.
  • The effective child scope over accepted parent fix(governance): preserve valid exact-head peer receipts #362 is config.py, engine.py, reconcile.py, and tests/test_lossless_ignored_capture.py; the final repair commit changes only reconcile.py and its focused tests.
  • Local Ruff was unavailable in the content workspace; GitHub's hosted lint check is the authoritative lint result for this exact head.
  • Sidecar persistence remains best-effort and audit-only. Pattern timeout/error behavior remains fail-safe.
  • This PR remains open and stacked; no merge or close is requested by this update.

Refs

  • Depends on fix(governance): preserve valid exact-head peer receipts #362 (ignored sidecar lifecycle/store API), contained exactly via merge commit 7121b565f5da3caa77ef766272d312b2a1a2c243
  • Reviewed publication head: 482cd905d7e47adf5c56b175b74d57470c78d601
  • Capture sites: engine.py::_ingest_messages, engine.py::_append_off_current_session_end_suffix
  • Restart reconciliation: reconcile.py

ai-ag2026 and others added 11 commits July 12, 2026 02:23
Groundwork for storing ignore_message_patterns drops losslessly instead
of discarding them. Adds an additive ignored_messages child table plus
the store read/write API, with no ingest behavior change yet: no drop
path is redirected here, so replay, FTS, token counts, and cursor
reconciliation are untouched.

The sidecar is deliberately separate from the messages table. Keeping
ignored content out of the parent means the ~10 'stored row == replay
row' reconciliation assumptions and the external-content FTS rebuild
never see it, so lossless capture needs no changes to those fragile
subsystems (the reason a messages.ignored column was rejected).

Migration is version-fenceless: ensure_ignored_messages_table runs
unconditionally in run_versioned_migrations and does not bump
SCHEMA_VERSION, so an older build simply ignores the table and keeps
dropping -- downgrades stay safe. append_ignored_batch routes rows
through the same ingest protection as append_batch (redaction /
externalization before persist). Session rebind and delete keep the
sidecar bound to its parent session.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…PR-2)

Builds on the ignored_messages sidecar (PR-1). When the new
lossless_ignored_enabled flag is set, messages that match
ignore_message_patterns are persisted into the sidecar instead of being
discarded outright; they still stay out of replay, FTS, token counts,
and cursor reconciliation exactly as before, so only their durable
audit/recall value is added.

Both drop sites are covered: the main _ingest_messages raw-pattern drop
and the late session-end suffix drop. Persistence is best-effort and
fully decoupled from the active path -- a sidecar write failure is
swallowed (audit-only) and never breaks ingest. The existing
_ignore_pattern_dropped_count is preserved (augmented, not replaced), and
get_status surfaces lossless_ignored_enabled plus a per-session
ignored_sidecar_count when enabled.

The flag defaults off, so the drop path is byte-identical to today unless
an operator opts in (LCM_LOSSLESS_IGNORED_ENABLED).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Both sides added independent pre-scan blocks at the top of
_find_reconciled_cursor_for_store_tail (ignored-prefix cursor gating from
this PR, fresh-tail boundary counting from main); keep both.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@100yenadmin

Copy link
Copy Markdown
Member Author

Imported upstream pr-conversation

@​codex review current head d60a884. Rebuilt cleanly on repaired #362 head b22be578; capture-only diff remains scoped to config.py, engine.py, and tests/test_lossless_ignored_capture.py. Validation on exact stacked head: 1622 passed, 12 xfailed; ruff and hygiene gates passed.


Imported upstream pr-conversation

Stack re-gate evidence for current head 025ae75a7995f088b178970633e69f64edeca087:

@​codex please review this exact stacked head.


Imported upstream pr-conversation

@​codex Please review rebuilt stacked head 025ae75a7995f088b178970633e69f64edeca087.

This branch is rebuilt directly on repaired #362 head ea5edcf30ca3e9b9e800c0853b2d772db8d4e5b4 (ancestry verified). It retains the lossless ignored-capture change while inheriting the ignored-only cleanup regression/fix.

Fresh local evidence: focused 20 passed; full suite 1623 passed / 12 xfailed; ruff, compileall, py_compile, shell syntax, and git diff --check passed. GitHub Actions: 6/6 check runs completed successfully.

Please review #362 first, then the capture delta in this stacked PR.


Imported upstream pr-conversation

@​codex review\n\nCurrent rebuilt child head: ed105485b4c8ba5ac062d60f020812cb12ac8e33, directly based on parent #362 head 5033da015819cacbd3b8b87248bfdca9b8221edb. Please review the current head.


Imported upstream pr-conversation

@​codex review — head moved to 821ef86 since your last pass: the 16.07 reconcile fix (_matching_ignored_sidecar_prefix_count matched-prefix advance, _protected_ignored_replay_identity normalization — addressing all three of your P2s) plus a merge of current origin/main resolving the reconcile.py conflict (both sides' pre-scan blocks kept: this PR's ignored-prefix cursor gating and main's fresh-tail boundary counting). Full suite 1706 passed. Branch is mergeable again.


Imported upstream pr-conversation

Moved to draft to reduce the size of the open review queue, as requested.

Nothing is closed and no work is lost: the branch and its history stay intact. This will be reopened once the corresponding workstream base has clear review direction. Queue context: #416.


Imported upstream pr-review-commented

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 025ae75a79

ℹ️ 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".


Imported upstream pr-review-commented

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ed105485b4

ℹ️ 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".


Imported upstream pr-review-commented

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 821ef86e22

ℹ️ 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".


Imported upstream pr-review-comment

P2 Badge Avoid duplicating ignored-only sidecar rows after restart

When lossless capture is enabled and a session contains only messages matching ignore_message_patterns, there are no rows in messages, so a later engine restart does not schedule cursor reconciliation (it keys off the active message count). If the host replays the same ignored-only transcript, the cursor is still 0 and this sidecar persist runs again, appending duplicate ignored_messages rows on every restart for cron/noise sessions with no active rows. The sidecar needs to participate in the restart/reconciliation guard, or ignored-only replays need to be deduplicated before appending.

Useful? React with 👍 / 👎.


Imported upstream pr-review-comment

P2 Badge Advance only the matched ignored prefix

When lossless capture is enabled for an ignored-only session, this guard only advances if the entire incoming batch is ignored and the whole min(len(messages), len(ignored_rows)) prefix matches. If the process restarts after sidecar-storing ignored row A and the first batch is [A, real turn] (or [A, new ignored turn]), the cursor remains 0 and _ingest_messages appends A to ignored_messages again, corrupting the sidecar count. Reconcile the longest matching ignored prefix instead of requiring the whole batch to be an ignored-only replay.

Useful? React with 👍 / 👎.


Imported upstream pr-review-comment

P2 Badge Normalize incoming ignored rows before comparison

This identity comparison uses the raw incoming ignored message, but append_ignored_batch stores ignored rows after protect_messages_for_ingest. With sensitive-pattern redaction or payload externalization enabled, a replayed ignored message that was already sidecar-stored no longer has the same identity as the protected stored row, so restart reconciliation fails and the same ignored message is stored again. Build the incoming identity from the same protected/redacted form used for sidecar storage while still using the raw message for pattern matching.

Useful? React with 👍 / 👎.


Imported upstream pr-review-comment

P2 Badge Align ignored replay checks to the tail window

When an existing long session already has earlier ignored sidecar rows, a restarted gateway can replay only the durable tail. This code compares the first ignored message in that tail against stored_ignored_identities[0] (the first ignored row in the whole session), so any tail containing the second/later ignored row fails the gate, prevents cursor advancement, and causes both the active tail and the ignored sidecar row to be appended again. The sidecar identities need to be aligned to the replayed tail/suffix window rather than always starting at the oldest ignored row.

Useful? React with 👍 / 👎.

@100yenadmin 100yenadmin added accepted-defer Concrete accepted work deferred until its recorded revisit trigger active-continuation Active continuation of an attributed upstream item data-integrity Durable or active-context correctness and ownership defect eva-direct Direct impact on Electric Sheep supported Hermes/LCM paths P2 Significant supported-path regression or bounded correctness failure upstream-evidence Preserves links and attribution to the upstream report or pull request upstream-pr Imported upstream pull-request evidence labels Aug 16, 2026
@100yenadmin

Copy link
Copy Markdown
Member Author

Triage: your red CI is stale, not broken — and here is the actual blocker.

This PR shows a failing check, but the failure is a CodeQL infrastructure error from 2026-08-16, not a problem with your code:

##[error]Encountered a fatal error ... CodeQL could not process any code written
in JavaScript/TypeScript. Exit code was 32

This is a Python repo, so that extractor found nothing to analyse and aborted. The job has passed on every PR landed since, so the configuration is already fixed and this PR is simply carrying an old red. It needs a re-run, not repair. Twelve of the sixteen "failing" PRs in this repo are in the same position — detail and recommendation in #241.

The real blocker is the rebase. This branch is ~40 commits behind and conflicts against current main. That is what needs doing before it can be evaluated on its merits.

Recorded disposition: NEEDS-REBASE. Not closed, not abandoned — it is a feature branch whose capability I verified is still absent from main. Priority order in this lane is external contributions, then correctness fixes, then reproducible issues, then feature drafts, so this sits behind the correctness work rather than being forgotten.

If you want it prioritised sooner, say so on the PR and I will pick it up next.

@100yenadmin

Copy link
Copy Markdown
Member Author

Thank you @ai-ag2026. This draft mirrors upstream hermes-lcm#363, which was closed upstream without being merged. Its preservation record (#100) is closed. It depends on #162 and targets engine/reconcile code that has been largely rewritten since, through v0.24.0 and v0.24.1. Closing as a stale mirror. If a lossless sidecar for ignore-pattern drops is still wanted, please refile it against current main together with the sidecar substrate.

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

Labels

accepted-defer Concrete accepted work deferred until its recorded revisit trigger active-continuation Active continuation of an attributed upstream item data-integrity Durable or active-context correctness and ownership defect eva-direct Direct impact on Electric Sheep supported Hermes/LCM paths P2 Significant supported-path regression or bounded correctness failure upstream-evidence Preserves links and attribution to the upstream report or pull request upstream-pr Imported upstream pull-request evidence

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants