refactor(sources): migrate all eleven origins onto OriginSpec (polylogue-2qx.1.2)#3250
Conversation
…Spec Problem: OriginSpec (polylogue-2qx.1.1) admits all eleven current Origin tokens, but polylogue.sources.assembly.get_assembly_spec() is a second, un-declared provider-to-ProviderAssemblySpec dispatch that Claude Code, Codex, and AI Studio/Drive rely on for sidecar/title/orchestration enrichment. polylogue-2qx.2/j2zz/ih67 need one typed admission point for that hook rather than a private inventory. What changed: add OriginSpec.assembly_spec_path (a "module:ClassName" declaration of the live ProviderAssemblySpec, or None) plus validate_assembly_spec_parity(), which checks the declaration against get_assembly_spec's actual per-provider return value the same way validate_dispatch_precedence/validate_stream_parser_parity already check detector tightness and stream-parser bindings against dispatch.py. The live get_assembly_spec dispatch in polylogue/sources/assembly.py is left as the one production registry; OriginSpec's field only declares and parity-checks against it. Ref polylogue-2qx.1.2 Co-Authored-By: Claude <noreply@anthropic.com>
Problem: docs/provider-origin-identity.md documented the Provider/Origin/ Source vocabulary boundary but never named polylogue/sources/origin_specs.py as the per-origin admission authority, so a reader had no pointer from the doctrine to the mechanically parity-checked declaration it should trust over a hand-maintained list. What changed: add an "Origin admission" section naming origin_specs.py as the one per-origin declaration, listing what it declares, and pointing at the two live-production parity checks (validate_dispatch_precedence, validate_assembly_spec_parity) so future readers extending Claude/Codex/ AI Studio assembly hooks (polylogue-2qx.2/j2zz/ih67) know where the typed admission point is. Ref polylogue-2qx.1.2 Co-Authored-By: Claude <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe change adds optional assembly-spec bindings to origin specifications, declares bindings for three executable origins, validates declarations against the live assembly registry, adds unit coverage, and documents the registry’s parity-check role. ChangesOrigin assembly parity
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant OriginSpecRegistry
participant validate_assembly_spec_parity
participant get_assembly_spec
OriginSpecRegistry->>validate_assembly_spec_parity: executable origin specifications
validate_assembly_spec_parity->>get_assembly_spec: resolve each provider assembly spec
get_assembly_spec-->>validate_assembly_spec_parity: assembly spec or None
validate_assembly_spec_parity-->>OriginSpecRegistry: parity diagnostics
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: bd3fb3a1a4
ℹ️ 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".
| declares_assembly = spec.assembly_spec_path is not None | ||
| has_live_assembly = any(assembly_spec_resolver(provider) is not None for provider in spec.provider_wires) | ||
| if declares_assembly == has_live_assembly: |
There was a problem hiding this comment.
When get_assembly_spec drifts to a different class—or, for AISTUDIO_DRIVE, moves the hook from Provider.GEMINI to Provider.DRIVE—this validator still returns no diagnostic because it reduces both the declared path and all provider results to booleans. Consequently, a typo such as assembly_gemini.py:WrongClass or a wrong live provider/class binding passes the advertised parity check, leaving assembly_spec_path unreliable as the typed discovery point; derive and compare each returned object's module/class path (and provider binding) to the declaration instead.
AGENTS.md reference: AGENTS.md:L379-L381
Useful? React with 👍 / 👎.
|
Coordinator review (stand-in while CodeRabbit is pending): read the full diff. Additive-only migration completion — audit confirmed AC1/3/5 were already satisfied on master via #3051/#3087/#3088/#3092/#3201/#3228/#3246; this PR adds the genuinely missing typed assembly admission point (assembly_spec_path + validate_assembly_spec_parity against the live get_assembly_spec registry). Verified locally: all three declared paths resolve to live classes on master; parity test includes a real anti-vacuity mutation (resolver that returns None flags exactly the declaring origins). AC6 honestly documented as misframed (2qx.2 already closed without OriginSpec, blocking edge was force-closed as over-blocking). Residual noted for coordinator follow-up: cli/shell_completion_values.py _ORIGIN_DESCRIPTIONS (8/11 hand-maintained, no parity) — will be fixed in a follow-up deriving from OriginSpec. Merging on green quick-gate. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@polylogue/sources/origin_specs.py`:
- Around line 961-978: Update the assembly validation around assembly_spec_path
and assembly_spec_resolver to compare the declared module/class path with the
resolved assembly spec’s actual path, not merely whether both are present.
Preserve the existing parity diagnostic for presence mismatches, add a
diagnostic for incorrect declared paths, and add a mutation test proving a wrong
assembly path is rejected.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7df3418f-1886-4a39-911f-d1f97c2576e2
📒 Files selected for processing (3)
docs/provider-origin-identity.mdpolylogue/sources/origin_specs.pytests/unit/sources/test_origin_specs.py
| declares_assembly = spec.assembly_spec_path is not None | ||
| has_live_assembly = any(assembly_spec_resolver(provider) is not None for provider in spec.provider_wires) | ||
| if declares_assembly == has_live_assembly: | ||
| continue | ||
| diagnostics.append( | ||
| OriginSpecDiagnostic( | ||
| code="assembly_spec_parity_mismatch", | ||
| message=( | ||
| f"{spec.origin.value}: live assembly registry returns a spec " | ||
| f"({has_live_assembly}) but OriginSpec declares " | ||
| f"assembly_spec_path={spec.assembly_spec_path!r}" | ||
| ), | ||
| origin=spec.origin, | ||
| owner_path=spec.declaration.owner_path, | ||
| repair_command=spec.declaration.repair_command, | ||
| ) | ||
| ) | ||
| return tuple(sorted(diagnostics, key=lambda item: (item.origin.value, item.code))) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Validate the declared assembly path, not just its presence.
A stale or incorrect non-None assembly_spec_path passes whenever any provider wire resolves an assembly object, so the advertised module/class binding is never verified. Compare the declaration to the resolved spec’s actual module/class path and add a wrong-path mutation test.
Proposed fix
- declares_assembly = spec.assembly_spec_path is not None
- has_live_assembly = any(assembly_spec_resolver(provider) is not None for provider in spec.provider_wires)
- if declares_assembly == has_live_assembly:
+ live_paths = set()
+ for provider in spec.provider_wires:
+ assembly_spec = assembly_spec_resolver(provider)
+ if assembly_spec is not None:
+ assembly_type = type(assembly_spec)
+ live_paths.add(
+ f"{assembly_type.__module__.replace('.', '/')}.py:{assembly_type.__name__}"
+ )
+
+ if (
+ spec.assembly_spec_path is None and not live_paths
+ ) or spec.assembly_spec_path in live_paths:
continue📝 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.
| declares_assembly = spec.assembly_spec_path is not None | |
| has_live_assembly = any(assembly_spec_resolver(provider) is not None for provider in spec.provider_wires) | |
| if declares_assembly == has_live_assembly: | |
| continue | |
| diagnostics.append( | |
| OriginSpecDiagnostic( | |
| code="assembly_spec_parity_mismatch", | |
| message=( | |
| f"{spec.origin.value}: live assembly registry returns a spec " | |
| f"({has_live_assembly}) but OriginSpec declares " | |
| f"assembly_spec_path={spec.assembly_spec_path!r}" | |
| ), | |
| origin=spec.origin, | |
| owner_path=spec.declaration.owner_path, | |
| repair_command=spec.declaration.repair_command, | |
| ) | |
| ) | |
| return tuple(sorted(diagnostics, key=lambda item: (item.origin.value, item.code))) | |
| live_paths = set() | |
| for provider in spec.provider_wires: | |
| assembly_spec = assembly_spec_resolver(provider) | |
| if assembly_spec is not None: | |
| assembly_type = type(assembly_spec) | |
| live_paths.add( | |
| f"{assembly_type.__module__.replace('.', '/')}.py:{assembly_type.__name__}" | |
| ) | |
| if ( | |
| spec.assembly_spec_path is None and not live_paths | |
| ) or spec.assembly_spec_path in live_paths: | |
| continue |
🤖 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 `@polylogue/sources/origin_specs.py` around lines 961 - 978, Update the
assembly validation around assembly_spec_path and assembly_spec_resolver to
compare the declared module/class path with the resolved assembly spec’s actual
path, not merely whether both are present. Preserve the existing parity
diagnostic for presence mismatches, add a diagnostic for incorrect declared
paths, and add a mutation test proving a wrong assembly path is rejected.
…3252) ## Summary Deletes the last hand-maintained per-origin description inventory (`cli/shell_completion_values.py:_ORIGIN_DESCRIPTIONS`, 8 of 11 origins, no parity check) by declaring a required `display_description` on `OriginSpec` and deriving the CLI completion dict from `ORIGIN_SPECS`. ## Problem PR #3250's audit flagged this as the one remaining stale parallel origin inventory: shell completion for `--origin` silently omitted `beads-issue`, `grok-export`, and `unknown-export`, and nothing would catch future drift. ## Solution - `OriginSpec.display_description` (required field, declared once per origin in `sources/origin_specs.py`). - `_ORIGIN_DESCRIPTIONS` becomes a derivation over `ORIGIN_SPECS` — parity by construction. - Regression test pins the derived inventory to the exact `Origin` enum and non-empty text; blanking a description or dropping a spec fails it. ## Verification - `devtools test tests/unit/sources/test_origin_specs.py tests/unit/cli/test_command_aux_runtime.py` → 27 passed. - `devtools verify --quick` → exit 0. Ref polylogue-2qx.1.2 (residual from #3250). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01QUsH3Rhq6oAZpYPWcsZqnZ Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…pts (#3253); file meoz delete-detonation bug; yqeo incident note Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QUsH3Rhq6oAZpYPWcsZqnZ
1 participant
Summary
Audits and closes the remaining gap in polylogue-2qx.1.2 ("migrate every
current origin onto OriginSpec"). Most of this bead's scope was already
shipped to
masteruntagged across #3051/#3087/#3088/#3092/#3201/#3228/#3246(closing
polylogue-2qx.1.1's notes confirm this). This PR verifies thatprior work end-to-end, proves the parity checks actually catch regressions
(not just pass today), and closes one concrete residual gap: a second,
un-declared provider-to-assembly-hook dispatch that Claude Code, Codex, and
AI Studio/Drive depend on.
Problem
polylogue/sources/origin_specs.pyalready declares all elevenOrigintokens with lifecycle/detector-tightness/parser/fixture/coverage bindings,
and
polylogue/sources/provider_completeness.pyalready derives its rowsfrom
ORIGIN_SPECS(PACKAGE_MODE_SPECSis a tuple comprehension over it,parity-proven by
test_provider_completeness_is_a_projection_of_every_origin_spec).Dispatch-order and stream-parser-binding parity are also already
mechanically checked (
validate_dispatch_precedence,validate_stream_parser_parity) against the real production values insources/dispatch.py(RECORD_DETECTOR_PROVIDER_ORDER,STREAM_RECORD_PROVIDERS).What was genuinely missing: the bead's design explicitly calls for "Claude
Code and Codex specs [to] expose typed assembly/orchestration/title/action
extension hooks (consumed later by 2qx.2, j2zz, ih67) — a typed field on
the spec, not another registry."
polylogue.sources.assembly.get_assembly_spec()is exactly that second, un-declared provider dispatch
(
ProviderAssemblySpecfactory forProvider.CLAUDE_CODE/Provider.CODEX/Provider.GEMINI) — it existed in production but had no OriginSpecdeclaration or parity check, so a caller had no way to discover the hook
from
origin_specs.pyand no test would catch it drifting from the realregistry.
Also:
docs/provider-origin-identity.mddocumented Provider/Origin/Sourcedoctrine but never pointed at
origin_specs.pyas the per-origin admissionauthority a reader should trust over any hand-maintained list.
Solution
polylogue/sources/origin_specs.py: addedOriginSpec.assembly_spec_path(
"module/path.py:ClassName"orNone) declaring whichProviderAssemblySpeca given origin's provider wire binds. Populated forthe three origins with a real live hook:
claude-code-session→assembly_claude_code.py:ClaudeCodeAssemblySpec,codex-session→assembly_codex.py:CodexAssemblySpec,aistudio-drive→assembly_gemini.py:GeminiAssemblySpec(itsProvider.GEMINIwire is theone
get_assembly_speckeys on). Addedvalidate_assembly_spec_parity(),following the exact idiom of
validate_dispatch_precedence/validate_stream_parser_parity: it checks the declared field against thelive
get_assembly_spec(provider)return value rather than replacing thatregistry with a second one in
origin_specs.py.tests/unit/sources/test_origin_specs.py: two new tests — one assertingthe three declared hook paths (and
Nonefor two origins without a livehook), one asserting
validate_assembly_spec_parity(get_assembly_spec) == ()plus the mutation shape (a resolver that always returns
Nonemakes everyorigin that declares a hook path report
assembly_spec_parity_mismatch).docs/provider-origin-identity.md: added an "Origin admission" sectionnaming
origin_specs.pyas the per-origin declaration authority andpointing at both live-production parity checks.
none left inside my ownership (
polylogue/sources/**) — see the residualbelow for the one still outstanding elsewhere.
Residual found outside owned scope
polylogue/cli/shell_completion_values.py:_ORIGIN_DESCRIPTIONSis ahand-maintained
dict[str, str]of 8 of the 11 origin tokens (missingbeads-issue,grok-export,unknown-export) used for shell-completionhelp text, and it is not parity-checked against
Origin/ORIGIN_SPECSthe way
polylogue/agent_integration/spec.py:ORIGIN_MEANINGSalready is(that one has a
RuntimeErrorguard requiring exactOrigin-enum ordermatch — already satisfies AC4, no action needed there).
cli/**is out ofthis lane's ownership (per dispatch instructions, another lane owns it
concurrently); flagging as a follow-up rather than touching it here.
Verification
devtools test tests/unit/sources/test_origin_specs.py tests/unit/sources/test_provider_completeness.py→ 15 passeddevtools test tests/unit/sources/ -k "origin_specs or provider_completeness or dispatch or assembly"→ 144 passeddevtools test tests/unit/sources/test_parsers_props.py(protected file) → 43 passeddevtools verify --quick→ all 16 stepsok,"exit_code": 0(ruff format/check, mypy --strict, render all, topology/layering/closure-matrix, manifests, ci-workflows, doc-commands, docs-coverage, test-infra-currency, test-clock-hygiene, pytest-timeout-overrides, degrade-loudly)git checkout --), each confirmed the existing production dependency actually fails on the mutation before I added anything:_grok_spec()from the registration loop →ORIGIN_SPEC_REGISTRY.diagnostics()reportsmissing_origin_specforgrok-export.detector_tightnessbelowgemini_cli_session's (contradicting its real last-in-branch-order position) →validate_dispatch_precedence(RECORD_DETECTOR_PROVIDER_ORDER)reportsdispatch_tightness_mismatch.assembly_spec_pathdeclaration →validate_assembly_spec_parity(get_assembly_spec)reportsassembly_spec_parity_mismatchforcodex-session(this is the mutation the new code in this PR is proven against).AC matrix (polylogue-2qx.1.2)
Origintokens are declared exactly once inORIGIN_SPECS;set(by_origin) == set(Origin)is asserted intest_origin_specs_cover_the_public_enum_and_admission_lifecycles, andOriginSpecRegistry.diagnostics()emitsmissing_origin_specfor any omission (proven above).claude-code-sessionadditionally declares 6 typedOriginArtifactRules for its workflow/journal/transcript/sidecar/adopt-manifest/coordinator-stream family.grok-exportislifecycle="executable"today (it shipped a real parser via feat(sources): wire Grok account-data export parser #3201/polylogue-611 after 2qx.1.1's pilot design assumed it would stay reserved — documented at the top oforigin_specs.py); thereservedlifecycle contract itself is proven against a synthetic variant since no currentOriginis genuinely reserved.unknown-exportislifecycle="compatibility-only"with explicit fallback semantics;beads-issueis executable withissue-jsonlacquisition and its own non-acceptedcompleteness maturity caveat (no harvested schema sample yet). This PR adds the Claude/Codex/AI-Studio assembly hook declaration.validate_dispatch_precedence/validate_stream_parser_parityparity-check tightness and stream-binding againstsources/dispatch.py's realRECORD_DETECTOR_PROVIDER_ORDER/STREAM_RECORD_PROVIDERS; mutation-proven above (deleting/reordering a declaration fails).provider_completeness.py; this PR extends it to the assembly-hook axis.PACKAGE_MODE_SPECSis a direct tuple comprehension overORIGIN_SPECS, parity-proven bytest_provider_completeness_is_a_projection_of_every_origin_spec.agent_integration/spec.py:ORIGIN_MEANINGSis separately parity-guarded againstOriginat import time. Deferred:cli/shell_completion_values.py:_ORIGIN_DESCRIPTIONSremains a stale, un-parity-checked hand-maintained inventory — out of this lane's ownership (cli/**), flagged as a residual above rather than a named follow-up bead (small, mechanical, single-file fix for whichever lane next touches CLI completions).aistudio-drive'sprovider_wires=(Provider.GEMINI, Provider.DRIVE)withcollision_policyis registration-enforced (OriginSpecRegistry.registerraisesValueErroriflen(provider_wires) > 1without acollision_policy, tested intest_origin_spec_rejects_missing_fixture_and_noninjective_collision_without_policy);docs/provider-origin-identity.mdalready documented the non-reversal doctrine, and this PR adds the pointer from that doc to the enforcing code.polylogue-2qx.2(Claude Code orchestration artifact admission) already closed via PR feat(sources): admit Claude Workflow artifacts through OriginSpec #3088, and its own close reason states it shipped as aclaude_workflow_materializerconvergence stage without the OriginSpec migration this bead was meant to gate — the blocking edge was explicitly force-closed as "over-blocking" because the merged implementation disproves that ordering. So AC6 as written ("Claude/Codex extension hooks are the only admission path used by 2qx.2...") cannot be retroactively true for2qx.2.j2zz(Codex child-action lowering) andih67(Codex title enrichment) are still open; this PR's newassembly_spec_path/validate_assembly_spec_paritygives them a real typed admission point to consume going forward, but I did not modifyassembly_codex.py/ih67/j2zzscope — those remain their own beads' work, not migrated by me here.Files touched
polylogue/sources/origin_specs.pytests/unit/sources/test_origin_specs.pydocs/provider-origin-identity.mdRef polylogue-2qx.1.2
Summary by CodeRabbit
Documentation
Improvements
Tests