refactor(converter): own the ATIF conversion; harbor becomes a public-API dependency - #6
Merged
Merged
Conversation
… frozen goldens The port away from harbor's private conversion API needs a fixed point to be measured against. This is it: harbor's two private converters reached from tests only, deterministic (asserted), frozen to goldens for both synthetic fixtures (asserted equal to the live oracle, so an upstream behavior change surfaces as a named diff), and a live-corpus parity test over the newest N local sessions of each agent that skips where there is no corpus.
…-API dependency atif-converter used to call two PRIVATE harbor methods, ClaudeCode._convert_events_to_trajectory and Codex._convert_events_to_trajectory (1,029 and 741 lines, 26 and 18 upstream commits since June), through a symlink-staging layer that rebuilt the directory shape they expected. harbor documents only the ATIF data classes (harbor.models.trajectories, RFC 0001) and the validator; that is now all production code imports from it. The conversion is ours: domain/claude_code_conversion.py and domain/codex_conversion.py are parity ports of harbor 0.22.0's converters (Apache-2.0, attributed), built on the public models, reached through infrastructure/claude_code_converter.py and infrastructure/codex_converter.py, which read the files directly. No staging, no temp dirs, no private-API probe, no HarborPrivateApiMissing, no exit 127 path (the code stays in the table so it never renumbers). The pin widens to harbor>=0.22.0,<1 and litellm, which harbor lazily imported for per-call cost, is declared as our own dependency. Parity is measured, not assumed. harbor's private converters survive in ONE place, tests/harbor_oracle.py, as the oracle: deterministic (asserted), frozen to goldens for both synthetic fixtures (the live oracle must equal the frozen one, so an upstream behavior change becomes a named JSON-path diff), and diffed against our converters over the newest N local sessions (ATIF_PARITY_LIMIT=0 = all). Measured: 0 divergent paths over 2,437 Claude Code sessions and 78 Codex rollouts on this host. An ast guard pins the harbor allowlist to the two public modules; import-linter cannot express a sub-module of an external package. Two fidelity "gaps" are retired because their mechanism is gone: Claude Code's workflow_subagents_missed (our converter discovers every side file itself) and Codex's single_rollout_per_directory (one file in, one trajectory out). Their values are not reused; corpora materialized before this carry them still. The data-loss gaps stay, by choice: this is a parity port, and fixing one is a decision to diverge from the oracle, to be recorded in the fidelity policy.
…table A pre-merge review found the frozen goldens depended on a remote file: litellm loads its pricing table from raw.githubusercontent.com at import time unless LITELLM_LOCAL_MODEL_COST_MAP is set, and the goldens carry per-call cost_usd priced through it. A BerriAI price edit, or an unreachable GitHub in CI, would have failed the never-skipped golden gate with no change in this repo, and the test's docstring would have blamed harbor. The converter conftest now pins the table to the one bundled with the locked litellm (setdefault, so a run can still opt into the remote table), the Codex fixture model moves to one the bundled table prices so the cost path stays frozen rather than becoming None, and the goldens are re-frozen. Measured: the bundled table had no entry for the previous fixture model, and the golden tests failed under it. Also from the review: the Codex reader opens with an explicit utf-8 like its Claude Code sibling; the set-ordered agent extras and the fixture's single-valued cwd/gitBranch are documented as harbor behavior kept on purpose; four doc sentences that still advertised exit 127 or the retired gap as live are corrected; and a test pins that an unreadable side file becomes a ConversionError carrying the OSError.
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.
atif-converterno longer calls harbor's private methods. The Claude Code and Codex conversions are ours, ported from harbor 0.22.0 under Apache-2.0 and built on the two things harbor actually documents: the ATIF data classes inharbor.models.trajectories(RFC 0001) andharbor.utils.trajectory_validator.Why
harbor documents the data classes and the validator, and nothing that converts a native session log. We were calling
ClaudeCode._convert_events_to_trajectoryandCodex._convert_events_to_trajectory(1,029 and 741 lines), through a symlink-staging layer that rebuilt the directory shape those methods expect. The two upstream files took 26 and 18 commits between June and September 2026. That was the drift we were exposed to, and it was why the pin had a minor-version ceiling and a runtime probe with its own exit code.What changed
domain/claude_code_conversion.pyanddomain/codex_conversion.pyare parity ports (attributed), reached throughinfrastructure/claude_code_converter.pyandinfrastructure/codex_converter.py, which read the files directly. Staging, temp dirs, the private-API probe,HarborPrivateApiMissingand the exit 127 handlers are gone (the code stays in the table so it never renumbers).tests/harbor_oracle.py, as the oracle: asserted deterministic, frozen to goldens for both synthetic fixtures (the live oracle must equal the frozen one, so an upstream behavior change becomes a named JSON-path diff rather than a mystery), and diffed against our converters over the newest N local sessions withATIF_PARITY_LIMIT(0= all).astguard pins production imports to the two public modules; import-linter cannot express a sub-module of an external package. Proven to fail by dropping aharbor.agentsimport intosrc/.harbor>=0.22.0,<1;litellm>=1.92.0declared directly, since harbor's converters lazily import it for per-call cost and the port keeps that.workflow_subagents_missed(our converter discovers every side file itself) and Codex'ssingle_rollout_per_directory(one file in, one trajectory out). Values are not reused. The data-loss gaps stay on purpose: this is a parity port, and fixing one is a decision to diverge from the oracle, recorded in the fidelity policy.Measured
ATIF_PARITY_LIMIT=0ATIF_PARITY_LIMIT=0mise run checkmise run docs:gatePreserved harbor behavior worth a later decision (not changed here)
agent.extracwds/git_branches/agent_idsare built from Python sets, so list order for multi-element values is hash-seed dependent across processes. Pre-existing in every corpus on disk; a follow-up can sort them once we decide to diverge.extra.metadataandextra.tool_result_metadata).web_search_callyields an emptytool_call_idand an empty observation result.Docs updated: CONTRACT, CONTRIBUTING (the pin section is now the oracle section), AGENTS.md, README, RELEASING, the PR template, and every generated-docs passage that named the private method.
After a pre-merge review (second commit)
The review found one real defect: litellm loads its pricing table from GitHub at import time, and the frozen goldens carry per-call
cost_usdpriced through it, so a remote price edit or an unreachable GitHub in CI would have failed the never-skipped golden gate with no change here. The converter conftest now pinsLITELLM_LOCAL_MODEL_COST_MAP=true(the table bundled with the locked litellm), the Codex fixture model moved to one that table prices so the cost path stays frozen, and the goldens were re-frozen. Measured: the bundled table had no entry for the previous fixture model and the golden tests failed under it.Also from the review: explicit utf-8 on the Codex reader to match its sibling, comments recording the set-ordered agent extras as harbor behavior kept on purpose, four doc sentences that still advertised exit 127 or the retired gap as live, and a test that an unreadable side file becomes a
ConversionErrorcarrying theOSError.Follow-ups this PR does not take
convertprocess still pays litellm's import (about 2 s measured) and, unlessLITELLM_LOCAL_MODEL_COST_MAPis set, a GitHub fetch for the pricing table. That is harbor's cost path carried over. Pinning the bundled table in production too would makeconvertoffline-safe; it is a pricing-freshness decision, so it is left for its own change.agent.extracwds/git_branches/agent_idswould make multi-valued sessions reproducible across processes, at the price of a deliberate divergence from the oracle.