chore: catch fork main up to upstream v0.21.0-rc2 (47 commits behind) - #220
Conversation
Resolves PR stephenschoettler#436's CONFLICTING state. Upstream main was only ONE commit ahead — 49e99a2 'chore: prepare v0.20.0 release candidate' — and only two files genuinely conflicted, both documentation. Both resolved as a UNION, not by picking a side: - upstream's tool list is the accurate one (it adds lcm_recall and lcm_recent, which DO exist in wave-1's code: 31 and 20 non-doc files respectively — they were simply missing from wave-1's docs); - wave-1's skill-discovery lines are additional accurate content upstream had dropped. So the merged docs list all ten tools AND retain the plugin-qualified skill-loading note. Version strings take upstream's v0.20.0 throughout (plugin.yaml, README, docs/packaging.md, tests). No code conflicts — wave-1's 259 commits and upstream's release-candidate bump touched disjoint code.
…phenschoettler#361) Converting a rollback-journal database to WAL needs the exclusive lock, and SQLite can return SQLITE_BUSY for that upgrade without consulting the busy handler while sibling connections are mid-setup on the same file. Concurrent process startup (gateway + CLI + sub-agents) on a not-yet-WAL database therefore crashed sporadically with 'database is locked' from configure_connection. Wrap the journal_mode pragma in a bounded exponential-backoff retry (budget = SQLITE_BUSY_TIMEOUT_MS) and set busy_timeout first. Once the database is in WAL mode the pragma is a plain read, so steady state is unaffected. Also harden the concurrent-migration regression test that exposed this: its barrier had no timeout, so the pre-barrier failure parked the seven surviving threads forever and deadlocked the whole test run instead of reporting the error. The barrier now times out and aborts on failure, join is bounded, and the assert surfaces the root-cause exception. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
tests/test_tokens_encoder_loading.py::test_encoder_adopted_after_background_load fails intermittently (observed on CI py3.13 while 3.11/3.12/3.14 passed) with assert started.wait(timeout=2.0) -> False i.e. the patched loader was never CALLED, not that it was slow. Root cause is a cross-test race, not timing. test_count_tokens_does_not_block_on_hung_loader leaves its loader thread parked in release.wait(timeout=30); that thread outlives the test body. _encoder_loader() sets _encoder_ready = True even when _load_encoder() raises -- which is deliberate, a failed/hung tiktoken fetch must not be retried on every count_tokens() call -- so when the leftover thread finally wakes and raises, it re-marks the encoder ready. If that write lands AFTER the next test's autouse reset, _get_encoder() short-circuits at 'if _encoder_ready: return _encoder' and never spawns a loader, so the next test's patched loader never runs and its start event never fires. Fix is test-only: join any live encoder thread in the reset fixture's teardown, so no loader thread can outlive the test that started it. No production behaviour changes -- setting _encoder_ready on failure is the intended no-retry contract. NOT YET VERIFIED LOCALLY: a Phase 1A latency measurement is using the machine and running the suite would contend with it. Verify on py3.13 before this is pushed anywhere.
Conflicts resolved: - README/operator-guide tool count: both sides stale (10/8); resolved to 15 per the merged get_tool_schemas() - CHANGELOG: upstream's cut v0.20.0 (07-23) entry stands; the train's work moved under Unreleased as the consolidated release entry (version number is the maintainer's call) Also: fork-review working docs relocated from repo root to bench/release-kit/review-logs/ (transparency kept, root kept clean).
… fix cycle closed
- evidence_compiler: finite coverage counts DISTINCT grounded exact_refs - query_view: hit CAS verifies generation/rowcount + re-reads readiness - adaptive_retrieval: persisted slot refs intersect the selected set - trajectory: schema version rejected before repair DDL runs - vector_store: deadline-expired paths skip COUNT(*); totals stay meaningful from the enumerated candidate set All validated against source before fixing (triage: 5 mechanical of 14 real findings; the 9 architecture items are decided/tracked separately — see bench/DECISIONS-R3-UPSTREAM-ARCH.md on the fork docs branch for the first three). Full suite exact-name parity; 9 focused regressions.
…y remaining unguarded site of the lease-release class (upstream review batch 2, comment 3673094016; pattern matches evidence_compiler/rollup_builder)
…-maintenance fix: defer temporal rollup maintenance from session start
…r#440) into upstream-wave-1 One conflict, engine.py module scope: both sides added independent definitions (wave's assertion-extraction process slot / main's _RollupMaintenanceScheduler) — resolution keeps both verbatim. Full suite on the merged tree: 2723 passed; the 35 failures are the known environment-sensitive baseline (packaging/focus/host/path), distribution identical pre/post merge, none in merge-touched surfaces.
…ve-1 [wave 1 · R2] Trajectory/experience-memory subsystem + scaling fixes + citable delivery + benchmark evidence
…se/v0.21.0-rc1-prep chore: prepare v0.21.0-rc1 release candidate
…pr491-backup-recall-contract docs: correct backup and pre-answer provider contracts
…jk-safe-tiktoken-chunking fix: preserve Unicode boundaries in tiktoken state chunks
…se/v0.21.0-rc2 chore: prepare v0.21.0-rc2 release candidate
Fork main had never pulled upstream down. Contribution flowed the other way -- upstream b6288eb merged our own stephenschoettler#436 wave -- so main sat 47 behind while the Teams branch had already merged upstream directly and looked current. Only the branch was. Brings v0.20.0 + v0.21.0-rc1/rc2 release prep, Tosko4's async-rollup maintenance train (stephenschoettler#440), CJK-safe tiktoken chunking (stephenschoettler#492), and the WAL lock-contention retry (stephenschoettler#361), plus our stephenschoettler#436 wave in upstream's shape. Auto-merges clean: 20 files touched on both sides, zero conflicts. # Conflicts: # adaptive_retrieval.py # query_view_store.py # reasoning.py # requirements_compiler.py # tests/test_adaptive_retrieval.py # tests/test_evidence_contract.py # tests/test_query_view_store.py # tests/test_trajectory_state_semantic_expansion.py # tests/test_trajectory_store.py # tests/test_vector_store.py # tools.py # trajectory_store.py # vector_store.py
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThis PR updates release delivery for curated RC notes, revises evidence and reasoning behavior for trusted dates and bounded arithmetic, hardens assertion and retrieval consistency, moves rollup maintenance to an asynchronous leased scheduler, tightens trajectory schema and chunking checks, and adds validation and regression coverage. ChangesRelease and runtime behavior
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Session
participant LCMEngine
participant RollupScheduler
participant SummaryDAG
Session->>LCMEngine: bind lifecycle session
LCMEngine->>RollupScheduler: schedule rollup maintenance
RollupScheduler->>SummaryDAG: run maintenance with private DAG
SummaryDAG-->>RollupScheduler: completion or failure
RollupScheduler-->>LCMEngine: owner/key drain state
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
…gine CI caught this on all four Python versions -- 3 failures in the compute anchor/sidecar path (test_reasoning, test_lcm_recall), each falling back instead of computing. I resolved the tools.py conflict as either/or and took upstream's `session_dates=`. It is a UNION: fork main added `engine=`, upstream added `session_dates=`, and `ground_evidence` (reasoning.py:1197) declares both. Dropping `engine=` cost the sidecar its occurrence-time resolution, so compute reported 'fallback'. The keep-ours/keep-theirs method is what produced this. Re-audited the other two ground_evidence callers (evidence_pack.py:764, requirements_compiler.py:1213/1932) against the gate-verified resolution on teams/lcm-teams-v1: both byte-identical, no other drops.
Sign-offCI green on What CI earned hereThe first push failed on all four Python versions — 3 failures in the compute anchor/sidecar path. I had resolved the Local testing could not have caught this. This laptop reports 57 collection errors ( After the fix I re-audited the other two Resolution audit
Teams stays parked0 occurrences of Deployment impact: noneNo customer consumes this fork. Verified on a live box today: Merging. |
…m-teams-v1 main just caught up to upstream (#220) after never having done so. Pulling it back in immediately so integration debt does not re-accumulate -- the last time this branch let it build, the merge was 52 hunks through the retrieval core and hid a leak in code that never conflicted. Expected to be near-trivial: the branch already contained both parents' content, and #220's own resolutions were taken FROM this branch.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 22
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
requirements_compiler.py (1)
1213-1219: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPass
enginetoground_evidence.Line 1213 omits
engine=engine. The finite-enumeration path andtools.pypass it. Engine-dependent grounding can fail on the normal sum, difference, date-interval, and order paths.Proposed fix
grounding = ground_evidence( raw_operands, messages=engine._store, assertions=getattr(engine, "_assertions", None), as_of=question_date_as_of_epoch(contract.question_as_of), session_dates=getattr(engine, "_session_occurrence_dates", None), + engine=engine, )🤖 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 `@requirements_compiler.py` around lines 1213 - 1219, Update the ground_evidence call in the relevant requirements compilation path to pass the current engine via engine=engine, matching the finite-enumeration path and tools.py. Preserve all existing operands, messages, assertions, date, and session-date arguments.reasoning.py (1)
1495-1501: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueRecheck order hides the empty-window reason.
The cardinality recheck at lines 1495-1497 runs before the
if not selectedcheck at line 1498. When a plan declaresexact_operandsand the temporal filter removes every operand, the caller receives "date_filter requires exactly N operands" instead of the more specific "no grounded evidence falls inside the resolved date window".Both are correct fallbacks. The second is more actionable. Consider moving the empty check first.
🤖 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 `@reasoning.py` around lines 1495 - 1501, Move the `if not selected` fallback check before the `_cardinality_error(plan, selected)` recheck so an empty temporal-filtered selection returns “no grounded evidence falls inside the resolved date window”; preserve the cardinality fallback for non-empty selections.assertion_state.py (1)
119-119: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the relation-limit constant.
relations_truncatedcompares against the literal500, which must stay equal to thelimit=500passed at line 115. The two literals now drive a published correctness contract: if one changes and the other does not, truncation is either never detected or always reported.Define one module constant and use it in both places.
♻️ Single source for the relation limit
+_MAX_RELATIONS_PER_STATE_QUERY = 500raw_relations = ( - store.query_relations(assertion_ids=ids, as_of=as_of, limit=500) + store.query_relations( + assertion_ids=ids, as_of=as_of, limit=_MAX_RELATIONS_PER_STATE_QUERY + ) if ids else [] ) - relations_truncated = len(raw_relations) == 500 + relations_truncated = len(raw_relations) == _MAX_RELATIONS_PER_STATE_QUERY
reasoning.pyline 925 passes the samelimit=500and should import this constant too.🤖 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 `@assertion_state.py` at line 119, Define a module-level relation-limit constant in assertion_state.py, then replace both the limit argument near relations retrieval and the relations_truncated comparison in the relevant function with that constant. Export or expose the constant for reasoning.py to import and use in its corresponding limit=500 call, ensuring all relation limits share one source of truth.
🤖 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 `@assertion_extraction.py`:
- Around line 538-554: Document the intentional source-versus-context
distinction at the _source_supports_assertion_value call site in the assertion
candidate flow: commitment cues may be found in the full message context, while
cancellation cues and token matches must be found within the cited source span.
Add a short comment without changing the existing arguments or validation
behavior.
- Around line 465-478: Update _source_supports_assertion_value to accept a depth
parameter with a small maximum bound, and reject values once that bound is
exceeded using the existing malformed-input validation path. In both Mapping and
sequence branches, pass depth=depth + 1 to recursive calls while preserving
current value checks and behavior for inputs within the limit.
- Around line 484-485: The completed branch in _source_supports_assertion_value
must validate the source span rather than unconditionally returning true.
Require an explicit completion cue in the span, matching the existing
cancellation validation pattern, while preserving the current kind restrictions
for action, event, and status.
In `@assertion_state.py`:
- Around line 181-182: Apply the truncation-aware unknown contract to conflict
fields: update the type of conflict_assertion_ids, set both unresolved_conflict
and conflict_assertion_ids to null when relations are truncated, and preserve
their current derived values otherwise. Update the corresponding lcm_query_state
handling in tools.py, including the guards near active_assertion_ids, so null
conflict fields are emitted safely.
In `@engine.py`:
- Around line 322-344: Restructure the worker loop around the job execution and
follow-up handling so the active-key cleanup always runs in a finally block,
including when job() raises a BaseException such as SystemExit. Ensure
_active_keys.discard(key), _running_follow_up reset,
_release_key_owners_if_idle_locked(key), and condition notification execute on
every exit path, while preserving follow-up job processing when available.
In `@FINDINGS-VERDICTS-UP1.md`:
- Around line 1-2: Add a blank line between the FINDINGS-VERDICTS-UP1 heading
and the Source head list item to satisfy Markdown heading-spacing requirements.
In `@README.md`:
- Around line 198-200: Update the README verification checklist’s tool list to
include all 15 tools, adding lcm_query_state, lcm_compute, lcm_compile_evidence,
lcm_evidence_pack, and lcm_retrieve to the existing entries. Ensure the
checklist and the referenced sample-output count remain consistent with the
complete list, matching docs/operator-guide.md.
In `@reasoning.py`:
- Around line 654-662: In the ordinal handling around _requested_order_ordinal,
reject contradictory plans at compile time when exact_operands is set and
order_index is greater than or equal to that exact count. Ensure this validation
reports the impossible ordinal through the appropriate cardinality/order error
path instead of allowing _cardinality_error to report only the smaller operand
count.
- Around line 803-815: Define a module-level compiled pattern named
_WORD_NUMBER_RE from _WORD_NUMBERS with re.IGNORECASE, then replace the inline
word-number pattern construction in both _explicit_numbers and the shown
mentions logic with _WORD_NUMBER_RE. Preserve the existing finditer behavior and
word-number lookup.
- Around line 1580-1644: Reduce the Decimal context precision used during unit
conversion in the arithmetic flow around _apply_arithmetic, rather than applying
_MAX_NUMERIC_DIGITS to the entire with localcontext() block. Preserve sufficient
precision for the required six-decimal reporting and bounded-result checks,
while avoiding expansion of non-terminating conversion ratios to 1000
significant digits.
- Around line 537-549: Update the regex in _requested_order_ordinal to use
re.IGNORECASE, ensuring ordinal detection works consistently for raw or
normalized question text without changing the existing matching behavior.
- Around line 1133-1139: In the source_session_day branch, keep
source_observed_at unchanged and store the synthesized end-of-day timestamp in a
distinct local variable before performing the as_of comparison. Also reuse a
small helper for this end-of-day boundary calculation and comparison in both the
source and occurrence session-date branches, preserving their existing return
behavior.
- Around line 532-534: Restrict the bare-currency fallback in the result-unit
detection logic to questions that name no competing unit. Update the `re.search`
branch returning `"usd"` so incidental `$` or “dollars” mentions do not override
computable units such as hours, while preserving explicit currency-result
detection and the existing `None` fallback.
In `@selective_compiler.py`:
- Around line 132-145: Add a concise comment immediately before the `return
max(candidates) if candidates else None` statement in `_ref_observed_at`,
documenting that the maximum across store, raw, and date-derived timestamps
enforces the latest-signal cutoff and must not be changed to `min` or
first-match.
- Around line 114-131: Update the store hydration lookup in the compiler around
store.get to catch sqlite3.Error, while retaining the existing safe fallback of
treating failed lookups as no stored value. Remove or preserve conversion guards
only as appropriate, but ensure database failures from MessageStore.get do not
escape the selective compilation path or fail the request.
In `@tests/test_assertion_extraction_state.py`:
- Around line 460-499: Add a positive case to
test_state_changing_relation_requires_semantic_cue_in_exact_quote using a quote
such as “Actually I prefer coffee now” that matches the supersedes cue pattern,
then call parse_assertion_extraction with the supersedes relation and assert the
result contains exactly one relation; retain the existing rejection case for the
cue-less quote.
- Around line 401-410: Remove the vacuous store.query_assertions assertion, or
update the test to invoke the full store.publish_source persistence path and
then verify that the expected ValueError leaves no assertion rows for
person:alice.
- Around line 502-539: Extend
test_relation_truncation_marks_lifecycle_and_active_state_unknown to assert the
truncation behavior for conflict fields: verify unresolved_conflict is unknown
and conflict_assertion_ids is unset or otherwise marked indeterminate according
to the chosen contract. Update assertion_state.py so these fields are not
published as definite when relations_truncated is true, while preserving their
existing values for complete relation results.
In `@tests/test_reasoning.py`:
- Around line 913-933: Expand test_how_long_ago_uses_one_anchored_operand to
verify the bare question sets plan.interval_unit_fixed to False, then add
fixed-unit coverage for “in years?” asserting interval_unit_fixed is True and
interval_unit is “year”. Also assert that the bare question reports a multi-year
interval in years and a three-week interval in weeks, preserving the existing
anchored-operand setup.
In `@tests/test_rollup_builder.py`:
- Around line 463-467: Widen the asynchronous timing thresholds in
tests/test_rollup_builder.py at lines 463-467 and 815-819 from 0.15 seconds to
1.0 second: update the bind_elapsed assertion after engine.on_session_start and
the shutdown_elapsed assertion in the corresponding shutdown test, preserving
the existing timing checks and regression coverage.
In `@tests/test_selective_compiler.py`:
- Around line 266-279: Add a one-line comment immediately before the direct SQL
update in test_historical_cutoff_hydrates_trusted_store_timestamp explaining
that it intentionally leaves observed_at NULL to exercise the timestamp fallback
in _ref_observed_at, rather than using _evidence(observed_at=...). Preserve the
existing private connection access and test setup.
In `@trajectory_store.py`:
- Around line 641-665: Restrict the fallback around the schema-version lookup to
genuine missing-column cases only; do not treat lock or other
sqlite3.OperationalError failures as legacy tables. Update the logic near the
lcm_trajectory_corpora schema check to distinguish the absent schema_version
column before returning, while propagating unrelated database errors unchanged,
and add a regression test covering a non-schema OperationalError such as a lock.
---
Outside diff comments:
In `@assertion_state.py`:
- Line 119: Define a module-level relation-limit constant in assertion_state.py,
then replace both the limit argument near relations retrieval and the
relations_truncated comparison in the relevant function with that constant.
Export or expose the constant for reasoning.py to import and use in its
corresponding limit=500 call, ensuring all relation limits share one source of
truth.
In `@reasoning.py`:
- Around line 1495-1501: Move the `if not selected` fallback check before the
`_cardinality_error(plan, selected)` recheck so an empty temporal-filtered
selection returns “no grounded evidence falls inside the resolved date window”;
preserve the cardinality fallback for non-empty selections.
In `@requirements_compiler.py`:
- Around line 1213-1219: Update the ground_evidence call in the relevant
requirements compilation path to pass the current engine via engine=engine,
matching the finite-enumeration path and tools.py. Preserve all existing
operands, messages, assertions, date, and session-date arguments.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8081adc2-e448-4b6a-8919-46a36dcd6374
📒 Files selected for processing (54)
.github/ISSUE_TEMPLATE/bug_report.yml.github/release-notes/v0.21.0-rc1.md.github/release-notes/v0.21.0-rc2.md.github/workflows/release.ymlCHANGELOG.mdFINDINGS-VERDICTS-UP1.mdREADME.md__init__.pyadaptive_retrieval.pyanswer_contract.pyassertion_extraction.pyassertion_state.pybench/release-kit/review-logs/DELTA-REVIEW-PACKET.mdbench/release-kit/review-logs/FINDINGS-VERDICTS-R2.mdbench/release-kit/review-logs/FINDINGS-VERDICTS-R3.mdbench/release-kit/review-logs/FINDINGS-VERDICTS.mdbench/release-kit/review-logs/REGRESSION-REPORT.mdbench/release-kit/review-logs/REVIEW-PACKET.mdcommand.pydb_bootstrap.pydocs/operator-guide.mddocs/packaging.mdengine.pyescalation.pyevidence_pack.pyplugin.yamlquery_view_store.pyreasoning.pyrequirements_compiler.pyselective_compiler.pystore.pytests/test_adaptive_retrieval.pytests/test_answer_contract.pytests/test_assertion_extraction_state.pytests/test_assertion_lifecycle_tools.pytests/test_crash_safe_wal.pytests/test_embedding_backfill.pytests/test_evidence_contract.pytests/test_evidence_pack.pytests/test_lcm_command.pytests/test_lcm_core.pytests/test_lcm_engine.pytests/test_packaging_install.pytests/test_reasoning.pytests/test_release_workflow.pytests/test_rollup_builder.pytests/test_rollup_introspection.pytests/test_selective_compiler.pytests/test_tokens_encoder_loading.pytests/test_trajectory_state_semantic_expansion.pytests/test_trajectory_store.pytests/test_vector_store.pytools.pytrajectory_store.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: test (3.13)
🧰 Additional context used
🪛 ast-grep (0.45.0)
tests/test_packaging_install.py
[info] 937-946: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
{
"hits": [
{
"exact_ref": f"lcm:{fact_id}:0-{len(fact)}",
"content": fact,
}
]
}
)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
tests/test_evidence_contract.py
[info] 694-696: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
concert_result, indent=2
)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
requirements_compiler.py
[warning] 1719-1719: Regex pattern passed to re is built from a non-literal (variable, call, concatenation, or f-string) value. If that value is attacker-controlled it can introduce a malicious pattern with catastrophic backtracking (ReDoS). Use a hardcoded literal pattern, or validate/escape untrusted input with re.escape() and bound the regex complexity before compiling.
Context: re.search(pattern, normalized)
Note: [CWE-1333] Inefficient Regular Expression Complexity.
(redos-non-literal-regex-python)
reasoning.py
[warning] 809-813: Regex pattern passed to re is built from a non-literal (variable, call, concatenation, or f-string) value. If that value is attacker-controlled it can introduce a malicious pattern with catastrophic backtracking (ReDoS). Use a hardcoded literal pattern, or validate/escape untrusted input with re.escape() and bound the regex complexity before compiling.
Context: re.finditer(
r"\b(?:" + "|".join(_WORD_NUMBERS) + r")\b",
quote,
re.IGNORECASE,
)
Note: [CWE-1333] Inefficient Regular Expression Complexity.
(redos-non-literal-regex-python)
🪛 LanguageTool
FINDINGS-VERDICTS-UP1.md
[grammar] ~4-~4: Ensure spelling is correct
Context: ...IDATED/FIXED: hit CAS checks generation/rowcount and re-reads readiness before returning...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
CHANGELOG.md
[typographical] ~9-~9: To join two clauses or introduce examples, consider using an em dash.
Context: ... additional changes yet. ## v0.21.0-rc2 - 2026-08-05 ### Changed - #492 corrects...
(DASH_RULE)
[typographical] ~19-~19: To join two clauses or introduce examples, consider using an em dash.
Context: ...eplacement characters. ## v0.21.0-rc1 - 2026-08-03 ### Highlights - Add the tr...
(DASH_RULE)
[typographical] ~56-~56: To join two clauses or introduce examples, consider using an em dash.
Context: ...- or embedding-backed paths. ## v0.20.0 - 2026-07-23 Release focus: Lossless-Claw...
(DASH_RULE)
bench/release-kit/review-logs/DELTA-REVIEW-PACKET.md
[style] ~11-~11: Consider using the typographical ellipsis character here instead.
Context: ...he mode split (finding 5's real fix):** search(..., allow_operators=False) default with h...
(ELLIPSIS)
bench/release-kit/review-logs/REGRESSION-REPORT.md
[style] ~8-~8: All-uppercase text might be considered intrusive (unless these are acronyms or it is a legal text).
Context: ...y_and_uses_output_sandbox` ## Verdict GENUINE REGRESSION, FIXED. Before the fix, the stress CLI exited ...
(ALL_UPPERCASE)
bench/release-kit/review-logs/REVIEW-PACKET.md
[style] ~26-~26: Redundant conjunctions can lead to confusion; consider removing a conjunction here.
Context: ...rror or as an unintended OPERATOR (NEAR/AND/OR/NOT semantics, column filters col:, `...
(AND_OR)
[grammar] ~27-~27: Ensure spelling is correct
Context: ...` prefix)? Does term-splitting match unicode61 exactly (unicode categories, diacritics...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
bench/release-kit/review-logs/FINDINGS-VERDICTS.md
[style] ~12-~12: Consider using the typographical ellipsis character here instead.
Context: ...IRMED-FIXED | The hint consumer selects lcm_expand(node_id=...) only when from_current_session is p...
(ELLIPSIS)
🪛 markdownlint-cli2 (0.23.2)
FINDINGS-VERDICTS-UP1.md
[warning] 1-1: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
bench/release-kit/review-logs/REVIEW-PACKET.md
[warning] 7-7: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 17-17: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 23-23: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 44-44: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🪛 zizmor (1.29.0)
.github/workflows/release.yml
[info] 27-27: action functionality is already included by the runner (superfluous-actions): use gh release in a script step
(superfluous-actions)
🔇 Additional comments (106)
trajectory_store.py (2)
694-699: LGTM!
1745-1804: LGTM!tests/test_trajectory_state_semantic_expansion.py (2)
1199-1219: LGTM!
1222-1257: LGTM!tests/test_trajectory_store.py (2)
22-22: LGTM!
264-289: LGTM!.github/ISSUE_TEMPLATE/bug_report.yml (1)
47-47: LGTM!.github/release-notes/v0.21.0-rc1.md (1)
1-26: LGTM!.github/release-notes/v0.21.0-rc2.md (1)
1-17: LGTM!.github/workflows/release.yml (1)
18-24: LGTM!Also applies to: 26-32
CHANGELOG.md (1)
7-62: LGTM!README.md (1)
201-208: LGTM!Also applies to: 247-257
docs/operator-guide.md (1)
82-113: LGTM!Also applies to: 126-138, 172-172, 242-285
bench/release-kit/review-logs/FINDINGS-VERDICTS.md (1)
1-22: LGTM!bench/release-kit/review-logs/REGRESSION-REPORT.md (1)
1-57: LGTM!bench/release-kit/review-logs/REVIEW-PACKET.md (1)
1-47: LGTM!docs/packaging.md (1)
33-33: LGTM!plugin.yaml (1)
2-2: LGTM!tests/test_lcm_command.py (1)
420-420: LGTM!Also applies to: 460-460
tests/test_lcm_engine.py (1)
1142-1142: LGTM!Also applies to: 1168-1172, 1210-1210
tests/test_packaging_install.py (3)
380-380: LGTM!
859-876: LGTM!
905-965: LGTM!tests/test_release_workflow.py (1)
6-17: LGTM!Also applies to: 19-24, 27-42, 45-56, 58-68, 71-80
bench/release-kit/review-logs/DELTA-REVIEW-PACKET.md (1)
1-24: LGTM!bench/release-kit/review-logs/FINDINGS-VERDICTS-R2.md (1)
1-19: LGTM!bench/release-kit/review-logs/FINDINGS-VERDICTS-R3.md (1)
1-21: LGTM!engine.py (8)
148-201: LGTM!
203-248: LGTM!
250-270: LGTM!
272-311: LGTM!
607-609: LGTM!
1634-1653: LGTM!
1655-1702: LGTM!
1755-1755: LGTM!command.py (2)
2419-2432: LGTM!
2494-2498: LGTM!db_bootstrap.py (2)
103-108: LGTM!
127-137: 🩺 Stability & AvailabilityConfirm that
db_bootstrap.pyimportstime.Line 127 calls
time.monotonic()on everyconfigure_connection()invocation, and line 136 callstime.sleep(). The module header is not part of this review context. If thetimeimport is absent, every connection setup raisesNameError, not only the retry path.The retry logic itself is sound: non-lock
OperationalErrorvalues re-raise immediately, and the deadline check inside theexceptclause guarantees loop termination.#!/bin/bash # Verify that db_bootstrap.py imports the `time` module and defines SQLITE_BUSY_TIMEOUT_MS. set -euo pipefail fd -t f 'db_bootstrap.py' --exec sh -c ' echo "=== $1 ===" rg -n "^\s*(import|from)\s+time\b" "$1" || echo "NO time IMPORT FOUND" rg -n "SQLITE_BUSY_TIMEOUT_MS\s*[:=]" "$1" || echo "SQLITE_BUSY_TIMEOUT_MS not defined locally" rg -n "^\s*(import|from)\s+" "$1" | head -30 ' sh {}escalation.py (4)
74-115: LGTM!
135-168: LGTM!
170-196: LGTM!
387-392: LGTM!tests/test_crash_safe_wal.py (1)
236-261: LGTM!tests/test_embedding_backfill.py (1)
1029-1038: LGTM!tests/test_lcm_core.py (1)
477-503: LGTM!tests/test_rollup_builder.py (4)
3-7: LGTM!Also applies to: 306-306, 346-346, 848-848, 927-927, 1108-1113, 1126-1126, 1150-1166
479-539: LGTM!
541-710: LGTM!
713-753: LGTM!tests/test_rollup_introspection.py (4)
5-11: LGTM!Also applies to: 31-31
176-211: LGTM!
213-231: LGTM!
233-252: LGTM!tests/test_tokens_encoder_loading.py (1)
27-36: LGTM!tests/test_vector_store.py (1)
2012-2012: LGTM!__init__.py (1)
189-202: LGTM!Also applies to: 265-265
answer_contract.py (1)
383-404: LGTM!Also applies to: 475-477
requirements_compiler.py (1)
715-734: LGTM!Also applies to: 968-970, 1704-1804, 1845-1845, 1889-1892, 1926-1937, 2070-2220, 2305-2305
store.py (1)
18-18: LGTM!Also applies to: 117-120
tests/test_answer_contract.py (1)
50-65: LGTM!Also applies to: 93-108
tests/test_evidence_contract.py (1)
9-10: LGTM!Also applies to: 168-215, 546-574, 609-747
tests/test_evidence_pack.py (1)
306-325: LGTM!query_view_store.py (1)
657-665: LGTM!tests/test_adaptive_retrieval.py (1)
549-573: LGTM!tests/test_assertion_lifecycle_tools.py (1)
374-374: LGTM!evidence_pack.py (2)
181-182: LGTM!
763-773: 🗄️ Data Integrity & IntegrationConfirm the omission of
engine=here is intentional.
tools.pyline 706-713 callsground_evidencewith bothsession_dates=andengine=. This call passes onlysession_dates=. Inreasoning.py,resolve_occurrence_time_with_trustreads the sidecar fromengine._session_occurrence_dates. Withengine=None, arelative_to_sessionoperand cannot reachanchor_trust="engine_sidecar", so it is reported aslow_trusteven when the same map is supplied throughsession_dates. Grounding still succeeds becausetrusted_session_datesupplies the anchor, so this affects trust reporting only.If the evidence-pack wire is expected to carry the same temporal-trust classification as
lcm_compute, passengine=enginehere too.#!/bin/bash # Compare every ground_evidence call site for session_dates/engine argument parity. rg -nP -A 8 '\bground_evidence\s*\(' --type=pyreasoning.py (22)
13-13: LGTM!Also applies to: 39-40
96-133: LGTM!
262-281: 🗄️ Data Integrity & IntegrationVerify no consumer asserts an exact
plan.as_dict()shape.
as_dictnow emits two new keys,interval_unit_fixedandorder_index.lcm_computepublishes this dict asplanon its public response (tools.pyline 694). Any test or downstream consumer that compares the whole plan dict for equality will fail.
requirements_compiler.pyline 1174-1193 constructsEvidencePlanwithout either field, so both take their defaults; that path is unaffected in behavior.#!/bin/bash # Find exact-equality assertions or consumers of the plan wire. rg -nP -C 4 'as_dict\(\)\s*==|\["plan"\]\s*==|plan_dict\s*==' --type=py rg -nP -C 3 'interval_unit_fixed|order_index' --type=py
417-423: LGTM!
571-591: LGTM!
690-699: LGTM!
744-769: LGTM!
772-791: LGTM!
889-889: LGTM!Also applies to: 935-935, 979-991
1012-1022: LGTM!
1079-1083: 🗄️ Data Integrity & IntegrationConfirm the
engine is Noneconjunct in the relative-occurrence rejection.This guard rejects an unanchored relative expression only when no engine is supplied. When an engine is supplied but its sidecar is empty and
source_session_dayisNone, the resolver returns the samereason == "relative_expression_without_session_date", the guard does not fire, and the operand grounds withtemporal_certified = False.Both cases carry identical trust metadata, so the presence of an engine object does not add evidence about the source day. The asymmetry means the same ungrounded relative expression is rejected on one call path and accepted on another.
If the intent is "reject when no trusted source day exists", the condition should test
source_session_day is Nonerather thanengine is None.#!/bin/bash # Find tests that pin the current asymmetric behavior before changing the guard. rg -nP -C 6 'relative occurrence requires a trusted source observation date|relative_expression_without_session_date' --type=py
1203-1203: LGTM!Also applies to: 1219-1219
1260-1269: LGTM!
1290-1311: LGTM!
1328-1339: 🩺 Stability & AvailabilityConfirm the inline scoped-flag syntax against the declared Python floor.
Lines 1331-1332 use the scoped inline flag group
(?i:...).reaccepts this form from Python 3.11 onward; earlier interpreters raisere.errorat pattern-compile time, which happens at import of this module and would break every consumer.The PR description states CI runs Python 3.11 through 3.14, so this is compatible. Please confirm the repository's declared floor matches, because a
requires-pythonof>=3.10would make this a hard import failure rather than a runtime edge case.#!/bin/bash # Read the declared Python floor from the project manifests. fd -H -t f 'pyproject.toml|setup.cfg|setup.py|\.python-version|tox.ini' -x sh -c 'echo "== $1"; rg -n "requires-python|python_requires|target-version|envlist" "$1" || true' _ {} fd -H -t f -e yml -e yaml . .github/workflows -x sh -c 'echo "== $1"; rg -n "python-version" "$1" || true' _ {}
1342-1385: LGTM!
1388-1423: LGTM!
1440-1472: LGTM!Also applies to: 1513-1515
1524-1524: LGTM!Also applies to: 1544-1547
1672-1677: LGTM!Also applies to: 1707-1707
1759-1782: LGTM!
1861-1862: LGTM!selective_compiler.py (2)
20-20: LGTM!Also applies to: 229-229, 239-243, 387-390
166-169: 🗄️ Data Integrity & IntegrationConfirm that refs with no time signal should be dropped.
When
question_cutoffis set, a ref whose_ref_observed_atreturnsNoneis skipped. A ref carries no time signal when the store lookup finds nothing (or no engine was supplied) and the raw ref has noobserved_at,timestamp, ordate.The result is fail-closed, which is the right direction for a historical cutoff. It is also a behavior change: a caller that supplies bare
{exact_ref, quote}refs together with aquestion_datenow receivesreason_code = "no_exact_evidence_handles"where it previously received handles.Both new tests supply either a
dateor an engine-backed row, so neither covers the bare-ref case. Please confirm the pre-answer path in__init__.pyalways supplies an engine or dated refs.#!/bin/bash # Inspect how baseline refs reach prepare_selective_compiler and whether they carry time signals. rg -nP -B 10 -A 8 'prepare_selective_compiler\s*\(' --type=py rg -nP -C 5 'no_exact_evidence_handles' --type=pytools.py (2)
343-347: LGTM!Also applies to: 459-463, 492-497
706-713: LGTM!tests/test_reasoning.py (4)
171-173: LGTM!Also applies to: 494-518
591-662: LGTM!Also applies to: 664-743
822-865: LGTM!Also applies to: 867-911
1038-1039: LGTM!Also applies to: 1052-1067, 1132-1167, 1169-1215
tests/test_selective_compiler.py (1)
238-263: LGTM!adaptive_retrieval.py (2)
25-25: LGTM!Also applies to: 337-337
907-911: Correct fix for the stale-publish window.Capturing the snapshot at
startand threading it intoclaim_buildcloses a real TOCTOU gap.claim_builddefaultscorpus_snapshottoself.corpus_snapshot(), which reads the corpus state at claim time; any rows appended during the retrieval would then be inside the token's baseline andpublish_readycould not detect the advance. Taking the snapshot at retrieval start makes the comparison meaningful, andtest_corpus_advance_during_retrieval_rejects_view_publicationpins thestale_before_publishoutcome.Taking the snapshot before the
lookupcall at line 913-914 is also the safe ordering: the recorded generation is never newer than the data the lookup observed.Also applies to: 1198-1201
assertion_extraction.py (3)
66-83: LGTM!
706-713: LGTM!
525-537: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe loop variable shadows the
indexparameter.
_assertion_candidatetakesindexas a keyword parameter and uses it at line 501 to buildlabel. The comprehension at line 531 rebindsindexin the enclosing function scope. No defect exists today becauselabelis materialized at line 501, before the shadowing, and lines 533 and 546 readlabelrather thanindex.The trap is that any future code in this function that reads
indexafter line 531 gets the loop remnant, andlabelwould then disagree with it. Rename the loop variable.Also confirm that
_canonical_subjectalways returns atype:nameform. Line 525 indexessubject.split(":", 1)[1]unguarded, so a colon-free subject raisesIndexErrorrather than a validationValueError.♻️ Rename the loop variable
if not any( - source_tokens[index:index + len(identity_tokens)] == identity_tokens - for index in range(len(source_tokens) - len(identity_tokens) + 1) + source_tokens[offset:offset + len(identity_tokens)] == identity_tokens + for offset in range(len(source_tokens) - len(identity_tokens) + 1) ):> Likely an incorrect or invalid review comment.assertion_state.py (1)
30-30: LGTM!Also applies to: 175-180, 183-191, 199-203
tests/test_assertion_extraction_state.py (1)
366-386: LGTM!
| if isinstance(value, Mapping): | ||
| return all( | ||
| _source_supports_assertion_value( | ||
| source, item, kind=kind, context=context | ||
| ) | ||
| for item in value.values() | ||
| ) | ||
| if isinstance(value, (list, tuple)): | ||
| return all( | ||
| _source_supports_assertion_value( | ||
| source, item, kind=kind, context=context | ||
| ) | ||
| for item in value | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Bound the recursion depth over object_value.
_source_supports_assertion_value recurses through Mapping and sequence values with no depth limit. object_value arrives from the extraction payload, which is model-generated, so its nesting depth is not controlled by this module. A deeply nested value raises RecursionError.
RecursionError is not a ValueError, so it does not travel the validation-rejection path that every other malformed input takes. It escapes parse_assertion_extraction as an unexpected exception.
Add a depth parameter and reject beyond a small bound.
🛡️ Add a depth bound
def _source_supports_assertion_value(
source: str,
value: Any,
*,
kind: str,
context: str | None = None,
+ depth: int = 0,
) -> bool:
context = source if context is None else context
+ if depth > 8:
+ return False
if value is None:
return TruePass depth=depth + 1 in both recursive calls.
🤖 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 `@assertion_extraction.py` around lines 465 - 478, Update
_source_supports_assertion_value to accept a depth parameter with a small
maximum bound, and reject values once that bound is exceeded using the existing
malformed-input validation path. In both Mapping and sequence branches, pass
depth=depth + 1 to recursive calls while preserving current value checks and
behavior for inputs within the limit.
| if kind in {"action", "event", "status"} and text == "completed": | ||
| return True |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
completed bypasses source grounding entirely.
Every other branch in _source_supports_assertion_value checks the value against the source. This branch returns True for any action, event, or status assertion whose value is the literal "completed", with no reference to source or context.
Compare the neighbouring branches:
- line 482-483 (
commitment) requires acommit|promise|willcue in the context. - line 486-487 (
status/canceled) requires the cancellation cue in the span. - line 484-485 (
completed) requires nothing.
This matters because completion drives lifecycle transitions. parse_assertion_extraction line 683-689 permits a fulfills relation when from_row.kind is in {action, event, status} and to_row.kind is commitment. An extractor can therefore emit an action assertion with value completed, cite any arbitrary span, and close out a commitment. The new validation, whose stated purpose is to keep assertion values inside their exact source span, does not stop it.
Require a completion cue in the span, matching the pattern used for cancellation.
🛡️ Require an explicit completion cue
+_COMPLETION_CUE = re.compile(
+ r"\b(?:complet(?:e|ed|es|ing)|finish(?:ed|es|ing)?|done|"
+ r"submitted|delivered|wrapped\s+up)\b",
+ re.IGNORECASE,
+) if kind == "commitment" and text in {"committed", "promised"}:
return bool(re.search(r"\b(?:commit|promise|will)\b", context, re.IGNORECASE))
if kind in {"action", "event", "status"} and text == "completed":
- return True
+ return bool(_COMPLETION_CUE.search(source))📝 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.
| if kind in {"action", "event", "status"} and text == "completed": | |
| return True | |
| _COMPLETION_CUE = re.compile( | |
| r"\b(?:complet(?:e|ed|es|ing)|finish(?:ed|es|ing)?|done|" | |
| r"submitted|delivered|wrapped\s+up)\b", | |
| re.IGNORECASE, | |
| ) | |
| if kind in {"action", "event", "status"} and text == "completed": | |
| return bool(_COMPLETION_CUE.search(source)) |
🤖 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 `@assertion_extraction.py` around lines 484 - 485, The completed branch in
_source_supports_assertion_value must validate the source span rather than
unconditionally returning true. Require an explicit completion cue in the span,
matching the existing cancellation validation pattern, while preserving the
current kind restrictions for action, event, and status.
| source_span = snapshot.content[start:end] | ||
| kind = str(row["kind"] or "").strip().lower() | ||
| value_text = str(row["value_text"] or "") | ||
| if not _source_supports_assertion_value( | ||
| source_span, row["object_value"], kind=kind, context=snapshot.content | ||
| ) or not _source_supports_assertion_value( | ||
| source_span, value_text, kind=kind, context=snapshot.content | ||
| ): | ||
| raise ValueError(f"{label} assertion value is not in the exact source span") | ||
| return AssertionCandidate( | ||
| source_span_start=start, | ||
| source_span_end=end, | ||
| subject_key=subject, | ||
| predicate_key=_canonical_predicate(row["predicate_key"]), | ||
| object_value=row["object_value"], | ||
| value_text=str(row["value_text"] or ""), | ||
| kind=str(row["kind"] or "").strip().lower(), | ||
| value_text=value_text, | ||
| kind=kind, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Document the source versus context split.
Line 542 and line 544 pass the span as source and the whole message as context. Inside the helper, the branches disagree on which one they consult: the commitment branch at line 483 searches context, while the cancels branch at line 487 and the token match at line 489 search source.
Both choices are defensible. A commitment cue can sit outside the cited clause, while a cancellation must be inside it. The split is invisible at the call site and easy to invert by accident. Add a short comment recording the rule.
🤖 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 `@assertion_extraction.py` around lines 538 - 554, Document the intentional
source-versus-context distinction at the _source_supports_assertion_value call
site in the assertion candidate flow: commitment cues may be found in the full
message context, while cancellation cues and token matches must be found within
the cited source span. Add a short comment without changing the existing
arguments or validation behavior.
| item["unresolved_conflict"] = assertion_id in unresolved_conflicts | ||
| item["attribution"] = _attribution(item) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Conflict fields stay definite while active state becomes unknown.
This change nulls active, lifecycle_status, and active_assertion_ids when relations are truncated, because those values are derived from an incomplete relation set. unresolved_conflict at line 181 and conflict_assertion_ids at lines 204-206 are derived from the same incomplete set but are still published as definite values.
Both inputs are affected by truncation:
explicit_conflictsis filled by the loop at lines 134-146 over the truncatedrelations, so a droppedcontradictsrelation hides a real conflict and the field reports no conflict.unresolved_conflictsis intersected withactive_idsat line 153 and extended by the group-variant loop at lines 166-169, both keyed onactive_ids. A droppedcancelsrelation leaves a cancelled assertion insideactive_ids, which can invent a conflict that does not exist.
The internal consumer is protected: reasoning.py line 936 sets group_truncated and execute_plan rejects truncated latest_fact state. The public surface is not. lcm_query_state in tools.py lines 459-464 emits "active_assertion_ids": null next to a confident "conflict_assertion_ids": [...] in the same response, so a caller cannot tell that the second list is equally unreliable.
Apply the same unknown contract to the conflict fields.
🛡️ Null the conflict fields under truncation
- item["unresolved_conflict"] = assertion_id in unresolved_conflicts
+ item["unresolved_conflict"] = (
+ None if relations_truncated else assertion_id in unresolved_conflicts
+ )- conflict_assertion_ids=tuple(
- assertion_id for assertion_id in ids if assertion_id in unresolved_conflicts
+ conflict_assertion_ids=(
+ None
+ if relations_truncated
+ else tuple(
+ assertion_id
+ for assertion_id in ids
+ if assertion_id in unresolved_conflicts
+ )
),This requires the matching type change on conflict_assertion_ids at line 31, plus the same is not None guards in tools.py at lines 464 and 498-500 that active_assertion_ids already received.
Also applies to: 204-206
🤖 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 `@assertion_state.py` around lines 181 - 182, Apply the truncation-aware
unknown contract to conflict fields: update the type of conflict_assertion_ids,
set both unresolved_conflict and conflict_assertion_ids to null when relations
are truncated, and preserve their current derived values otherwise. Update the
corresponding lcm_query_state handling in tools.py, including the guards near
active_assertion_ids, so null conflict fields are emitted safely.
| while True: | ||
| try: | ||
| job() | ||
| except (Exception, asyncio.CancelledError): | ||
| logger.warning( | ||
| "LCM background temporal rollup maintenance failed for database=%s scope=%s", | ||
| key[0], | ||
| key[1], | ||
| exc_info=True, | ||
| ) | ||
| with self._condition: | ||
| if self._follow_up_key == key and self._follow_up_job is not None: | ||
| job = self._follow_up_job | ||
| self._follow_up_key = None | ||
| self._follow_up_job = None | ||
| self._running_follow_up = True | ||
| self._condition.notify_all() | ||
| continue | ||
| self._active_keys.discard(key) | ||
| self._running_follow_up = False | ||
| self._release_key_owners_if_idle_locked(key) | ||
| self._condition.notify_all() | ||
| break |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Protect the active-key bookkeeping against a BaseException escape.
The except (Exception, asyncio.CancelledError) clause does not catch other BaseException subclasses, such as SystemExit raised inside a job. If one escapes, the worker thread dies while key stays in _active_keys and its owners stay in _owned_keys. Two consequences follow:
drain_owner()withtimeout=Nonenever returns for that owner.- Every later
schedule()for the same key takes thekey in self._active_keysfollow-up branch, so that scope never runs maintenance again, even afterschedule()starts a replacement worker.
Move the state cleanup into a finally block so the key is always released.
🛠️ Proposed fix to release the active key on any exit path
- while True:
- try:
- job()
- except (Exception, asyncio.CancelledError):
- logger.warning(
- "LCM background temporal rollup maintenance failed for database=%s scope=%s",
- key[0],
- key[1],
- exc_info=True,
- )
- with self._condition:
- if self._follow_up_key == key and self._follow_up_job is not None:
- job = self._follow_up_job
- self._follow_up_key = None
- self._follow_up_job = None
- self._running_follow_up = True
- self._condition.notify_all()
- continue
- self._active_keys.discard(key)
- self._running_follow_up = False
- self._release_key_owners_if_idle_locked(key)
- self._condition.notify_all()
- break
+ try:
+ while True:
+ try:
+ job()
+ except (Exception, asyncio.CancelledError):
+ logger.warning(
+ "LCM background temporal rollup maintenance failed for database=%s scope=%s",
+ key[0],
+ key[1],
+ exc_info=True,
+ )
+ with self._condition:
+ if self._follow_up_key == key and self._follow_up_job is not None:
+ job = self._follow_up_job
+ self._follow_up_key = None
+ self._follow_up_job = None
+ self._running_follow_up = True
+ self._condition.notify_all()
+ continue
+ break
+ finally:
+ with self._condition:
+ self._active_keys.discard(key)
+ self._running_follow_up = False
+ self._release_key_owners_if_idle_locked(key)
+ self._condition.notify_all()📝 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.
| while True: | |
| try: | |
| job() | |
| except (Exception, asyncio.CancelledError): | |
| logger.warning( | |
| "LCM background temporal rollup maintenance failed for database=%s scope=%s", | |
| key[0], | |
| key[1], | |
| exc_info=True, | |
| ) | |
| with self._condition: | |
| if self._follow_up_key == key and self._follow_up_job is not None: | |
| job = self._follow_up_job | |
| self._follow_up_key = None | |
| self._follow_up_job = None | |
| self._running_follow_up = True | |
| self._condition.notify_all() | |
| continue | |
| self._active_keys.discard(key) | |
| self._running_follow_up = False | |
| self._release_key_owners_if_idle_locked(key) | |
| self._condition.notify_all() | |
| break | |
| try: | |
| while True: | |
| try: | |
| job() | |
| except (Exception, asyncio.CancelledError): | |
| logger.warning( | |
| "LCM background temporal rollup maintenance failed for database=%s scope=%s", | |
| key[0], | |
| key[1], | |
| exc_info=True, | |
| ) | |
| with self._condition: | |
| if self._follow_up_key == key and self._follow_up_job is not None: | |
| job = self._follow_up_job | |
| self._follow_up_key = None | |
| self._follow_up_job = None | |
| self._running_follow_up = True | |
| self._condition.notify_all() | |
| continue | |
| break | |
| finally: | |
| with self._condition: | |
| self._active_keys.discard(key) | |
| self._running_follow_up = False | |
| self._release_key_owners_if_idle_locked(key) | |
| self._condition.notify_all() |
🤖 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 `@engine.py` around lines 322 - 344, Restructure the worker loop around the job
execution and follow-up handling so the active-key cleanup always runs in a
finally block, including when job() raises a BaseException such as SystemExit.
Ensure _active_keys.discard(key), _running_follow_up reset,
_release_key_owners_if_idle_locked(key), and condition notification execute on
every exit path, while preserving follow-up job processing when available.
| def test_relation_truncation_marks_lifecycle_and_active_state_unknown(): | ||
| target = "a" * 64 | ||
| row = { | ||
| "assertion_id": target, | ||
| "subject_key": "user:self", | ||
| "predicate_key": "drink.preference", | ||
| "scope_key": "", | ||
| "kind": "preference", | ||
| "object_value": "tea", | ||
| "polarity": "positive", | ||
| "value_text": "tea", | ||
| "speaker_role": "user", | ||
| } | ||
| relations = [ | ||
| { | ||
| "relation_type": "confirms", | ||
| "from_assertion_id": target, | ||
| "to_assertion_id": target, | ||
| } | ||
| for _ in range(500) | ||
| ] | ||
|
|
||
| class FakeStore: | ||
| def query_assertions(self, **_kwargs): | ||
| return [row] | ||
|
|
||
| def query_relations(self, *, limit, **_kwargs): | ||
| return relations[:limit] | ||
|
|
||
| state = query_assertion_state( | ||
| FakeStore(), subject_key="user:self", predicate_key="drink.preference" | ||
| ) | ||
|
|
||
| assert state.relations_truncated is True | ||
| assert state.active_assertion_ids is None | ||
| assert state.assertions[0]["active"] is None | ||
| assert state.assertions[0]["lifecycle_status"] is None | ||
| assert state.assertions[0]["semantic_state"] == "unknown" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Good truncation test. Extend it to the conflict fields.
The construction is precise: the 500 relations are confirms, which is absent from _INACTIVE_RELATION_STATUS, so lifecycle stays empty and the assertion would read as active. The test therefore proves the truncation override rather than incidental inactivity.
unresolved_conflict and conflict_assertion_ids are not asserted. Those are the two fields that assertion_state.py still publishes as definite under truncation, despite deriving from the same incomplete relation set. Adding assertions here pins whichever contract you decide on.
💚 Suggested additional assertions
assert state.relations_truncated is True
assert state.active_assertion_ids is None
assert state.assertions[0]["active"] is None
assert state.assertions[0]["lifecycle_status"] is None
assert state.assertions[0]["semantic_state"] == "unknown"
+ # Conflict state derives from the same truncated relation set.
+ assert state.assertions[0]["unresolved_conflict"] is None
+ assert state.conflict_assertion_ids is 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_relation_truncation_marks_lifecycle_and_active_state_unknown(): | |
| target = "a" * 64 | |
| row = { | |
| "assertion_id": target, | |
| "subject_key": "user:self", | |
| "predicate_key": "drink.preference", | |
| "scope_key": "", | |
| "kind": "preference", | |
| "object_value": "tea", | |
| "polarity": "positive", | |
| "value_text": "tea", | |
| "speaker_role": "user", | |
| } | |
| relations = [ | |
| { | |
| "relation_type": "confirms", | |
| "from_assertion_id": target, | |
| "to_assertion_id": target, | |
| } | |
| for _ in range(500) | |
| ] | |
| class FakeStore: | |
| def query_assertions(self, **_kwargs): | |
| return [row] | |
| def query_relations(self, *, limit, **_kwargs): | |
| return relations[:limit] | |
| state = query_assertion_state( | |
| FakeStore(), subject_key="user:self", predicate_key="drink.preference" | |
| ) | |
| assert state.relations_truncated is True | |
| assert state.active_assertion_ids is None | |
| assert state.assertions[0]["active"] is None | |
| assert state.assertions[0]["lifecycle_status"] is None | |
| assert state.assertions[0]["semantic_state"] == "unknown" | |
| def test_relation_truncation_marks_lifecycle_and_active_state_unknown(): | |
| target = "a" * 64 | |
| row = { | |
| "assertion_id": target, | |
| "subject_key": "user:self", | |
| "predicate_key": "drink.preference", | |
| "scope_key": "", | |
| "kind": "preference", | |
| "object_value": "tea", | |
| "polarity": "positive", | |
| "value_text": "tea", | |
| "speaker_role": "user", | |
| } | |
| relations = [ | |
| { | |
| "relation_type": "confirms", | |
| "from_assertion_id": target, | |
| "to_assertion_id": target, | |
| } | |
| for _ in range(500) | |
| ] | |
| class FakeStore: | |
| def query_assertions(self, **_kwargs): | |
| return [row] | |
| def query_relations(self, *, limit, **_kwargs): | |
| return relations[:limit] | |
| state = query_assertion_state( | |
| FakeStore(), subject_key="user:self", predicate_key="drink.preference" | |
| ) | |
| assert state.relations_truncated is True | |
| assert state.active_assertion_ids is None | |
| assert state.assertions[0]["active"] is None | |
| assert state.assertions[0]["lifecycle_status"] is None | |
| assert state.assertions[0]["semantic_state"] == "unknown" | |
| # Conflict state derives from the same truncated relation set. | |
| assert state.assertions[0]["unresolved_conflict"] is None | |
| assert state.conflict_assertion_ids is 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/test_assertion_extraction_state.py` around lines 502 - 539, Extend
test_relation_truncation_marks_lifecycle_and_active_state_unknown to assert the
truncation behavior for conflict fields: verify unresolved_conflict is unknown
and conflict_assertion_ids is unset or otherwise marked indeterminate according
to the chosen contract. Update assertion_state.py so these fields are not
published as definite when relations_truncated is true, while preserving their
existing values for complete relation results.
| def test_how_long_ago_uses_one_anchored_operand(evidence_db): | ||
| messages, assertions = evidence_db | ||
| content = "I visited Paris on 2023-03-18." | ||
| store_id = _message(messages, content, "2023-03-18") | ||
| operands = _ground( | ||
| messages, | ||
| assertions, | ||
| [_raw(store_id, content, content, date="2023-03-18")], | ||
| question_date="2023-03-20", | ||
| ) | ||
|
|
||
| plan = compile_evidence_plan( | ||
| "How long ago did I visit Paris?", | ||
| "2023-03-20", | ||
| ).plan | ||
|
|
||
| assert plan.operation == "date_interval" | ||
| assert plan.exact_operands == 1 | ||
| assert plan.interval_unit == "day" | ||
| assert execute_plan(plan, operands).trace.result == "2 days" | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
This test does not exercise the new coarsest-unit path.
compile_evidence_plan("How long ago did I visit Paris?") now sets interval_unit_fixed = False, and execute_plan reports the coarsest exact calendar unit for such a plan. The interval here is 2 days, so the coarsest-fit branch selects "day" anyway. The assertions pass identically whether interval_unit_fixed is True or False, so the new flag is untested.
Add coverage that separates the two behaviors:
- assert
plan.interval_unit_fixed is Falsefor the bare question, - assert
interval_unit_fixed is Trueand unit"year"for "How long ago ... in years?", - assert a multi-year gap reports in years, and a 3-week gap reports in weeks, for the bare question.
💚 Suggested additional assertions
assert plan.operation == "date_interval"
assert plan.exact_operands == 1
assert plan.interval_unit == "day"
+ assert plan.interval_unit_fixed is False
assert execute_plan(plan, operands).trace.result == "2 days"
+
+ fixed = compile_evidence_plan(
+ "How long ago, in years, did I visit Paris?",
+ "2023-03-20",
+ ).plan
+ assert fixed.interval_unit_fixed is True
+ assert fixed.interval_unit == "year"🤖 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/test_reasoning.py` around lines 913 - 933, Expand
test_how_long_ago_uses_one_anchored_operand to verify the bare question sets
plan.interval_unit_fixed to False, then add fixed-unit coverage for “in years?”
asserting interval_unit_fixed is True and interval_unit is “year”. Also assert
that the bare question reports a multi-year interval in years and a three-week
interval in weeks, preserving the existing anchored-operand setup.
| started_at = time.monotonic() | ||
| engine.on_session_start(scope, conversation_id="slow-rollup-conversation") | ||
| bind_elapsed = time.monotonic() - started_at | ||
|
|
||
| assert bind_elapsed < 0.15 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Tight 0.15-second wall-clock thresholds risk CI flakes in tests/test_rollup_builder.py. Both tests separate asynchronous from synchronous behavior with a 150 ms budget, but each measured operation performs real SQLite work. The regression each test guards against would block for about 2 seconds, so a much wider threshold preserves the same discrimination.
tests/test_rollup_builder.py#L463-L467: raiseassert bind_elapsed < 0.15toassert bind_elapsed < 1.0; the bind also performs lifecycle writes and a possible empty-lifecycle GC pass.tests/test_rollup_builder.py#L815-L819: raiseassert shutdown_elapsed < 0.15toassert shutdown_elapsed < 1.0; the shutdown closes five SQLite connections, each runningPRAGMA wal_checkpoint(PASSIVE).
📍 Affects 1 file
tests/test_rollup_builder.py#L463-L467(this comment)tests/test_rollup_builder.py#L815-L819
🤖 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/test_rollup_builder.py` around lines 463 - 467, Widen the asynchronous
timing thresholds in tests/test_rollup_builder.py at lines 463-467 and 815-819
from 0.15 seconds to 1.0 second: update the bind_elapsed assertion after
engine.on_session_start and the shutdown_elapsed assertion in the corresponding
shutdown test, preserving the existing timing checks and regression coverage.
| def test_historical_cutoff_hydrates_trusted_store_timestamp(tmp_path): | ||
| engine = _engine(tmp_path) | ||
| past = _evidence(engine, "The first purchase cost $20.") | ||
| future = _evidence(engine, "The second purchase cost $30.") | ||
| engine._store._conn.executemany( | ||
| "UPDATE messages SET timestamp = ? WHERE store_id = ?", | ||
| [ | ||
| (1_704_067_200, int(past["exact_ref"].split(":")[1])), | ||
| (1_767_225_600, int(future["exact_ref"].split(":")[1])), | ||
| ], | ||
| ) | ||
| engine._store._conn.commit() | ||
| past["date"] = "2024-01-01" | ||
| future["date"] = "2024-01-01" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
State why this test writes SQL instead of using the _evidence helper.
_evidence already accepts observed_at=, so the raw UPDATE looks redundant. It is not. Passing observed_at= sets msg["timestamp"], which MessageStore.append normalizes into the observed_at column, and _ref_observed_at reads observed_at first. This test patches the timestamp column while observed_at stays NULL, so it exercises the timestamp fallback branch at selective_compiler.py lines 122-123 specifically.
That intent is invisible to a reader and the test reaches through two private attributes (engine._store._conn). Add a one-line comment so a future cleanup does not "simplify" it into the helper and silently stop testing the fallback.
📝 Proposed comment
engine = _engine(tmp_path)
past = _evidence(engine, "The first purchase cost $20.")
future = _evidence(engine, "The second purchase cost $30.")
+ # Patch the `timestamp` column directly and leave `observed_at` NULL, so
+ # this covers the timestamp FALLBACK in _ref_observed_at. Using
+ # _evidence(observed_at=...) would populate observed_at and test the
+ # primary branch instead.
engine._store._conn.executemany(
"UPDATE messages SET timestamp = ? WHERE store_id = ?",📝 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_historical_cutoff_hydrates_trusted_store_timestamp(tmp_path): | |
| engine = _engine(tmp_path) | |
| past = _evidence(engine, "The first purchase cost $20.") | |
| future = _evidence(engine, "The second purchase cost $30.") | |
| engine._store._conn.executemany( | |
| "UPDATE messages SET timestamp = ? WHERE store_id = ?", | |
| [ | |
| (1_704_067_200, int(past["exact_ref"].split(":")[1])), | |
| (1_767_225_600, int(future["exact_ref"].split(":")[1])), | |
| ], | |
| ) | |
| engine._store._conn.commit() | |
| past["date"] = "2024-01-01" | |
| future["date"] = "2024-01-01" | |
| def test_historical_cutoff_hydrates_trusted_store_timestamp(tmp_path): | |
| engine = _engine(tmp_path) | |
| past = _evidence(engine, "The first purchase cost $20.") | |
| future = _evidence(engine, "The second purchase cost $30.") | |
| # Patch the `timestamp` column directly and leave `observed_at` NULL, so | |
| # this covers the timestamp FALLBACK in _ref_observed_at. Using | |
| # _evidence(observed_at=...) would populate observed_at and test the | |
| # primary branch instead. | |
| engine._store._conn.executemany( | |
| "UPDATE messages SET timestamp = ? WHERE store_id = ?", | |
| [ | |
| (1_704_067_200, int(past["exact_ref"].split(":")[1])), | |
| (1_767_225_600, int(future["exact_ref"].split(":")[1])), | |
| ], | |
| ) | |
| engine._store._conn.commit() | |
| past["date"] = "2024-01-01" | |
| future["date"] = "2024-01-01" |
🤖 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/test_selective_compiler.py` around lines 266 - 279, Add a one-line
comment immediately before the direct SQL update in
test_historical_cutoff_hydrates_trusted_store_timestamp explaining that it
intentionally leaves observed_at NULL to exercise the timestamp fallback in
_ref_observed_at, rather than using _evidence(observed_at=...). Preserve the
existing private connection access and test setup.
| except sqlite3.OperationalError: | ||
| # Two very different tables reach this branch, and only one of them | ||
| # should be tolerated: | ||
| # | ||
| # LEGACY -- a corpora table written before the ``schema_version`` | ||
| # column existed. It still carries the core identity | ||
| # columns, and failing the open on it would brick a | ||
| # store that is merely old. Treat it as unversioned, | ||
| # the same way the table-absent branch above does. | ||
| # MALFORMED -- a table that is missing the core columns as well. | ||
| # Nothing can be recovered from it, and swallowing the | ||
| # error here lets the open continue until some later | ||
| # INSERT fails on an arbitrary column, reporting the | ||
| # wrong cause and doing so AFTER FTS repair has run. | ||
| # | ||
| # Distinguish them by a column the legacy table certainly has. | ||
| columns = { | ||
| str(row[1]) | ||
| for row in self._conn.execute( | ||
| "PRAGMA table_info(lcm_trajectory_corpora)" | ||
| ) | ||
| } | ||
| if "identity_digest" not in columns: | ||
| raise | ||
| return |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Restrict the legacy fallback to a missing schema_version column.
Line 641 catches every sqlite3.OperationalError. SQLite uses this exception for lock and other database failures, not only missing columns.
If a lock error occurs and clears before PRAGMA table_info runs, Line 663 accepts a current table as legacy. Writable initialization can then continue into migration or FTS repair instead of reporting the original database failure.
Catch only the missing-schema_version error, or inspect table columns before the SELECT. Add a regression test for a non-schema OperationalError.
#!/bin/bash
python - <<'PY'
import sqlite3
import tempfile
with tempfile.NamedTemporaryFile(suffix=".db") as handle:
setup = sqlite3.connect(handle.name)
setup.execute(
"CREATE TABLE lcm_trajectory_corpora "
"(singleton INTEGER PRIMARY KEY, identity_digest TEXT, schema_version INTEGER)"
)
setup.commit()
setup.close()
locker = sqlite3.connect(handle.name, isolation_level=None)
locker.execute("BEGIN EXCLUSIVE")
reader = sqlite3.connect(handle.name, timeout=0.01)
try:
reader.execute(
"SELECT schema_version FROM lcm_trajectory_corpora WHERE singleton = 1"
)
except sqlite3.OperationalError as exc:
print(f"OperationalError from a lock: {exc}")
finally:
reader.close()
locker.rollback()
locker.close()
PY🤖 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 `@trajectory_store.py` around lines 641 - 665, Restrict the fallback around the
schema-version lookup to genuine missing-column cases only; do not treat lock or
other sqlite3.OperationalError failures as legacy tables. Update the logic near
the lcm_trajectory_corpora schema check to distinguish the absent schema_version
column before returning, while propagating unrelated database errors unchanged,
and add a regression test covering a non-schema OperationalError such as a lock.
4 participants
Why this exists
Fork
mainhad never merged upstream. Its merge history is entirely internal (r1/r2 fix rounds, PR ports) — noupstream/mainmerge anywhere. Contribution flowed the other way: upstreamb6288ebmerged our own100yenadmin/upstream-wave-1(stephenschoettler#436). Nothing ever came back down.The Teams branch merged
upstream/maindirectly, which made the fork look current. Only the branch was.mainsat 47 behind / 56 ahead.What it brings
Fork-only work is preserved: 48 commits by patch-id, mostly
docs(bench)plussession_expand_v1,fts_prose_mode, and profile-scoped trajectory embeddings.Conflicts: 13 files, 45 hunks — how each was resolved
git merge-treereported clean; the real merge did not. (My dry-run grepped for<<<<<<<markers, which that output form doesn't emit — the predicate was wrong, not the merge.)11 of 13 files took the resolution from
teams/lcm-teams-v1. That branch already merged these exact two sides at65df063, and those 11 files carry zero Teams tokens there — so its version is the correct non-Teams resolution, already gate-verified (battery 21/21, default-off A/B 22/22).The 2 Teams-bearing files were resolved by hand:
vector_store.py(4 hunks) — kept fork main's_scanned_knn_resulthelper over upstream's inlineKNNResultconstruction. Verified the helper subsumes upstream's logic (samescanned/totalpolicy;deadline_expiredwhere upstream saidstopped_early), and that fork main's helper is byte-identical to the gate-verified one on the Teams branch.tools.py(1 hunk) — took upstream'ssession_dates=getattr(engine, "_session_occurrence_dates", None)over fork main'sengine=engine. Confirmed against the callee:reasoning.py:889acceptssession_dates, notengine. Keeping our side here would have been a TypeError at runtime.Teams stays parked
Verified on the merge result: 0 occurrences of
access_scope/TeamsPolicy/teams_enabled_v1, and noteams/,access_context/, oraccess_policy/directories. This changes nothing about #215, which remains deliberately unmerged.Verification
cannot import name 'LCMConfig') — and the pre-merge baseline reports the identical 57. Environment artifact, unchanged by the merge, and it's a collection failure so it proves nothing either way. CI across 3.11–3.14 is the gate here.Deployment impact: none
No customer consumes this fork.
evaos-runtime.jsonpinsowner_repo: stephenschoettler/hermes-lcmatv0.19.0 @ df9239efwithpin_mode: git_ref_and_commit. Verified on David Dorman's box today — the installed tree carries that exact commit and tree hash.Summary by CodeRabbit
New Features
Bug Fixes
Documentation