Skip to content

feat: record recurring failures and gate their repair (#1557) - #1586

Merged
Teingi merged 8 commits into
masterfrom
workbuddy/main-94d4d492
Sep 18, 2026
Merged

Teingi merged 8 commits into
masterfrom
workbuddy/main-94d4d492

Conversation

@AlexStocks

Copy link
Copy Markdown
Contributor

Closes #1554

Implements RFC #1557 (recurring failure repair) with an append-only recurrence ledger and evidence-bound runtime recording.

What changes are included

  • Add ExperienceContent.failure with stable failure signatures, repair surfaces, and verification criteria.
  • Add pure recurrence matching and verdict logic, the single runtime write-path orchestrator, and append-only persistence tables/repository.
  • Record selected, recurred, avoided, and unknown only from positive evidence; read/prepare paths do not write recurrence events.
  • Add ScopeStats.recurrence aggregation and API/OpenAPI generated models, mapping, and schema updates.
  • Add focused unit, persistence, runtime, statistics, CLI, contract, and cross-component E2E tests.
  • Add the Chinese PRD/design diagrams and synchronize troubleshooting guidance in English and Chinese.

Compatibility

ScopeStats.recurrence is required in the public API. This is a breaking response-schema change; consumers must accept the new recurrence block. The recurrence tables are append-only and do not add a new artifact family or MCP tool.

Validation

  • Focused recurrence and affected tests: 109 passed.
  • API contract and JS operation tests: 48 passed.
  • Integration manifest: 26 passed.
  • ruff check tests/ src/: passed.
  • ruff format --check tests/ src/: passed.
  • Generated schema and enabled server OpenAPI both match openapi/powercontext.yaml exactly.
  • Full Windows unit-test comparison: implementation tree 2252 passed, 58 skipped, 47 failed; clean baseline 2177 passed, 58 skipped, 46 failed. The extra implementation-tree failure was the stale CLI stats fixture and is fixed in this PR; the remaining failures are existing Windows/platform/environment failures in the same groups as baseline.
  • make check integration and prek hooks pass; ty check remains non-zero with 7 pre-existing diagnostics in multiprocessing stubs and Windows-only os.WNOHANG tests.

AI usage statement

Implemented and validated with OpenAI Codex (GPT-5-based coding agent). The agent generated and edited code, tests, OpenAPI generated artifacts, and documentation; all changes were reviewed against the repository contract and verified with the commands listed above.

Add failure signatures, append-only recurrence observations, runtime ledger orchestration, persistence, statistics, API contract updates, and the related documentation and tests. Keep recurrence evidence write-only on the outcome path and expose the required ScopeStats.recurrence block.

Constraint: ScopeStats.recurrence is a required breaking API field; Windows-only baseline failures and seven pre-existing ty diagnostics remain outside this change.

Tested: 109 focused tests; 48 contract tests; 26 integration-manifest tests; ruff check and format check; generated schema and enabled server OpenAPI match.

Not-tested: make check remains non-zero only because of the seven baseline ty diagnostics; full unit test comparison is 2252 passed, 58 skipped, 47 failed on the implementation tree versus 2177 passed, 58 skipped, 46 failed on clean baseline.

Co-authored-by: OmX <omx@oh-my-codex.dev>
Signed-off-by: Xin.Zh <alexstocks@foxmail.com>
Ignore Mermaid diagrams in license-eye because the checker cannot determine their comment style, and update the existing base-access E2E assertion for the optional Experience failure field.

Constraint: Preserve the required ScopeStats.recurrence and ExperienceProposal.failure contract without weakening validation.

Tested: targeted base-access E2E test and API contract/JS tests pass; Ruff check and format check pass; licenserc parses as YAML.

Co-authored-by: OmX <omx@oh-my-codex.dev>
Signed-off-by: Xin.Zh <alexstocks@foxmail.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unresolved critical and moderate findings affect evidence integrity, concurrency, runtime provenance, Review routing, and statistics.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Implements evidence-bound recurring-failure tracking with append-only persistence, runtime recording, Review routing, statistics, API updates, tests, and documentation.

Changes:

  • Adds failure metadata, recurrence matching, and verdict logic.
  • Adds runtime ledger recording and persistence tables.
  • Updates statistics, OpenAPI models, generated schemas, and mappings.
  • Adds tests and bilingual operational documentation.
File summaries
File Reviewed change
tests/test_cli.py Updates CLI statistics fixtures.
tests/test_api_contract.py Verifies recurrence API contracts.
tests/e2e/test_recurring_failure_repair.py Adds cross-component recurrence scenarios.
tests/builtin/runtime/test_statistics.py Tests recurrence statistics.
tests/builtin/runtime/test_recurrence_ledger.py Tests runtime ledger recording.
tests/builtin/persistence/test_recurrence_repository.py Tests recurrence persistence.
tests/builtin/artifacts/experience/test_search.py Tests failure search rendering.
tests/builtin/artifacts/experience/test_recurrence.py Tests recurrence matching and verdicts.
tests/builtin/artifacts/experience/test_models.py Tests failure model validation.
src/powercontext/server/mapping.py Maps recurrence API values.
src/powercontext/http/_generated/schema.py Updates generated schemas.
src/powercontext/http/_generated/models.py Updates generated API models.
src/powercontext/http/__init__.py Exports transport models.
src/powercontext/builtin/statistics/models.py Defines recurrence statistics models.
src/powercontext/builtin/statistics/aggregation.py Aggregates per-scope recurrence data.
src/powercontext/builtin/statistics/__init__.py Exports statistics types.
src/powercontext/builtin/runtime/statistics.py Loads recurrence statistics.
src/powercontext/builtin/runtime/relational.py Integrates recurrence and Review routing.
src/powercontext/builtin/runtime/recurrence.py Orchestrates runtime recurrence recording.
src/powercontext/builtin/persistence/tables.py Defines recurrence tables.
src/powercontext/builtin/persistence/recurrence.py Persists append-only recurrence events.
src/powercontext/builtin/persistence/__init__.py Exports recurrence persistence.
src/powercontext/builtin/artifacts/experience/search.py Renders Experience failure data.
src/powercontext/builtin/artifacts/experience/recurrence.py Implements recurrence matching and evidence rules.
src/powercontext/builtin/artifacts/experience/prompts.py Updates Experience generation guidance.
src/powercontext/builtin/artifacts/experience/models.py Defines structured failure metadata.
src/powercontext/builtin/artifacts/experience/__init__.py Exports failure types.
src/powercontext/artifacts/models.py Adds immutable artifact values.
openapi/powercontext.yaml Extends the API contract.
docs/zh/prd/1557_recurring_failure_repair_prd.md Adds the Chinese PRD.
docs/zh/docs/operate/troubleshoot.md Updates Chinese troubleshooting guidance.
docs/zh/design/1557_recurring_failure_repair_sequence-diagram.mermaid Adds the recurrence sequence diagram.
docs/zh/design/1557_recurring_failure_repair_class-diagram.mermaid Adds the recurrence class diagram.
docs/en/docs/operate/troubleshoot.md Updates English troubleshooting guidance.
Review details

Suppressed comments (8)

src/powercontext/builtin/artifacts/experience/recurrence.py:190

  • A Handoff may cite the same immutable Experience revision more than once. This comprehension preserves duplicate refs, so one matching revision is counted twice and match_result becomes ambiguous, even though freeze_candidate_set has correctly deduplicated the snapshot; that suppresses the expected recurred event. Deduplicate eligible refs by immutable ref identity before counting them.
    return tuple(
        ref
        for ref, content in sorted(candidates, key=lambda pair: _ref_identity(pair[0]))
        if _stored_signature_key(content) == key
    )

src/powercontext/builtin/artifacts/experience/recurrence.py:279

  • The ledger columns and generated digests use the canonical 71-character form sha256:<64 lowercase hex>, but this validator accepts any string with only the prefix. Values such as sha256:not-a-digest can therefore survive model validation and be persisted as locators, weakening the immutable evidence identity; validate the exact length and lowercase hexadecimal body here.
        if not self.item_digest.startswith(_DIGEST_PREFIX):
            raise ValueError("item_digest must be a sha256 digest")  # noqa: TRY003

src/powercontext/builtin/artifacts/experience/search.py:45

  • Prepared context calls this renderer, but the new failure block only emits the recall cue and symptom. The repair_surface and both verification bindings are therefore stored and exposed by the API but omitted from the context the agent receives, so a recalled failure cannot route the repair or know which condition/check to exercise. Include those fields in the rendered block.
    if content.failure is not None:
        lines.append(f"Failure cue: {content.failure.signature.recall_cue}")
        if content.failure.signature.symptom is not None:
            lines.append(f"Symptom: {content.failure.signature.symptom}")

src/powercontext/builtin/persistence/recurrence.py:141

  • This has the same silent-failure problem for observation writes: any integrity violation is returned as False, so a malformed event can be mistaken for an already-recorded event and the cursor can advance without the ledger row. Restrict the no-op result to a verified duplicate of the relevant uniqueness key and re-raise unrelated violations.
        try:
            await connection.execute(insert(RECURRENCE_OBSERVATION_TABLE).values(**values))
        except IntegrityError:
            return False

src/powercontext/builtin/runtime/recurrence.py:123

  • link is None conflates an outcome with no receipt and one whose handoff_receipt_ref is present but cannot be resolved. The latter is a provenance gap, but this branch snapshots scope heads and records recurred, turning missing linkage into apparent recall failure. Only a genuinely receipt-less outcome should use scope_heads; unresolved receipt windows should remain unclassified or be surfaced as coverage gaps.
        candidates: _Contents = contents if link is not None else await self._experience_heads(connection)

src/powercontext/builtin/runtime/recurrence.py:413

  • Handoff citations are not required to be unique, so this slice can pass duplicate revisions into _contents. A single Experience cited twice then appears as two eligible candidates and is classified as ambiguous; enough duplicates can also consume the 64-entry bound before later distinct revisions are considered. Deduplicate by exact ArtifactRef identity before applying the limit.
        citations = handoff_experience_citations(handoff.content)[:MAX_RECURRENCE_CANDIDATES]
        return await self._contents(connection, citations)

src/powercontext/builtin/runtime/relational.py:1743

  • This proposal targets the exact revision frozen in the match. If a newer revision is published while a Handoff still cites the older one, _validate_target rejects this candidate because only the current head may be targeted; the exception occurs in the same transaction as record_window, rolling back the append-only recurrence rows and cursor. Stale revisions need to be skipped/deferred or explicitly retargeted so Review proposal failure cannot erase the ledger event.
                        reason=proposal.reason,

src/powercontext/builtin/runtime/statistics.py:259

  • If one active Handoff contains a missing or stale Experience citation, get_many() raises and this handler returns an empty tuple, dropping every other valid citation from the coverage calculation. A scope with one valid unlinked citation and one missing citation will therefore report zero gaps; resolve citations independently or use a partial batch lookup and retain successful results.
        try:
            experiences = await self._artifacts.get_many(connection, self._scope_id, tuple(cited))
        except RepositoryNotFoundError:
            return ()
  • Files reviewed: 37/37 changed files
  • Comments generated: 5
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/powercontext/builtin/persistence/recurrence.py
Comment thread src/powercontext/builtin/runtime/recurrence.py Outdated
Comment thread src/powercontext/builtin/runtime/relational.py Outdated
Comment thread src/powercontext/builtin/artifacts/experience/models.py
Comment thread src/powercontext/builtin/persistence/recurrence.py Outdated

@Teingi Teingi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed 4ee2d2ae. All 146 focused tests passed, but separate SQLite probes reproduced these three issues and the three existing findings I replied to. The probes cover the public runtime and ledger behavior; they do not include a real LLM or OceanBase run.

Comment thread src/powercontext/builtin/artifacts/experience/recurrence.py
Comment thread src/powercontext/builtin/runtime/recurrence.py
Comment thread src/powercontext/builtin/artifacts/experience/recurrence.py Outdated

@Teingi Teingi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Rechecked 5f51b7f. I recommend addressing the P1 and three P2 findings before merging. The signature-specific false positive is detailed in my reply to the existing discussion; the other three findings are inline below.

The 210 focused tests passed on this exact head, and I reran all four counterexamples using public SQLite runtime/repository probes and a disposable local MySQL 8.0.45 instance. The third recurrence now reaches Review successfully. These probes do not include a real OceanBase or external LLM run.

Comment thread src/powercontext/builtin/persistence/tables.py Outdated
Comment thread src/powercontext/builtin/review/service.py Outdated
Comment thread src/powercontext/builtin/runtime/recurrence.py Outdated
AlexStocks and others added 2 commits September 16, 2026 13:24
Lore: A scope-head match must use every active Experience head because its persisted candidate snapshot is evidence for an immutable recurrence decision.

Constraint: Keep the independent 64-entry statistics Handoff scan unchanged; this only removes undocumented ledger candidate truncation.

Tested: 78 focused recurrence tests and 48 API contract tests pass; Ruff, targeted ty, and git diff checks pass.

Not-tested: Full Windows prek remains blocked by nine pre-existing artifact-processing and os.WNOHANG ty diagnostics outside this change.

Co-authored-by: OmX <omx@oh-my-codex.dev>
@AlexStocks

Copy link
Copy Markdown
Contributor Author

针对 5f51b7f 的复查结论,当前 Head 8202c77f 已完成以下修复:

  • candidate_set_mode 已从 VARCHAR(16) 扩展为 VARCHAR(32),覆盖 handoff_citations 与 scope_heads;
  • Experience candidate admission 现在会解析匹配 failure item 内嵌的 Source、Artifact、Memory 引用,缺失引用不再构成 failure evidence;
  • ledger 匹配候选不再对 Handoff citations 或 scope heads 作未文档化的 64 条截断;新增 SQLite 回归覆盖 64 条普通 Experience 后的唯一匹配 Experience,确认会得到 matched 并写入 recurred。

关于 signature-specific false positive:该反例在当前 RFC/PRD 的既有规则下仍是可表达的边界,而不是已被实现遗漏的检查。P0-11 规定 recurred 以失败 item 本身(WorkClaim.text 或 TaskCheck.name)与 recall_cue 的精确归一化匹配判定;P0-28 则仅把 check_subject 用于 avoided 路径中通过的绑定检查。将该 observation 场景改为不产生 recurrence,会改变这两个既有绑定的语义,需要先形成 RFC/PRD 层面的 contract 决策,而不应在本 PR 中静默改写为另一套判定规则。

当前 Head 的 quality、Python 3.11-3.14、SQLite/OceanBase acceptance 及其余可见检查均已通过。该结论仅覆盖当前 CI 和已处理评论,不替代所需的人工 review 决策。

Lore: Refresh the recurring failure repair PR on top of the latest master while keeping the generated HTTP API surface derived from the merged OpenAPI contract.

Constraint: Resolve only the generated model conflict; keep recurrence behavior and the new generic Artifact API both intact.

Tested: make contract-test; uv run pytest -q tests/builtin/artifacts/experience/test_recurrence.py tests/builtin/persistence/test_recurrence_repository.py tests/builtin/review/test_service.py tests/builtin/runtime/test_recurrence_ledger.py tests/builtin/runtime/test_statistics.py tests/e2e/test_recurring_failure_repair.py; uv run pytest -q tests/e2e/test_topic_memory_generic_api.py; uv run ruff check src/powercontext/http/_generated/models.py src/powercontext/http/_generated/schema.py tests/test_api_contract.py; git diff --cached --check.

Co-authored-by: OmX <omx@oh-my-codex.dev>
@AlexStocks

AlexStocks commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor Author

@Teingi current head df8e9392 has no merge conflict, no current unresolved review thread, and all visible checks are green. The prior review findings have been addressed or replied to. Please continue review when you have time.

@Teingi Teingi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed df8e9392. I recommend fixing the P1 and P2 below before merging.

All 218 focused tests passed. Separate SQLite probes reproduced both findings, including a valid-evidence retry case with one injected transient Review failure. I also confirmed the column-width fix on MySQL 8.0.45 in strict mode and the candidate-snapshot fix beyond 64 entries. Normal third-recurrence Review admission succeeds. This validation does not include a real OceanBase or external LLM run.

Comment on lines +79 to +80
async with connection.begin_nested():
await connection.execute(insert(RECURRENCE_MATCH_TABLE).values(**values))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[P1] Keep recurrence savepoints inside the outer SQLite transaction

With the current SQLite profile, if the window has only read before this call, there is no actual BEGIN before SAVEPOINT. Releasing the savepoint commits the ledger write, so a later failure cannot roll it back. The same applies to append_observation().

On this head, both in-memory and file SQLite retain one match and one event after an outer rollback; an explicit-BEGIN control retains neither. Through the public runtime, I used valid evidence and injected one transient error before the third recurrence's Review insert. After the error, recurred=3 was already persisted. Retrying advanced the cursor but returned candidate_count=0 with an empty Inbox: _record_recurrences() treats the retained event as a replay and skips the proposal.

Please ensure these savepoints participate in a real outer transaction, and cover rollback/retry of the ledger, candidate, and cursor together.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 774a0de7.

append_match() and append_observation() now ensure SQLite has a real outer BEGIN before opening their nested savepoints. This keeps the ledger writes inside the incubation transaction even when the window only performed reads before the append. Added rollback regressions for both match and observation writes after read-only work; both now disappear after the outer rollback.

Local validation:

  • 33 passed for tests/builtin/persistence/test_recurrence_repository.py, tests/builtin/runtime/test_recurrence_ledger.py, and tests/e2e/test_recurring_failure_repair.py.
  • ruff check, ruff format --check, git diff --check, and scoped ty check passed on the changed files.
  • Full ty check still reports pre-existing unrelated diagnostics in artifact_processing.py, opencode.py, and test_artifact_processing.py; none point at this change.

GitHub checks are running on 774a0de7.

Comment on lines +313 to +315
item = _item(outcome, failure_ref)
if item is None:
continue

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[P2] Resolve nested evidence before recording recurrence verdicts

The admission fix validates nested citations when proposing a failure Experience, but this ledger path still checks only the Outcome item and its digest. It never resolves the item's evidence. _record_avoided() has the same gap.

On this head, I first approved a failure Experience using valid evidence, then used public Source capture to ingest failed checks citing SourceRef(source_type='content', source_id='never-existed'). Two incubation windows still produced recurred=1 and recurred=2. In another scope, a condition and passing check with that nonexistent citation produced avoided=1 and reset an existing streak from 1 to 0.

Please resolve the relevant nested Source, Artifact, and Memory references in the current scope before freezing matches or recording terminal verdicts. Unresolvable evidence must remain insufficient; rejecting a candidate only when the third recurrence reaches Review leaves the earlier ledger entries incorrect.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 774a0de7.

The recurrence ledger now validates the nested evidence on the exact TaskOutcome items before recording terminal verdicts. _freeze_matches() validates the failure item evidence before freezing a match that can become recurred; _record_avoided() validates both the condition and passed-check evidence before writing avoided. Unresolvable Source, Artifact, or Memory citations now leave that item insufficient instead of writing an incorrect ledger verdict.

Added regressions for both sides: a failed check citing a missing Source no longer records recurred, and a passed condition/check pair citing a missing Source no longer records avoided.

Local validation:

  • 33 passed for tests/builtin/persistence/test_recurrence_repository.py, tests/builtin/runtime/test_recurrence_ledger.py, and tests/e2e/test_recurring_failure_repair.py.
  • ruff check, ruff format --check, git diff --check, and scoped ty check passed on the changed files.
  • Full ty check still reports pre-existing unrelated diagnostics in artifact_processing.py, opencode.py, and test_artifact_processing.py; none point at this change.

GitHub checks are running on 774a0de7.

Lore: SQLite savepoints can commit independently when no real outer BEGIN exists, and recurrence verdicts must be backed by resolvable nested evidence.
Constraint: Ledger append savepoints must roll back with the incubation transaction, and terminal recurred/avoided verdicts must not be written when their TaskOutcome item evidence cannot be resolved in the current scope.
Tested: uv run pytest -q tests/builtin/persistence/test_recurrence_repository.py tests/builtin/runtime/test_recurrence_ledger.py tests/e2e/test_recurring_failure_repair.py
Tested: uv run ruff check src/powercontext/builtin/persistence/recurrence.py src/powercontext/builtin/runtime/recurrence.py src/powercontext/builtin/runtime/relational.py tests/builtin/persistence/test_recurrence_repository.py tests/builtin/runtime/test_recurrence_ledger.py tests/e2e/test_recurring_failure_repair.py
Tested: uv run ruff format --check src/powercontext/builtin/persistence/recurrence.py src/powercontext/builtin/runtime/recurrence.py src/powercontext/builtin/runtime/relational.py tests/builtin/persistence/test_recurrence_repository.py tests/builtin/runtime/test_recurrence_ledger.py tests/e2e/test_recurring_failure_repair.py
Tested: uv run ty check src/powercontext/builtin/persistence/recurrence.py src/powercontext/builtin/runtime/recurrence.py src/powercontext/builtin/runtime/relational.py tests/builtin/persistence/test_recurrence_repository.py tests/builtin/runtime/test_recurrence_ledger.py tests/e2e/test_recurring_failure_repair.py
Co-authored-by: OmX <omx@oh-my-codex.dev>

@Teingi Teingi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@Teingi
Teingi merged commit 7b53762 into master Sep 18, 2026
22 checks passed
AlexStocks added a commit to AlexStocks/powercontext that referenced this pull request Sep 18, 2026
Lore: Resolve PR oceanbase#1596 conflicts after PR oceanbase#1586 landed on master.

Constraint: Keep both recall-gate ExperienceSearchOutcome exports and recurrence failure model exports/imports.

Tested: uv run pytest -q tests/builtin/review/test_service.py

Tested: uv run pytest -q tests/builtin/runtime/test_recall_sufficiency.py tests/e2e/test_recall_sufficiency_gate.py tests/builtin/artifacts/experience/test_recurrence.py tests/builtin/runtime/test_recurrence_ledger.py

Tested: uv run ruff check src/powercontext/builtin/artifacts/experience/__init__.py tests/builtin/review/test_service.py

Tested: uv run ruff format --check src/powercontext/builtin/artifacts/experience/__init__.py tests/builtin/review/test_service.py

Tested: git diff --check

Not-tested: Full tox and live external LLM runs were not rerun for this merge-conflict-only update.

Co-authored-by: OmX <omx@oh-my-codex.dev>
@PsiACE
PsiACE deleted the workbuddy/main-94d4d492 branch September 28, 2026 03:05
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.

feat: localize recurring failures to memory components and gate their repair (Recuris)

3 participants