Skip to content

feat(parser): migrate kimi provider#764

Draft
mariusvniekerk wants to merge 1 commit into
provider-cortexfrom
provider-kimi
Draft

feat(parser): migrate kimi provider#764
mariusvniekerk wants to merge 1 commit into
provider-cortexfrom
provider-kimi

Conversation

@mariusvniekerk

Copy link
Copy Markdown
Collaborator

Kimi now has a concrete provider facade for its two wire.jsonl source layouts: legacy project/session directories and .kimi-code workdir/session/agents directories.

The provider composes the shared JSONL source helper for discovery, watching, changed-path classification, and fingerprinting while preserving Kimi-specific source validation and colon-delimited raw-ID lookup. It keeps symlinked directory support, project hints, invalid component filtering, and existing parse normalization.

@roborev-ci

roborev-ci Bot commented Jun 19, 2026

Copy link
Copy Markdown

roborev: Combined Review (62c2392)

No issues found.


Panel: ci_default_security | Synthesis: codex | Members: codex_default (codex/default, done, 4m5s), codex_security (codex/security, done, 1m18s) | Total: 5m23s

@roborev-ci

roborev-ci Bot commented Jun 20, 2026

Copy link
Copy Markdown

roborev: Review Unavailable (43057f4)

The review agent repeatedly failed to run (likely an agent or configuration error). roborev will try again on the next commit.

Last error: agent: claude-code failed stream: stream errors: You've hit your session limit · resets 5:50am (UTC): exit status 1

@roborev-ci

roborev-ci Bot commented Jun 21, 2026

Copy link
Copy Markdown

roborev: Combined Review (ade19bc)

Medium finding only: Kimi shadow comparison will mismatch every parsed source until provider fingerprints include file hashes.

Medium

  • internal/parser/kimi_provider.go:139
    Kimi is opted into ProviderMigrationShadowCompare, but its JSONLSourceSetOptions does not enable content hashing. Legacy processKimi sets sess.File.Hash from ComputeFileHash, while ObserveProviderSource uses the provider fingerprint with an empty Hash, so provider-parsed sessions leave Session.File.Hash empty and shadow comparison reports mismatches for every successfully parsed Kimi source.
    Fix: Add Hash: true to Kimi source set options and update the Kimi shadow test to assert a non-empty fingerprint hash instead of masking it.

Panel: ci_default_security | Synthesis: codex, 7s | Members: codex_default (codex/default, done, 4m50s), codex_security (codex/security, done, 1m21s) | Total: 6m18s

@mariusvniekerk

mariusvniekerk commented Jun 21, 2026

Copy link
Copy Markdown
Collaborator Author

This change is part of the following stack:

Change managed by git-spice.

@roborev-ci

roborev-ci Bot commented Jun 21, 2026

Copy link
Copy Markdown

roborev: Combined Review (1f6255a)

The PR has one medium issue: Kimi provider shadow comparison will report false mismatches due to missing file hash parity.

Medium

  • internal/parser/kimi_provider.go:139: The Kimi provider does not enable content hashing in JSONLSourceSetOptions, leaving Fingerprint.Hash empty and sess.File.Hash unset. The legacy processKimi path computes and stores the SHA-256 hash, so enabling ProviderMigrationShadowCompare will report every parsed Kimi session as a shadow mismatch on the session struct.
    • Fix: Set Hash: true for the Kimi source set or otherwise compute the same hash before returning parse results, and cover the actual shadow comparison path in the test.

Panel: ci_default_security | Synthesis: codex, 6s | Members: codex_default (codex/default, done, 3m53s), codex_security (codex/security, done, 2m6s) | Total: 6m5s

@roborev-ci

roborev-ci Bot commented Jun 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (8a2d016)

Migration is not ready: one medium-severity compatibility regression remains.

