Repository navigation
Point comments at public sources - #318
Merged
Merged
Conversation
Several comments, docstrings and manifest notes cited internal discussion threads, message numbers and local notes that a reader of this repository cannot open. They now cite the spec section or fixture, say that spec has ruled, or say a ruling is pending. Where the pointer was all a line said, it is gone. Five comments had gone stale and now describe current behavior: the OTel LLM span paragraph, the close_invocation docstring, the failure-isolated marker keys (whose exclusion set is renamed), the 058 manifest note, and the deferral note for fixture 030. The spec now specifies 030's per-branch dispatch span, so the parse test runs it.
There was a problem hiding this comment.
🟡 Changes recommended
The documented OTel lifecycle still permits unbounded retained invocation state, and one corrected precedence comment remains contradicted by the handler documentation.
2 open findings
What changed in this PR
Updates internal comments and docstrings to reference public specifications and current behavior.
Changes:
- Removes inaccessible coordination and local-note references.
- Corrects stale observability lifecycle and metadata comments.
- Enables parsing and round-trip checks for observability fixture 030.
| File | Description |
|---|---|
.github/dependabot.yml |
Removes a private planning reference. |
conformance.toml |
Refreshes proposal and fixture notes. |
src/openarmature/checkpoint/migration.py |
Removes an internal proposal reference. |
src/openarmature/graph/observer.py |
Cites the public observability specification. |
src/openarmature/llm/providers/openai.py |
Updates the wire-stability citation. |
src/openarmature/observability/langfuse/observer.py |
Removes private task references. |
src/openarmature/observability/otel/observer.py |
Updates LLM-span and lifecycle documentation. |
src/openarmature/prompts/prompt.py |
Replaces message references with specification citations. |
tests/conformance/test_case_vocabulary.py |
Rewords vocabulary rationale without private notes. |
tests/conformance/test_checkpoint.py |
Removes an internal ruling reference. |
tests/conformance/test_conformance_adapter.py |
Removes an inaccessible coordination citation. |
tests/conformance/test_fixture_parsing.py |
Activates fixture 030 parsing and round-trip coverage. |
tests/conformance/test_observability.py |
Removes private wiring references. |
tests/conformance/test_observability_langfuse.py |
Refreshes fixture activation notes. |
tests/unit/test_failure_isolation_middleware.py |
Updates metadata-precedence commentary. |
tests/unit/test_llm_provider.py |
Removes a private task pointer. |
tests/unit/test_observability_langfuse.py |
Replaces internal coordination references. |
tests/unit/test_observability_metadata.py |
Renames and explains unmapped-span exclusions. |
tests/unit/test_observability_otel.py |
Refreshes parallel-dispatch commentary. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The OTel close_invocation docstring recommended shutdown() for long-lived observers. shutdown() is process-final, and until it or close_invocation() runs, an invocation's root span stays open and is not exported. The docstring now says so; the lifecycle fix is tracked in #319. The Langfuse failure-isolated handler explained its caller-first order with "would win", which reads like current behavior. It now says "would otherwise win".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Several comments, docstrings and manifest notes cited internal discussion threads, message numbers and local notes that a reader of this repository cannot open. Each now cites the spec section or fixture it depends on, says that spec has ruled, or says a ruling is pending. Where the pointer was all a line said, it is gone.
Five comments had gone stale and now describe current behavior:
close_invocationdocstring: the observer does not release an invocation's state on its own.openarmature.failure_isolatedmarker keys: no §8.4.x table maps the span, so the keys are unreserved, and the caller set is applied first. The exclusion set and its paired test are renamed to match.conformance.toml: observability §4.4 settles the conflict it described.Testing: the changes are comments and docstrings, except that the parse test now runs 030's two cases. Full suite: 2341 passed, 481 skipped, two more passing than main.