fix(sources): profile-qualify Hermes verification-ledger session identity#3227
Conversation
…tity Problem: PR #3224 fixed the unqualified observer-id collapse pattern for ATIF/ATOF (hermes_spans.py) but hermes_verification.py retained the exact pre-fix pattern -- observer_session_provider_id/ hermes_verification_session_id_for built bare `verification:<raw_id>` identities and stripped the `@profile-<key>` qualifier. Two Hermes installs (profiles) sharing a raw session id collapse their verification-ledger evidence onto one archive session. Solution: mirror #3224. observer_session_provider_id and hermes_verification_session_id_for now thread an optional profile_key through hermes_identity.qualified_session_id/split_qualified_session_id, the same shared helper hermes_spans.py/hermes_state.py use. parse_verification_evidence_db/parse_verification_evidence_db_payload accept profile_root: Path | None, compute a profile key when known, and assert parent_session_provider_id back to the state-db conversational session only when the profile is known (fail-closed -- no edge is safer than a wrong one). The `verification:` family prefix stays distinct from ATIF/ATOF's `observer:` family. Wired profile_root=Path(spec.source_path).parent at every verification-evidence call site: dispatch.py, import_explain.py, source_parsing.py, live/batch.py. Left revision_backfill.py's existing call untouched (owned surface, and it already fails closed with profile_root defaulting to None). Did not touch hermes_spans.py/hermes_identity.py (consumed read-only) or the ATIF/ATOF call sites, which are owned by another active lane. Verification: - devtools test tests/unit/sources/parsers/test_hermes_verification.py tests/unit/sources/test_dispatch_payloads.py tests/unit/sources/test_hermes_import_explain.py tests/unit/sources/test_hermes_source_freshness_integration.py tests/unit/insights/test_hermes_verification_coverage.py tests/unit/sources/test_live_watcher_catchup_order.py tests/unit/core/test_sampling.py tests/unit/sources/test_parsers_local_agent.py -> "103 passed" - mypy --strict on the 5 touched modules -> "Success: no issues found in 5 source files" - devtools verify --quick -> exit_code 0 (format/lint/mypy/render/topology/ layering/manifests/doc-commands/docs-coverage/clock-hygiene/etc all ok) - Anti-vacuity: reverted observer_session_provider_id's profile-key qualification (collapsed back to unconditional `verification:<raw_id>`) and re-ran the new collapse/family-prefix/dispatch-wiring tests -- all 3 failed with the expected unqualified-id assertion mismatches, confirming they exercise the real production mutation. Restored the fix; tests green again. Ref polylogue-y9zx Co-Authored-By: Claude <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 58 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68ebaeed2c
ℹ️ 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".
| path: Path, | ||
| *, | ||
| fallback_id: str | None = None, | ||
| profile_root: Path | None = None, |
There was a problem hiding this comment.
When revision backfill or a derived-tier rebuild reparses a retained Hermes verification database, revision_backfill._parse_one still calls parse_verification_evidence_db(sqlite_path, fallback_id=...) without the available Path(source_path).parent. Because the new default here produces an unqualified verification:<raw_id> session with no parent edge, the same raw evidence has qualified identity during live ingest but unqualified identity during rebuild; multiple profiles can collapse again and verification coverage no longer resolves to the qualified conversational session. Update that replay call just as the state-db branch immediately above it already does.
Useful? React with 👍 / 👎.
|
Coordinator review stand-in (CodeRabbit rate-limited — no findings produced). Read the full diff. Mirrors merged #3224 exactly: profile_key threaded through the shared hermes_identity helpers; Same semantic-reparse note as #3225's P1 applies: already-materialized verification-ledger observer sessions carry old unqualified ids; covered by the coordinator's post-promote targeted Hermes reprocess bead (posted separately). 103 focused tests, mypy --strict, quick-gate green. Merging. |
…rmes-reprocess + provider-usage-500 beads Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QUsH3Rhq6oAZpYPWcsZqnZ
Problem: master gained #3226 (objective-posture insight, new module + topology regen) and #3227 (hermes_verification.py profile-qualified identity: verification:<id>[@PROfile-<key>], hermes_verification_session_id_for now PRESERVES the qualifier instead of stripping it) since this branch's last rebase. Rebasing surfaced one real conflict and one silent cross-file break the rebase's textual merge did not flag: - polylogue/sources/parsers/hermes_verification.py: conflicting docstring edits (both #3227 and this branch's earlier commit touched the same observer_session_provider_id/hermes_verification_session_id_for docstrings) -- resolved to describe the composed state: profile-qualified AND correctly referencing hermes_spans's renamed atif_session_provider_id/atof_session_provider_id (this branch) instead of the retired shared observer_session_provider_id. - tests/unit/sources/parsers/test_hermes_verification.py (added by #3227, never touched by this branch's commits, so the rebase reported no conflict): test_verification_family_prefix_stays_distinguishable_from_atif_atof_observer_family imported hermes_spans.observer_session_provider_id, a name this branch retired in favor of atif_session_provider_id/atof_session_provider_id -- an ImportError, not a merge conflict, so it only surfaced by actually running the suite. Fixed to import and assert against both renamed functions (verification: stays distinguishable from BOTH observer:atif: and observer:atof:, not just the old single observer: family). Semantic composition check (per coordinator request): hermes_topology_projection.py never hardcodes a session-id shape itself -- callers resolve each artifact's session_id (see HermesArtifactInput docstring) via hermes_spans.atif_session_provider_id/atof_session_provider_id or hermes_verification.hermes_verification_session_id_for -- so the module is scheme-agnostic by construction and needed no code change. Added test_composes_with_profile_qualified_verification_ids_like_atif_and_atof in tests/unit/insights/test_hermes_topology_projection.py, which drives the REAL hermes_verification_session_id_for/hermes_identity helpers (not a hand-typed "verification:<id>" string) from a profile-qualified conversational session id and proves: the resolved verification id preserves the qualifier (not stripped, matching #3227's fix), and two different profiles resolve to two distinct, non-colliding verification session ids for the same raw Hermes session id -- exactly the guarantee ATIF/ATOF already have. docs/hermes-operators.md: "Session identity" section and item 1 updated -- previously (written before #3227 merged) claimed hermes_verification's helper "still strips" the profile qualifier and was "explicitly left out of this fix's scope"; now correctly describes it as fixed separately (polylogue-y9zx, PR #3227) using the same shared hermes_identity.py scheme. docs/plans/topology-target.yaml, docs/topology-status.md: regenerated (devtools render topology-projection && render topology-status) to reflect #3226's new objective-posture insight module -- no textual conflict during rebase, but genuinely stale without regen. Verification: - devtools test tests/unit/insights/test_hermes_topology_projection.py tests/unit/sources/parsers/test_hermes_spans.py tests/unit/sources/test_dispatch_payloads.py tests/unit/sources/test_live_watcher.py tests/unit/sources/parsers/test_hermes_verification.py -- 151 passed. - devtools verify --quick -- exit 0. Ref polylogue-fs1.14 Co-Authored-By: Claude <noreply@anthropic.com>
/#3224/#3225/#3227/#3231 receipts Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QUsH3Rhq6oAZpYPWcsZqnZ
1 participant
Summary
hermes_verification.pyretained the exact unqualified observer-id collapsepattern that PR #3224 fixed for ATIF/ATOF (
hermes_spans.py). This PRmirrors that fix for the Hermes verification-ledger importer.
Problem
Two Hermes installs (profiles) can legitimately reuse the same raw session
id.
hermes_verification.observer_session_provider_id/hermes_verification_session_id_forbuilt a bareverification:<raw_id>identity and stripped any
@profile-<key>qualifier when mapping back froma conversational session id, so two installs' verification-ledger evidence
(lint/typecheck/build/test command outcomes) silently collapsed onto one
archive session.
Solution
observer_session_provider_id(hermes_session_id, profile_key=None)andhermes_verification_session_id_fornow thread an optional profile keythrough
hermes_identity.qualified_session_id/split_qualified_session_id-- the exact shared helperhermes_spans.pyand
hermes_state.pyalready use. Theverification:family prefix staysdistinct from ATIF/ATOF's
observer:family even when both qualify thesame raw id against the same profile.
parse_verification_evidence_db/parse_verification_evidence_db_payloadaccept
profile_root: Path | None, compute a profile key when known, andassert
parent_session_provider_idback to the state-db conversationalsession only when the profile root is known (fail-closed: no edge is
safer than a wrong one).
parent_session_linkfidelity capability, aggregated acrosssessions, mirroring
hermes_spans._parent_session_link_capability.profile_root=Path(spec.source_path).parent(orpath.parent) atevery verification-evidence call site:
dispatch.py,import_explain.py,source_parsing.py,live/batch.py.revision_backfill.py's existing call untouched per scope (it alreadyfails closed with
profile_rootdefaulting toNone).hermes_spans.py/hermes_identity.py(consumedread-only) or the ATIF/ATOF call sites -- those are owned by another
active lane.
Verification
devtools test tests/unit/sources/parsers/test_hermes_verification.py tests/unit/sources/test_dispatch_payloads.py tests/unit/sources/test_hermes_import_explain.py tests/unit/sources/test_hermes_source_freshness_integration.py tests/unit/insights/test_hermes_verification_coverage.py tests/unit/sources/test_live_watcher_catchup_order.py tests/unit/core/test_sampling.py tests/unit/sources/test_parsers_local_agent.py->103 passedmypy --stricton the 5 touched production modules ->Success: no issues found in 5 source filesdevtools verify --quick->exit_code: 0observer_session_provider_id(collapsed back to unconditionalverification:<raw_id>) and re-ran the new collapse/family-prefix/dispatch-wiring tests -- all 3 failed with the expected unqualified-id
assertion mismatches. Restored the fix; tests green again.
Ref polylogue-y9zx