Medium

  • internal/parser/types.go:392: Removing Kimi’s DiscoverFunc/FindSourceFunc drops it from call sites that still key off those legacy hooks. parse-diff --agent kimi will now be rejected or skipped by default parse-diff, token-use can no longer resolve unsynced Kimi IDs from disk, and the SSH resolver omits Kimi transfer roots.
    • Fix: Keep compatibility discovery/find adapters until those call sites are provider-aware, or update parse-diff, token-use resolution, and SSH remote resolution to use ProviderFactories/provider discovery for migrated providers.

Panel: ci_default_security | Synthesis: codex, 7s | Members: codex_default (codex/default, done, 5m42s), codex_security (codex/security, done, 2m7s) | Total: 7m56s

Kimi uses two wire.jsonl layouts whose raw IDs include colon-delimited path components, so it cannot rely entirely on the generic JSONL raw-ID lookup. Moving it behind a concrete provider keeps discovery and source classification on the shared JSONL helper while preserving Kimi-specific layout validation and lookup semantics.\n\nThe provider keeps legacy support for both the .kimi project/session layout and the .kimi-code workdir/session/agents layout, including symlinked directories, invalid component filtering, project hints, deleted-path classification, and parser output normalization.

test(parser): cover kimi new-layout provider parse

The roborev design review questioned whether the provider-backed Kimi migration proved the newer .kimi-code layout could round-trip through lookup and parsing. The existing parser and lookup code already handled that raw ID shape, but the provider tests only parsed the legacy layout.\n\nThis adds provider-level coverage for the .kimi-code workdir/session/agents layout so the branch itself documents the persisted session ID, project hint, source path, machine, hash propagation, and message output expected from that source shape.

test(parser): opt kimi into provider shadow

Kimi now has a concrete facade provider on this branch, so its migration mode should enter shadow comparison instead of remaining legacy-only and additive.

Lower provider opt-ins stay inherited and later branches own their provider modes.

Validation: go test -tags "fts5" ./internal/parser -run TestProviderMigrationModes -count=1; go test -tags "fts5" ./internal/parser -count=1; go vet ./...; git diff --check

test(sync): compare kimi shadow parity

Kimi is shadow-compared on this branch, so add source-level migration coverage that compares provider observation with ParseKimiSession.

The test covers both the legacy project/session wire.jsonl layout and the newer .kimi-code agents layout, keeping the fragile path-derived ID and project behavior visible during review.

Validation: go test -tags "fts5" ./internal/parser ./internal/sync -run 'TestObserveProviderSourceMatchesKimiLegacyParser|TestKimiProvider|TestParseKimi|TestSyncPathsAndSingleSession_KimiNewLayout|TestClassifyOnePath_Kimi' -count=1; go test -tags "fts5" ./internal/parser ./internal/sync -count=1; go fmt ./...; go vet ./...; git diff --check; ./custom-gcl run --config .golangci.nilaway.yml ./internal/parser/... ./internal/sync/...; make nilaway

refactor(parser): fold kimi into provider

Move Kimi parse and raw-ID source lookup onto the concrete provider and remove package-level discover/find/parse entrypoints. Route Kimi sync classification and processing through provider changed-path handling so the branch migrates the provider instead of preserving legacy dispatch.
@roborev-ci

roborev-ci Bot commented Jun 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (eb18a0a)

Summary verdict: one medium issue needs fixing before merge.

Medium

  • internal/parser/kimi_provider.go:171 - The migrated Kimi provider no longer computes a content hash in its fingerprint. The removed legacy processKimi path populated sess.File.Hash with ComputeFileHash, but JSONLSourceSetOptions omits Hash: true, so provider syncs pass an empty Fingerprint.Hash and can clear existing file_hash values for Kimi sessions.
    • Fix: Set Hash: true in newKimiSourceSet and add coverage asserting provider.Fingerprint(...).Hash is populated and propagated into the parsed session.

Panel: ci_default_security | Synthesis: codex, 7s | Members: codex_default (codex/default, done, 5m2s), codex_security (codex/security, done, 1m33s) | Total: 6m42s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant