Skip to content

[wave 1 · R2] Trajectory/experience-memory subsystem + scaling fixes + citable delivery + benchmark evidence - #436

Merged
stephenschoettler merged 345 commits into
stephenschoettler:mainfrom
100yenadmin:upstream-wave-1
Aug 3, 2026
Merged

stephenschoettler merged 345 commits into
stephenschoettler:mainfrom
100yenadmin:upstream-wave-1

Conversation

@100yenadmin

@100yenadmin 100yenadmin commented Jul 23, 2026 •

Copy link
Copy Markdown
Contributor

What this PR is

The consolidated wave-1 release: the V2 trajectory/experience-memory subsystem, the two scaling fixes found by
our 389× single-store probe, the query-path spend-guard configurability (supersedes #434), the summary-hit
store_id fix, the reference-strict citable-delivery engine, and the benchmark evidence trail
(bench/, F20–F37).

Measured results (all paired, pinned, reproducible — provenance blocks in bench/)

  • LongMemEval-V1: 455/500 (91.0%) on the consolidated base — paired vs the banked 444/500: net +11 on 29
    discordant rows, p=0.061 (reported as measured, not claimed as significant); instrument fail-closes 0 vs
    the prior run's 8 (F36, F37).
  • LongMemEval-V2 agentic: 298/451 (66.1%), tag bench-H6-P4-298.
  • Latency: −56.3 s/question end-to-end vs file-scan exploration (p<0.0001, 48/60 faster).
  • 389× scaling probe (F31 → F34): recall cliff eliminated (0.000 → 0.233 at ~200k messages, out-recalling
    file-scan at every rung); raw-query empty rate 100% → 0%. Full-coverage scan cost published: 20–45 ms at
    ≤2k sessions, 5.6 s at 20k — the ANN successor is Feature: include thread/topic scope in host-provided conversation_id #171. (An earlier "267 ms at 200k" figure measured the
    broken build's partial scan; it does not ship.)
  • Evidence-delivery causality: injecting missing gold evidence flips 23/35 wrong answers (p=1.9e-05).

What we found wrong and fixed (the honest part)

Review guide (for maintainers)

Highest-leverage files: search_query.py (sanitizer — six adversarial findings already fixed in review rounds,
regression-tested), vector_store.py (batched scan + cache semantics), tools.py (recall arms), the
lcm_trajectory_* subsystem (disjoint from V1's message store). Fork-side review: 7 rounds
(Codex sol·max, CodeRabbit, evaOS bot) — logs linked in bench/release-kit/. Known open harness-side issues
(NOT this PR): our reports upstream at LongMemEval-V2 #6/#7 and fork #165.

Relationship to other PRs

Supersedes #423 (closed; its answer-layer commits measured at no V1 delta — F32) and #434 (fix carried here
verbatim; closing with pointer). #436's earlier test-infra fix (the 3.13 encoder-thread leak) is already in.

…ead payload scans

The content_scope degrade regressions exercise the full-text payload
scan on a combined head with the externalized-payload-search train,
which resolves its directory from the engine home. The lightweight
SimpleNamespace fixture engine lacks one; real engines always have it.
…bound

Builds on the source-recency ordering fix (f9a240a): the DAG migration
adds latest_at without backfilling legacy rows, and a bare
latest_at DESC sorts those NULLs last — silently dropping legacy
summaries out of the bounded candidate window on upgraded databases.
COALESCE to created_at per-row so upgraded archives keep chronological
ordering. Regression: test_bounded_scan_keeps_null_latest_at_legacy_rows_by_created_at.
Add the chunk-corpus chunker: conversational/heads/full content policies,
~600-token turn-aligned windows with one-sentence overlap, error-signature
extraction for tool results, and (store_id, chunk_index, char_span) records
that map each chunk back to lcm_expand.
Add ensure_chunk_tables / verify_chunk_schema / chunk_schema_missing mirroring
the embeddings_v1 discipline: lazy additive tables (never created on a stock
install), structural verification over the marker, keyed (chunk_id,
identity_hash) with chunk profiles sharing lcm_embedding_profile under
task='chunk'.
- Allow task='chunk' identities; task-scope profile activation so summary and
  chunk profiles coexist (each with its own active profile).
- Extract corpus-agnostic _publish_under_lease from the summary publish CAS and
  reuse it for chunks (no forked concurrency logic).
- Add lazy chunk_vectors_v1 schema init, _write_chunk_row, record/publish chunk
  embeddings, bounded-candidate knn_chunks with the full|bounded|none coverage
  contract (message-keyed: direct source column, no lineage walk), and
  archive_chunks_for_messages soft-archiving on message purge.
Databases touched by interim development builds carry a numeric
schema_version ahead of this build's ladder while their actual schema is
the v5 shape plus named feature markers. The generic 'upgrade the plugin'
refusal was wrong for this case (no newer plugin exists).

- classify_version_mismatch(): read-only v5-shape vs genuinely-newer
  classification, reusing verify_embedding/temporal_rollup helpers; errs
  toward genuinely_newer so an unrecognised shape is never downgraded.
- refuse_schema_version_too_new(): interim-stamp refusal now names the
  remediation command; genuinely-newer keeps the restore-backup guidance.
- remediate_interim_schema_stamp(): dry-run-by-default, backup-first apply
  (caller-owned backup), refuses genuinely_newer, never auto-downgrades.
- /lcm doctor repair schema-stamp [apply]: opens the DB independently
  (read-only for preview), backup-first on apply, matching doctor UX.
- Tests for classify, refusal guidance, dry-run/apply, genuinely-newer
  refusal, and the command path.

Kept clear of db_bootstrap.py's FTS integrity-check region (owned by
fix/async-fts-integrity) to keep the assembly merge clean.
- default_chunk_model: voyage -> voyage-context-4; local providers reuse the
  configured model (local-first posture unchanged).
- embed_contextualized dispatcher: uses a provider's embed_contextualized when
  present, else flattens to the plain embed_documents path and regroups by doc.
  (Live voyage-context-4 wire-shape is a follow-up; the plain fallback yields
  correct per-chunk vectors and is what the chunk backfill uses today.)
Extend the backfill command with a chunk corpus that REUSES the shipped
lease/inflight/uncertain machinery unchanged (mark_inflight/mark_dispatched/
owned_inflight_transition key on (embedded_id, identity_hash) and are corpus-
agnostic). Chunk discovery streams policy-chunked messages; apply publishes via
publish_chunk_embedding_under_lease with the same CAS/crash-safety semantics.
Default --corpus summary is byte-identical (summary path untouched). Adds
LCM_EMBED_CONTENT_POLICY config. Dry-run works without a registered chunk
profile (estimates over default_chunk_model).
Real-data acceptance against the actual interim-stamped operator DB showed
the reused final-shape verifiers rejected EARLY feature-table variants
(lcm_rollups without generation/lease_nonce/failed_at and no
lcm_rollup_invalidations; lcm_embedding_profile keyed on model_name without
identity_hash/data_version), misclassifying a genuine interim stamp as
genuinely_newer.

- classify_version_mismatch: the numeric stamp only certifies CORE shape.
  interim_stamp = core v5 matches exactly AND every extra table is a known
  family prefix (lcm_rollup/lcm_embedding/lcm_chunk) or FTS shadow —
  regardless of the feature tables' internal shape (owned by each feature's
  own marker-gated init/verify). genuinely_newer stays for core mismatch or
  any non-family extra table.
- remediate apply now drops each family whose final-shape verifier fails
  (derived caches: rollups rebuild from the DAG, vectors re-backfill),
  including family-owned triggers, before resetting the stamp. Dry-run lists
  what would be dropped with rebuild hints; passing families are never
  touched.
- /lcm doctor repair schema-stamp surfaces would_drop/dropped lines + hints.
- End-to-end test: early-variant fixture -> interim_stamp, dry-run lists
  drops, apply drops + resets, then refuse passes and RollupStore +
  VectorStore reconstruct the final shape clean.

Hard acceptance on a fresh copy of the real operator DB: classify=interim,
apply drops the 6 early tables + resets to v5, refuse passes, and both
feature stores construct clean (verifiers return []).
Add engine._archive_chunks_for_messages mirroring _purge_embeddings_for_nodes
(best-effort, embeddings-gated). Wire it into the retention session-scope
message delete (same transaction, via archive_chunks_for_messages_on_connection)
and into transcript-GC tool-result rewrites (stale-content chunks archived).
…ease-test/2026-07-17

# Conflicts:
#	command.py
- retrieval_core: run_chunk_knn (chunk-corpus KNN, mirrors run_knn) and
  hydrate_chunk_hits (chunk_id -> store_id/span/excerpt message-excerpt hits,
  keyed by store_id so RRF fuses against FTS raw hits).
- embedding_provider: VoyageProvider.rerank (rerank-2.5-lite, one API call,
  single absolute deadline; raise=skip).
- config: rerank_enabled (LCM_RERANK_ENABLED, default off).
Forever-memory surface: one tool searching the entire local database by
meaning across all conversations. Three arms via retrieval_core (no duplicated
plumbing) — FTS raw (all sessions) + summary KNN (no filter) + chunk KNN (no
filter) — RRF-fused (chunk hits dedupe against FTS by store_id), optional
voyage rerank-2.5-lite (default off, skips silently on failure), then a soft
scope_bias + 30d-half-life recency prior (boosts, never filters). Bounded,
char-capped, honest degrade matrix (embeddings-off => FTS-only). Adds the
LCM_RECALL schema and the lcm_grep forward-pointer sentence.
Wire lcm_recall into engine.get_tool_schemas (after lcm_grep) and the
handle_tool_call dispatch map, the __init__ Path-A _TOOLS registry list, the
plugin.yaml provides_tools manifest, and the README + docs/retrieval-tools.md
tool tables (keeps the tool-contract sync + documentation tests green).
Seeds summaries + chunks + raw messages across three synthetic sessions and
asserts cross-session recall without a filter, scope_bias + recency boosts
(not filters), chunk/FTS store_id dedupe, include filtering, rerank apply/
skip/failure fallback, and the embeddings-off degrade to the FTS arm.
lcm_recall promised 'all conversations, all time' but the summary/chunk KNN
arms inherited lcm_grep's embedding_bounded_scan_rows (2000), which enumerates
only the 2000 MOST-RECENT vectors — structurally hiding the oldest memories.
On the 3,683-summary real DB the canonical DASHBOARD SPRINT v1.5.1 / Fleet v1.0
archives (recency rank ~3.4k) were unreachable. Thread an optional scan_rows
override through run_knn/run_chunk_knn (default None => unchanged, lcm_grep
stays byte-identical) and give recall its own config bound (recall_scan_rows,
LCM_RECALL_SCAN_ROWS, default 25000, still deadline-guarded). Live smoke now
returns coverage=full and recalls both archives cross-session.
Locks in the 'all time' contract: recall's summary + chunk KNN arms must pass
recall_scan_rows (not grep's embedding_bounded_scan_rows) to the VectorStore,
so the oldest memories stay reachable.
…cm_doctor tool

Mirrors the /lcm doctor text path: a prior non-blocking background
integrity scan persists fts_integrity_failed:<table> when it finds
corruption without rebuilding; the JSON doctor tool now reports it as a
fail-class check with the explicit repair guidance (deferred follow-up
from fix/async-fts-integrity, applied at train assembly once tools.py
was free of concurrent agent ownership).
The registration/host-capability suites enumerate the exact declared
tool set; lcm_recall (new in the recall train) registers correctly and
belongs in the expectation.
@100yenadmin

Copy link
Copy Markdown
Contributor Author

Full thread sweep complete — all 32 open review threads dispositioned and resolved. Breakdown: 11 confirmed already-fixed at HEAD (each reply cites the fixing commit and current file:line); 13 validated-real on default-off/non-delivery surfaces, tracked on our fork backlog with full validation notes (per this PR's declared stopping rule); 8 fixed here in batch 3 (head a2f519a): six adjudicated P1s on the wave's delivery/certification paths (query-view watermark at retrieval start; dated repeated-event keys; stored occurrence anchors over caller payloads; grounded explicit assertion subjects; question-bound distinct keys; enumerated sum operands) plus two adjacent P2s (how-long-ago interval routing; out-of-range epoch rejection). Every batch-3 fix carries a regression test proven failing-then-passing; the full suite on the batch head reproduces our documented environment-only failure set exactly. Adjudication method: independent source-validation of every thread, plus an adversarial refutation pass on all claimed P1s — the two downgrades to P2 came from that pass. Ready for your review; the branch remains merge-clean against current main.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a2f519a324

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread selective_compiler.py
Comment thread assertion_state.py
Comment thread assertion_extraction.py
Comment thread evidence_pack.py
Comment thread requirements_compiler.py
Comment thread requirements_compiler.py Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fa00ec9888

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread reasoning.py Outdated
Comment thread answer_contract.py
Comment thread assertion_extraction.py Outdated
Comment thread requirements_compiler.py Outdated
@100yenadmin

100yenadmin commented Aug 2, 2026 •

Copy link
Copy Markdown
Contributor Author

Validation verdict on fa00ec9 (your five-finding remediation): all five CONFIRMED — good fixes. Our review pipeline source-validated each against the tree and ran your tests: the selective-compiler as-of cutoff (with trusted-store timestamp hydration), the assertion-state truncation-to-unknown handling, the semantic-cue requirement on state-changing relations, the baseline-fact context rendering for internally-recalled facts, and the place-label date-stripping all check out; the full suite on your head reproduces our documented environment-only baseline (2,737 passing).

One measurement disclosure for the record: fa00ec9 changes certified-answer labeling behavior (notably the place-label extraction and first→last noun fallback) AFTER the benchmark numbers in this PR's body were measured. Those numbers describe the pre-fa00ec9 train; we'd flag any post-merge published number accordingly — will run a paired sanity slice on the merged result for a measured before/after.

Thread state: the ten new review threads are dispositioned — five were fixed by your commit (validated + resolved), one validated-real is tracked on our fork backlog (evidence-pack diversity cap vs. citations), and four new P1-class findings on untouched surfaces are in our adversarial-verification pipeline now; survivors get a fix batch with regression tests within the day, same discipline as batch 3.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 40f81bfdc4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread requirements_compiler.py Outdated
Comment on lines +1710 to +1711
else r"(?:attended|visited|went to|took|viewed|added|bought|returned from|"
r"participated in|completed|joined|traveled to)"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Restrict generic count verbs to completed events

For finite-enumeration questions about generic units, verbs such as bought, added, and viewed do not establish that the requested event occurred. For example, “I bought concert tickets” passes _source_event_clause(..., unit="concert"), so “How many concerts did I attend?” can count a ticket purchase as an attended concert. Fresh evidence after the prior counted-event fix is that the new adjacency check still accepts these non-occurrence verbs; use unit-specific completion verbs or otherwise verify that the verb asserts participation in the counted event.

Useful? React with 👍 / 👎.

Comment thread reasoning.py Outdated
Comment on lines +265 to +267
next_day = datetime.combine(
parsed + timedelta(days=1), datetime.min.time(), tzinfo=timezone.utc
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reject terminal dates before adding a day

When a caller supplies the syntactically valid date 9999-12-31, _parse_day() accepts it, but adding one day here raises OverflowError. lcm_compute() invokes this function before its fallback handling, so the tool raises instead of returning the documented bounded invalid-date response; the evidence-pack path has the same failure. Catch the overflow or reject dates whose exclusive end boundary is not representable.

Useful? React with 👍 / 👎.

Comment thread reasoning.py
Comment on lines +1387 to +1389
without_citations = re.sub(r"\s*\[lcm:\d+:\d+-\d+\]", "", text).strip()
if trace.result.casefold() not in without_citations.casefold():
return VerificationDecision("fallback", "candidate does not preserve the verified result")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Reject candidate prose that negates the verified result

When candidate_answer contains the canonical result as a substring but explicitly rejects it, this verifier still returns verified. For example, a trace whose result is 5 items accepts It was not 5 items [lcm:1:0-3] or 5 items is incorrect [lcm:1:0-3], because the citations, numbers, and units remain unchanged; lcm_compute() then returns that contradictory candidate instead of the canonical answer. Require a constrained affirmative rendering or reject negating/contradictory surrounding prose.

Useful? React with 👍 / 👎.

Comment thread reasoning.py Outdated
Comment on lines +1187 to +1194
values = [float(operand.value) for operand in selected] # type: ignore[arg-type]
if time_dimension and unit in time_factors:
values = [
value * time_factors[str(operand.unit)] / time_factors[unit]
for value, operand in zip(values, selected)
]
if plan.operation == "sum":
result_value = sum(values)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve integer precision during arithmetic

When an exact operand is an integer above the binary64 precision boundary, converting every value to float changes it before the deterministic calculation. For example, summing the explicitly grounded integers 9007199254740993 and 1 produces 9007199254740992 here rather than 9007199254740994; sufficiently large finite operands can also overflow the result to Infinity. Keep integer-only calculations as integers and use bounded decimal arithmetic for non-integral values.

Useful? React with 👍 / 👎.

Comment thread reasoning.py
if any(not isinstance(operand.value, (int, float)) for operand in selected):
return ComputationDecision("fallback", reason="numeric operand missing")
units = {operand.unit for operand in selected if operand.unit}
requested_unit = normalize_unit(plan.result_unit)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Carry the requested result unit into the plan

For sum or difference questions that explicitly request an output unit, compile_evidence_plan() never populates EvidencePlan.result_unit, so requested_unit is always None here. A question such as “What is the difference in hours between the two durations?” with operands 120 minutes and 1 hour is therefore reported as 60 minutes rather than the requested 1 hour. Parse the requested result unit from the question and store it on the plan before executing mixed-time arithmetic.

Useful? React with 👍 / 👎.

Comment thread reasoning.py
Comment on lines +1108 to +1110
if not selected:
return ComputationDecision(
"fallback", reason="no grounded evidence falls inside the resolved date window"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Revalidate cardinality after temporal filtering

When a temporal plan has a fixed cardinality, the executor validates the operand count before removing evidence outside the requested window and then only checks whether the filtered list is empty. For example, “Which three trips did I take last week?” produces an exact-three plan, but three supplied operands with one trip outside last week are accepted and returned as a two-item computed answer. Reapply exact_operands and minimum_operands to the filtered selection so a contradicted completeness premise falls back.

Useful? React with 👍 / 👎.

Comment thread reasoning.py Outdated
Comment on lines +1302 to +1303
result=" -> ".join(labels),
result_value=labels,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Select the requested ordinal after sorting events

For questions containing first, second, third, or previous, the planner chooses the order operation, but this branch always returns every sorted label. Thus “Which city did I visit second?” yields a full result such as Paris -> Rome -> Berlin rather than Rome, and “What was my previous address?” similarly exposes the entire history instead of the requested predecessor. Carry the requested ordinal into the plan and project the corresponding item after sorting.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2523ac93e1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread reasoning.py Outdated
Comment on lines +391 to +393
r"\b(?:which|what|who)\b[^?]{0,80}\b"
r"(?:was|were|is|came|happened|occurred)\s+(?:the\s+)?"
r"(previous|first|earliest|second|third)\b",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Match action-verb ordinal questions

Fresh evidence after the prior ordinal finding is that the new parser only recognizes ordinals following was, came, happened, and similar verbs. Common forms such as “Which city did I visit second?” or “What restaurant did I visit first?” still enter the order operation but leave order_index=None, so execution returns the full chronology instead of the requested item. Extend ordinal detection to action-verb constructions rather than limiting it to this copular/event-verb pattern.

Useful? React with 👍 / 👎.

Comment thread reasoning.py Outdated
Comment on lines +367 to +368
if re.search(r"(?:\$|\b(?:usd|dollars?)\b)", question, re.IGNORECASE):
return "usd"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Prioritize explicit output-unit phrases

When a duration question mentions an incidental price, this unconditional currency match overrides a later explicit output unit. For example, “What is the difference in hours between the $10 two-hour service and the $5 one-hour service?” produces result_unit="usd"; the executor then rejects the grounded hour operands as incompatible and falls back instead of computing one hour. Parse explicit in hours/as hours requests before treating any currency mention as the requested result unit.

Useful? React with 👍 / 👎.

Comment thread trajectory_store.py
Comment on lines +660 to +664
if missing:
raise TrajectorySchemaUnavailableError(
f"trajectory schema unavailable for read-only query: {missing}"
)
return

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Invoke the trajectory schema verifier on read-only open

When a current-stamped trajectory database has all expected table names but is missing a column, index, or trigger—for example after an interrupted or experimental schema creation—the read-only path accepts it here and returns without calling the already-defined _verify_trajectory_schema(). Subsequent manifest or query operations then fail with raw SQLite errors, and malformed FTS triggers can silently produce incomplete retrieval, instead of rejecting the store as TrajectorySchemaUnavailableError at binding time. Run the full verifier before accepting the read-only schema.

Useful? React with 👍 / 👎.

Comment thread requirements_compiler.py Outdated
Comment on lines +963 to +967
contract.temporal_window is None
and contract.operation
not in {"latest", "previous", "date_filter", "date_interval", "order"}
):
return True

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Apply the as-of cutoff to every contract operation

In requirements_v1 mode, an explicit historical question_as_of is ignored for scalar, sum, difference, and ordinary text contracts because this branch declares every candidate eligible without checking its observation time. For example, a February baseline row can answer “How much was my rent?” with a January cutoff, even though the evidence-pack and temporal-operation paths reject post-cutoff rows. Apply the availability boundary independently of the operation type so future knowledge cannot enter historical answers.

Useful? React with 👍 / 👎.

Comment thread assertion_extraction.py
source_span_start=start,
source_span_end=end,
subject_key=subject,
predicate_key=_canonical_predicate(row["predicate_key"]),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Ground extracted predicates in the cited span

The predicate is only syntax-validated and is never checked against the exact source span. A payload citing “Alice likes tea” can therefore use a grounded subject and value while setting predicate_key="employment.status"; publication succeeds, and a state query reports tea as Alice's employment status. Validate or derive the predicate semantics from the cited source before constructing the assertion candidate.

Useful? React with 👍 / 👎.

Comment thread assertion_extraction.py
Comment on lines +558 to +560
event_at=_timestamp(row["event_at"], f"{label}.event_at"),
valid_from=_timestamp(row["valid_from"], f"{label}.valid_from"),
valid_to=_timestamp(row["valid_to"], f"{label}.valid_to"),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Ground extracted temporal bounds in the cited source

The model-supplied event_at, valid_from, and valid_to values are accepted whenever they parse as timestamps, without verifying that the cited source expresses those dates. A payload for “I visited Paris on 2024-01-01” can set event_at to 2025-01-01 and persist a fabricated event time, causing temporal state and evidence queries to include or exclude the assertion in the wrong windows. Derive these bounds from grounded date expressions or reject values unsupported by the source.

Useful? React with 👍 / 👎.

Comment thread host_evidence.py
Comment on lines +77 to +79
json.dumps(contract, ensure_ascii=False, sort_keys=True),
"CODE_OWNED_INPUT:",
json.dumps(dict(request), ensure_ascii=False, sort_keys=True),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Filter secrets from the host selector envelope

The separate host-supplied selector path serializes every baseline quote into the prompt sent by call_auxiliary_selector() without applying evidence_compiler._SECRET_RE; the regex is only applied later to the model's returned proposal. When an answer-ready baseline contains an API key, token, or password, the credential has therefore already been transmitted to the configured auxiliary provider before validation can reject it. Remove sensitive baseline entries before constructing CODE_OWNED_INPUT.

Useful? React with 👍 / 👎.

Comment thread occurrence_time.py
Comment on lines +157 to +161
day = anchor - timedelta(days=count)
elif unit.startswith("week"):
day = anchor - timedelta(weeks=count)
else:
day = _subtract_months(anchor, count)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Bound relative-date counts before subtracting

When an exact quote contains a syntactically matched but extremely large relative duration such as 999999999999 days ago or 999999999999 months ago, converting and subtracting the count can raise OverflowError or ValueError. resolve_occurrence_time() is called during evidence hydration without containing those exceptions, so a malformed stored sentence can make evidence-pack and pre-answer compilation raise instead of returning an unknown occurrence time. Bound the count or catch date-range failures and degrade through _unknown().

Useful? React with 👍 / 👎.

100yenadmin added a commit to 100yenadmin/hermes-lcm that referenced this pull request Aug 2, 2026
…bit-identical context 60/60); endorsed with instrument receipts
@100yenadmin

Copy link
Copy Markdown
Contributor Author

Paired sanity-slice result on fa00ec9 — the instrument backs the endorsement. We ran the 60-question official web-batch paired slice on the frozen V2 instrument: a2f519a (pre) vs fa00ec9 (post), identical runtime inputs.

Result: your remediation is delivery-neutral, proven bitwise. memory_context is bit-identical on 60/60 questions across the two arms (0 content diffs, 0 token-count diffs) — the changes did not alter retrieval or delivered context on this slice at all. Scores were 19/60 vs 22/60 (net +3 against our net≥−3 no-regression bar; all 9 score flips have identical delivered context, i.e. pure reader sampling variance at temp 0.6 — we make no improvement claim from the +3). Zero instrument failures in both arms.

This upgrades our earlier source-review validation to a measured one, as promised. From our side #436 is fully clean — 32 threads resolved, all fix batches red/green tested, your five fixes now instrument-verified. The merge window is yours. Verdict record: bench/FINDING-F50-436-SLICE-VERDICT.md on our docs branch.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

ideal_hits = min(sum(1 for key in deduped if is_relevant(key)), k)
idcg = sum(1.0 / math.log2(rank + 1) for rank in range(1, ideal_hits + 1))

P1 Badge Normalize turn NDCG against all relevant evidence

For questions with multiple relevant turns, this derives IDCG only from relevant items that happened to appear in the retrieved list, so missing evidence does not lower turn-level NDCG. With three relevant turns and only one retrieved at rank 1, the function reports 1.0 because ideal_hits becomes one, whereas binary NDCG should normalize against all three relevant turns (approximately 0.469). This inflates the benchmark results for incomplete retrieval; derive the ideal ranking from the ground-truth relevance set under the declared turn/session granularity.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread reasoning.py
"count_distinct canonical keys must identify counted entities, "
"not the question subject"
)
return None

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Bind distinct keys to the counted entity

When a selector uses another explicit token from each quote as the canonical key, this branch accepts it as long as it is not the question subject. For example, two quotes saying Alice visited Paris on Monday and Tuesday can use monday and tuesday as keys for “How many cities did Alice visit?”, pass grounding, and produce a verified count of two cities. Validate that each key identifies the requested counted entity rather than merely appearing somewhere in the quote.

Useful? React with 👍 / 👎.

Comment thread assertion_extraction.py
Comment on lines +484 to +485
if kind in {"action", "event", "status"} and text == "completed":
return True

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Require completion evidence before storing completed

Fresh evidence after the earlier value-grounding fix is that this exception accepts completed unconditionally for action, event, and status assertions. An extractor payload can therefore cite “I might visit Paris,” set both values to completed, and persist a completed event even though the exact span establishes no completion. Require a past-action or explicit completion cue before allowing this derived canonical value.

Useful? React with 👍 / 👎.

Comment thread reasoning.py
Comment on lines +319 to +320
target = anchor - timedelta(days=1)
return TemporalWindow(target, anchor, "yesterday")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Bound relative windows at the minimum date

When question_date is the valid ISO date 0001-01-01 and the question contains yesterday, this subtraction raises OverflowError; the same underflow is possible in the nearby last-week and last-weekday branches. The end-of-day validator accepts this date, and lcm_compute() calls the planner without containing the exception, so a syntactically valid boundary input crashes instead of returning a bounded fallback. Catch date-range failures or reject anchors that cannot represent the requested relative window.

Useful? React with 👍 / 👎.

Comment thread evidence_compiler.py
)
for item in evidence
]
token = store.claim_build(identity)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Bind compiled views to the pre-selection corpus snapshot

When persist_view=True and a message is appended while selection or evidence compilation is running, this late claim snapshots the corpus only after the new message already exists. The selected evidence did not consider that message, but publication records the resulting view at the newer corpus generation, so later lookup sees no delta and can treat stale or incomplete evidence as current. Capture the corpus snapshot before selection starts and pass that snapshot to claim_build(), as the adaptive retrieval path does.

Useful? React with 👍 / 👎.

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.

2 participants