Clean up versioned runtime names#654
Conversation
📝 WalkthroughWalkthroughThe PR renames versioned runtime symbols to unversioned names across reducer, state, comparison/transfer, trust, and memory code, updates matching tests/docs, and moves v17 graph-model fixture artifacts into the v18 migration scripts tree. ChangesRuntime naming, types, and migration assets
Sequence Diagram(s)
Estimated code review effort Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Release Preflight
If you tag this commit as |
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/design/joinreducer-op-strategy.md (1)
48-48:⚠️ Potential issue | 🟡 MinorUpdate JSDoc
WarpStateV5references toWarpStateindocs/design/joinreducer-op-strategy.md
joinreducer-op-strategy.mdstill usesWarpStateV5in theOpStrategytypedef (lines 48-54) and in the documented signatures/descriptions (applyFast(...) => WarpStateV5, “all five WarpStateV5 fields”). The runtime code usesWarpState(e.g.,applyFast(...): WarpState) and there’s noWarpStateV5type undersrc/. Update the doc to consistently referenceWarpState(including the referenced lines around 48-54, 153, and 166).🤖 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 `@docs/design/joinreducer-op-strategy.md` at line 48, Update the documentation typedefs and signatures to use the current WarpState name instead of the obsolete WarpStateV5: replace all occurrences of "WarpStateV5" in the OpStrategy typedef (the mutate function signature and related lines), the documented applyFast(...) signature, and any textual references like "all five WarpStateV5 fields" with "WarpState" and corresponding phrasing; ensure references to functions/types such as OpStrategy, applyFast, and mutate consistently mention WarpState throughout the file.docs/design/joinreducer-split.md (1)
56-62:⚠️ Potential issue | 🟡 MinorAlign state-factory function names in joinreducer-split design doc with JoinReducer exports
src/domain/services/JoinReducer.tsexportscreateEmptyState(),cloneState(...), andjoinStates(...)(deprecated wrappers); there are nocreateEmptyStateV5/cloneStateV5exports insrc/.- Update
docs/design/joinreducer-split.md(notably lines 56-62, plus theWarpStateFactoryimport/export around lines ~160 and ~188) to use the unversioned names to match the code.🤖 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 `@docs/design/joinreducer-split.md` around lines 56 - 62, The doc uses V5-suffixed factory names that don't match the actual exports; update docs to reference the real unversioned exports: replace createEmptyStateV5 -> createEmptyState, cloneStateV5 -> cloneState, and ensure joinStates is referenced as exported joinStates; also keep the helper names (mergeProps, mergeEdgeBirthEvent, foldPatchDot, updateFrontierFromPatch) consistent, and update the WarpStateFactory import/export references around the noted sections to import/export the unversioned symbols (WarpStateFactory using createEmptyState, cloneState, joinStates) so the design doc matches src/domain/services/JoinReducer.ts.
🧹 Nitpick comments (6)
src/domain/types/VisibleStateReader.ts (1)
11-11: ⚡ Quick winUpdate comment to remove "V5" reference.
The comment describes this as accessing "V5 state," but the PR objective is to remove versioned nouns from the runtime surface. Since the type references are now unversioned, update the comment to simply "materialized state."
📝 Suggested fix
- * Read-only accessor over materialized V5 state with entity-local inspection. + * Read-only accessor over materialized state with entity-local inspection.🤖 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 `@src/domain/types/VisibleStateReader.ts` at line 11, Update the JSDoc/top-line comment on VisibleStateReader to remove the versioned term "V5" and refer to it as "materialized state" instead (e.g., change "materialized V5 state" to "materialized state"); locate the comment above the VisibleStateReader type declaration and edit only the wording to remove versioned nouns while preserving the rest of the description about read-only accessor and entity-local inspection.src/domain/memory/createBoundedMemoryCapabilityReport.ts (1)
15-16: ⚡ Quick winUpdate comment to remove version reference.
The comment still mentions "v18" which is inconsistent with the PR objective of removing versioned runtime names from active code.
♻️ Suggested wording
-/** Reports the current v18 bounded-memory truth without claiming release completion. */ +/** Reports the current bounded-memory capabilities without claiming release completion. */ export default function createBoundedMemoryCapabilityReport(): MemoryCapabilityReport {🤖 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 `@src/domain/memory/createBoundedMemoryCapabilityReport.ts` around lines 15 - 16, Update the top-of-function comment for createBoundedMemoryCapabilityReport to remove the hard-coded "v18" version reference and rephrase it to a neutral description (e.g., "Reports the current bounded-memory truth without claiming release completion") so it no longer contains runtime versioning; edit the comment immediately above the export default function createBoundedMemoryCapabilityReport() to reflect the new wording.test/unit/domain/services/controllers/ProvenanceController.test.ts (1)
62-62: ⚡ Quick winRename test mock variable to match unversioned function name.
The variable
mockReduceV5still contains the version suffix even though it mocks the renamedreducePatchesfunction. For full consistency with the PR's objective to remove version-named symbols, rename this variable tomockReducePatchesand update its usages on lines 231 and 238.♻️ Proposed refactor
-const mockReduceV5 = (reducePatches); +const mockReducePatches = (reducePatches);Also update usages on lines 231 and 238:
- (mockReduceV5 as any).mockReturnValue({ state: fakeState, receipts: fakeReceipts }); + (mockReducePatches as any).mockReturnValue({ state: fakeState, receipts: fakeReceipts });- expect(mockReduceV5).toHaveBeenCalledWith( + expect(mockReducePatches).toHaveBeenCalledWith(🤖 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 `@test/unit/domain/services/controllers/ProvenanceController.test.ts` at line 62, Rename the test mock variable mockReduceV5 to mockReducePatches to match the unversioned function name reducePatches; update all references to mockReduceV5 (the mock declaration and its usages in the ProvenanceController.test test cases) to mockReducePatches so the mock name and the function under test are consistent.test/unit/domain/services/JoinReducer.integration.test.ts (1)
1-11: 💤 Low valueConsider updating test file header to remove version references.
The header comments still reference "WARP v5" (lines 2, 4). For consistency with the PR objective to remove versioned names, consider updating to be version-agnostic (e.g., "WARP Integration Tests - KILLER TESTS").
🤖 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 `@test/unit/domain/services/JoinReducer.integration.test.ts` around lines 1 - 11, Update the file header comment to remove the versioned "v5" references: change occurrences of "WARP v5 Integration Tests" and "WARP v5 upgrade" to a version-agnostic phrase such as "WARP Integration Tests" and "WARP upgrade" (and update the `@module` tag "JoinReducer.integration.test" if it embeds a versioned name) so the header no longer mentions "v5"; ensure all header lines in the top comment block are edited consistently.src/domain/services/state/StateSerializer.ts (1)
11-21: 💤 Low valueConsider updating the module documentation to remove version references.
The header comment still references "WARP v5" in multiple places (lines 11, 16). Since the PR objective is to remove versioned names from active code, consider updating this to be version-agnostic (e.g., "State Serialization and Hashing for WARP").
🤖 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 `@src/domain/services/state/StateSerializer.ts` around lines 11 - 21, Update the file header in StateSerializer.ts to remove versioned names: replace occurrences of "WARP v5" and "V5 uses" with version-agnostic wording (e.g., "WARP" and "Uses ORSet-based state (rather than LWW registers...)"), and ensure the `@module` and any inline references in the top comment are non-versioned so the module documentation is neutral; keep the same explanatory content and spec references.src/domain/services/controllers/ComparisonSelector.ts (1)
1-545: ⚡ Quick winFile exceeds 500 LOC limit.
This source file contains 545 lines, exceeding the coding guideline limit of 500 LOC for source files. Consider splitting selector classes into separate files (e.g.,
LiveComparisonSelector.ts,CoordinateComparisonSelector.ts,StrandComparisonSelector.ts,StrandBaseComparisonSelector.ts) or extract the helper utilities into a focused module.As per coding guidelines:
**/*.{ts,tsx,js,mjs,cjs}: Limit source files to 500 lines of code (LOC).🤖 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 `@src/domain/services/controllers/ComparisonSelector.ts` around lines 1 - 545, The file exceeds the 500 LOC guideline; split the selector implementations and heavy helpers into smaller modules: extract each selector class (LiveComparisonSelector, CoordinateComparisonSelector, StrandComparisonSelector, StrandBaseComparisonSelector) into its own file exporting the class and import them back into ComparisonSelector.ts, move shared helpers (normalizeLamportCeiling, normalizeFrontierRecord, frontierRecordToMap, collectPatchEntriesForFrontier, patch/frontier utilities, buildStrandMetadata, finalizeSide-related helpers like summarizeVisibleState and finalizeSide if needed) into a new utilities module that ComparisonSelector.ts and the new selector files import, and keep only the selector normalization (normalizeSelector) and light wiring in the original file by updating imports/exports and preserving types like ComparisonRequestedSide/ResolvedComparisonSide; ensure function/class names (e.g., normalizeSelector, ResolvedComparisonSide, finalizeSide, computeStateHashForGraph) remain accessible where referenced.Source: Coding guidelines
🤖 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 `@src/domain/trust/schemas.ts`:
- Line 126: The CLI error fallback in bin/cli/commands/verify-audit.ts emits
mode: 'signed_evidence_v1' which no longer matches the TrustAssessmentSchema
discriminator (mode: 'signed_evidence'); update the catch/fallback payload in
the verify-audit command to emit mode: 'signed_evidence' instead of
'signed_evidence_v1' so the object validated against TrustAssessmentSchema (and
any downstream consumers expecting the 'signed_evidence' discriminator) will
pass validation.
In `@test/unit/domain/services/JoinReducer.edgeProps.test.ts`:
- Line 14: The test currently force-casts _reducePatches to an unsafe signature
using "as (...args: any[]) => any"; remove that cast and make the test call
sites match reducePatches' real type or update the reducePatches API to accept
the test usage. Concretely, stop using the cast on reducePatches/_reducePatches,
import the actual typed reducePatches, and update the test arguments (or the
function's exported signature) so TypeScript types line up without using any or
as any.
In `@test/unit/domain/services/JoinReducer.test.ts`:
- Line 19: The test uses an unsafe any assertion for reducePatches; replace
"(_reducePatches) as (...args: any[]) => any" by using the real function type
from the implementation (import the type/signature from JoinReducer.ts) or
assert using typeof _reducePatches (e.g., const reducePatches = _reducePatches
as typeof _reducePatches) so the parameters and return types match the actual
reducePatches signature; if a specific signature is required for the test, copy
the reducePatches function type (parameters and return type) from JoinReducer.ts
into the assertion instead of using any.
In `@test/unit/domain/services/JoinReducer.validation.test.ts`:
- Line 19: The test currently casts _reducePatches using an any type ("const
reducePatches = (_reducePatches) as (...args: any[]) => any;"); replace that
any-cast by using the actual function type imported from the production module
(or use typeof on the real exported function) so the test uses the concrete type
signature instead of any — locate the test symbol _reducePatches and change the
cast to the proper imported type (or typeof reducePatchesExport) for
reducePatches.
In `@test/unit/domain/services/MaterializedView.equivalence.test.ts`:
- Line 400: Remove the unsafe "as any" cast on fullState; instead give fullState
a concrete typed declaration or make reducePatches return the correct type.
Locate the variable fullState and the call reducePatches(patches) and either
annotate fullState with the correct interface/type (e.g., const fullState:
ExpectedStateType = reducePatches(patches)) or update the reducePatches
signature/generics so it returns ExpectedStateType directly; ensure patches is
typed compatibly so no "as any" is necessary.
- Line 274: Remove the `as any` cast on the reducePatches result: update the
code so `fullState` has an explicit, correct type instead of using `as any` (for
example declare `const fullState: MaterializedViewState =
reducePatches(patches)`), or change `reducePatches`'s signature to return the
proper typed state; locate the symbol `reducePatches` and the variable
`fullState` in the test and replace the cast with an explicit, concrete type
that matches the expected state produced from `patches`.
- Line 230: The test currently silences TypeScript by casting the result of
reducePatches(patches) to any when assigning to fullState; remove the `as any`
cast and instead ensure proper typing: update reducePatches() return type to the
correct shape or add an explicit type annotation for fullState that matches the
expected state interface used in this test (e.g., declare fullState:
ExpectedState = reducePatches(patches)). Locate the assignment to fullState and
replace the cast with a typed declaration or fix reducePatches' signature so the
cast is unnecessary.
In `@test/unit/domain/services/ProvenancePayload.test.ts`:
- Around line 4-5: The tests import reducePatches as _reducePatches and then
cast it to (...args: any[]) => any which introduces prohibited any usage; remove
that cast and simply use the imported function directly (replace the const
reducePatches = (_reducePatches) as (...args: any[]) => any pattern with
directly using reducePatches/_reducePatches), and do the same cleanup in the
other test files exhibiting this pattern so reducePatches (and related imports
like encodeEdgeKey, encodePropKey) retain their original TypeScript types.
In `@test/unit/domain/services/SyncProtocol.test.ts`:
- Line 15: The alias declaration for reducePatches currently forces an explicit
any type (const reducePatches: (...args: any[]) => any = _reducePatches;) —
remove the explicit any annotation and let TypeScript infer the type from the
imported _reducePatches (i.e., change the alias to simply const reducePatches =
_reducePatches) so the test obeys the no-any guideline while preserving the
original binding.
---
Outside diff comments:
In `@docs/design/joinreducer-op-strategy.md`:
- Line 48: Update the documentation typedefs and signatures to use the current
WarpState name instead of the obsolete WarpStateV5: replace all occurrences of
"WarpStateV5" in the OpStrategy typedef (the mutate function signature and
related lines), the documented applyFast(...) signature, and any textual
references like "all five WarpStateV5 fields" with "WarpState" and corresponding
phrasing; ensure references to functions/types such as OpStrategy, applyFast,
and mutate consistently mention WarpState throughout the file.
In `@docs/design/joinreducer-split.md`:
- Around line 56-62: The doc uses V5-suffixed factory names that don't match the
actual exports; update docs to reference the real unversioned exports: replace
createEmptyStateV5 -> createEmptyState, cloneStateV5 -> cloneState, and ensure
joinStates is referenced as exported joinStates; also keep the helper names
(mergeProps, mergeEdgeBirthEvent, foldPatchDot, updateFrontierFromPatch)
consistent, and update the WarpStateFactory import/export references around the
noted sections to import/export the unversioned symbols (WarpStateFactory using
createEmptyState, cloneState, joinStates) so the design doc matches
src/domain/services/JoinReducer.ts.
---
Nitpick comments:
In `@src/domain/memory/createBoundedMemoryCapabilityReport.ts`:
- Around line 15-16: Update the top-of-function comment for
createBoundedMemoryCapabilityReport to remove the hard-coded "v18" version
reference and rephrase it to a neutral description (e.g., "Reports the current
bounded-memory truth without claiming release completion") so it no longer
contains runtime versioning; edit the comment immediately above the export
default function createBoundedMemoryCapabilityReport() to reflect the new
wording.
In `@src/domain/services/controllers/ComparisonSelector.ts`:
- Around line 1-545: The file exceeds the 500 LOC guideline; split the selector
implementations and heavy helpers into smaller modules: extract each selector
class (LiveComparisonSelector, CoordinateComparisonSelector,
StrandComparisonSelector, StrandBaseComparisonSelector) into its own file
exporting the class and import them back into ComparisonSelector.ts, move shared
helpers (normalizeLamportCeiling, normalizeFrontierRecord, frontierRecordToMap,
collectPatchEntriesForFrontier, patch/frontier utilities, buildStrandMetadata,
finalizeSide-related helpers like summarizeVisibleState and finalizeSide if
needed) into a new utilities module that ComparisonSelector.ts and the new
selector files import, and keep only the selector normalization
(normalizeSelector) and light wiring in the original file by updating
imports/exports and preserving types like
ComparisonRequestedSide/ResolvedComparisonSide; ensure function/class names
(e.g., normalizeSelector, ResolvedComparisonSide, finalizeSide,
computeStateHashForGraph) remain accessible where referenced.
In `@src/domain/services/state/StateSerializer.ts`:
- Around line 11-21: Update the file header in StateSerializer.ts to remove
versioned names: replace occurrences of "WARP v5" and "V5 uses" with
version-agnostic wording (e.g., "WARP" and "Uses ORSet-based state (rather than
LWW registers...)"), and ensure the `@module` and any inline references in the top
comment are non-versioned so the module documentation is neutral; keep the same
explanatory content and spec references.
In `@src/domain/types/VisibleStateReader.ts`:
- Line 11: Update the JSDoc/top-line comment on VisibleStateReader to remove the
versioned term "V5" and refer to it as "materialized state" instead (e.g.,
change "materialized V5 state" to "materialized state"); locate the comment
above the VisibleStateReader type declaration and edit only the wording to
remove versioned nouns while preserving the rest of the description about
read-only accessor and entity-local inspection.
In `@test/unit/domain/services/controllers/ProvenanceController.test.ts`:
- Line 62: Rename the test mock variable mockReduceV5 to mockReducePatches to
match the unversioned function name reducePatches; update all references to
mockReduceV5 (the mock declaration and its usages in the
ProvenanceController.test test cases) to mockReducePatches so the mock name and
the function under test are consistent.
In `@test/unit/domain/services/JoinReducer.integration.test.ts`:
- Around line 1-11: Update the file header comment to remove the versioned "v5"
references: change occurrences of "WARP v5 Integration Tests" and "WARP v5
upgrade" to a version-agnostic phrase such as "WARP Integration Tests" and "WARP
upgrade" (and update the `@module` tag "JoinReducer.integration.test" if it embeds
a versioned name) so the header no longer mentions "v5"; ensure all header lines
in the top comment block are edited consistently.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ebc84a68-fca6-42d4-8aaa-ca561ee6abf9
📒 Files selected for processing (139)
CHANGELOG.mdbin/cli/commands/doctor/index.tsdocs/ADVANCED_GUIDE.mddocs/audits/2026-04-production-readiness.mddocs/design/0002-code-nav-tool/code-nav-tool.mddocs/design/0009-op-type-class-hierarchy/op-type-class-hierarchy.mddocs/design/0041-joinreducer-state-session.mddocs/design/0043-materialize-integration.mddocs/design/0099-btr-provenance-codec-boundary-repair.mddocs/design/0148-v18-warp-optic-realization-map/v18-warp-optic-realization-map.mddocs/design/MIGRATION_PROBLEM.mddocs/design/joinreducer-op-strategy.mddocs/design/joinreducer-split.mddocs/method/retro/0041-joinreducer-state-session/joinreducer-state-session.mddocs/specs/TRUST_CRYPTO_ALGORITHM.mddocs/trust/TRUST_OPERATOR_RUNBOOK.mdscripts/v18.0.0/migrations/graph-model/GitWarpGraphModelContractConformance.tsscripts/v18.0.0/migrations/graph-model/GitWarpWarpTtdGeneratedFamilySmoke.tsscripts/v18.0.0/migrations/graph-model/GraphModelMigrationCommandCli.tsscripts/v18.0.0/migrations/graph-model/GraphModelMigrationSourceInventoryCollector.tsscripts/v18.0.0/migrations/graph-model/V17GoldenFixtureScratchReadingProvider.tsscripts/v18.0.0/migrations/graph-model/V17GoldenGraphFixtureGenesisReading.tsscripts/v18.0.0/migrations/graph-model/V17GoldenGraphFixtureManifest.tsscripts/v18.0.0/migrations/graph-model/V17GoldenGraphFixtureManifestJsonAdapter.tsscripts/v18.0.0/migrations/graph-model/V17GoldenGraphFixturePropertyMappings.tsscripts/v18.0.0/migrations/graph-model/V17GoldenGraphFixtureRestore.tsscripts/v18.0.0/migrations/graph-model/V17GoldenGraphFixtureWetRunHarness.tsscripts/v18.0.0/migrations/graph-model/V17RestoredPublicReadLegacyReadingBuilder.tssrc/domain/WarpWorldline.tssrc/domain/capabilities/ComparisonCapability.tssrc/domain/memory/createBoundedMemoryCapabilityReport.tssrc/domain/memory/index.tssrc/domain/migrations/GraphModelMigrationFinalizationConfirmation.tssrc/domain/services/CoordinateFactExport.tssrc/domain/services/JoinReducer.tssrc/domain/services/JoinReducerSession.tssrc/domain/services/OpNormalizer.tssrc/domain/services/PatchBuilder.tssrc/domain/services/PatchCommitter.tssrc/domain/services/PatchHydrator.tssrc/domain/services/VisibleStateScope.tssrc/domain/services/codec/MessageSchemaDetector.tssrc/domain/services/codec/WarpMessageCodec.tssrc/domain/services/controllers/ComparisonController.tssrc/domain/services/controllers/ComparisonCoordinateSideReadPort.tssrc/domain/services/controllers/ComparisonEngine.tssrc/domain/services/controllers/ComparisonSelector.tssrc/domain/services/controllers/MaterializeController.tssrc/domain/services/controllers/MaterializeSessionBridge.tssrc/domain/services/controllers/ProvenanceController.tssrc/domain/services/provenance/ProvenancePayload.tssrc/domain/services/state/StateSerializer.tssrc/domain/services/state/checkpointLoad.tssrc/domain/services/strand/CanonicalConflictOp.tssrc/domain/services/strand/ConflictFrameLoader.tssrc/domain/services/strand/ConflictOpAnchor.tssrc/domain/services/strand/StrandMaterializer.tssrc/domain/services/sync/syncPatchLoader.tssrc/domain/services/sync/syncRequestResponse.tssrc/domain/services/transfer/VisibleStateTransferPlanner.tssrc/domain/services/transfer/transferOps.tssrc/domain/services/transfer/transferShape.tssrc/domain/trust/TrustAssessment.tssrc/domain/trust/TrustCanonical.tssrc/domain/trust/TrustEvaluator.tssrc/domain/trust/TrustRecord.tssrc/domain/trust/TrustRecordService.tssrc/domain/trust/TrustStateBuilder.tssrc/domain/trust/canonical.tssrc/domain/trust/reasonCodes.tssrc/domain/trust/schemas.tssrc/domain/trust/verdict.tssrc/domain/types/CoordinateComparison.tssrc/domain/types/Patch.tssrc/domain/types/VisibleEdgeView.tssrc/domain/types/VisibleStateProjection.tssrc/domain/types/VisibleStateReader.tssrc/domain/types/ops/unions.tssrc/domain/warp/WarpCoreRuntimeProduct.tssrc/infrastructure/adapters/BtrCodecAdapter.tssrc/infrastructure/adapters/TrailerCommitMessageCodecAdapter.tssrc/infrastructure/adapters/TrustCryptoAdapter.tssrc/ports/CommitMessageCodecPort.tstest/benchmark/ReducerV5.benchmark.tstest/benchmark/logicalIndex.benchmark.tstest/helpers/WarpGraphPatchFixtures.tstest/helpers/fixtureDsl.tstest/helpers/stateBuilder.tstest/helpers/topologyHelpers.tstest/integration/WarpGraph.integration.test.tstest/unit/domain/PatchOperationAssertions.tstest/unit/domain/WarpGraph.strands.compare.test.tstest/unit/domain/continuum/GitWarpGraphModelContractConformance.test.tstest/unit/domain/continuum/GitWarpWarpTtdGeneratedFamilySmoke.test.tstest/unit/domain/index.exports.test.tstest/unit/domain/migrations/GraphModelMigrationConstructorGuards.test.tstest/unit/domain/migrations/GraphModelMigrationReviewGuardCoverage.test.tstest/unit/domain/migrations/V17GoldenGraphFixtureGenesisReading.test.tstest/unit/domain/properties/Join.property.test.tstest/unit/domain/services/AuditVerifierService.test.tstest/unit/domain/services/BoundaryTransitionRecord.test.tstest/unit/domain/services/IncrementalIndexUpdater.test.tstest/unit/domain/services/JoinReducer.edgeProps.test.tstest/unit/domain/services/JoinReducer.integration.test.tstest/unit/domain/services/JoinReducer.opSets.test.tstest/unit/domain/services/JoinReducer.pathEquivalence.test.tstest/unit/domain/services/JoinReducer.receipts.test.tstest/unit/domain/services/JoinReducer.stateSession.test.tstest/unit/domain/services/JoinReducer.test.tstest/unit/domain/services/JoinReducer.trackDiff.test.tstest/unit/domain/services/JoinReducer.validation.test.tstest/unit/domain/services/LogicalIndexBuildService.determinism.test.tstest/unit/domain/services/LogicalIndexBuildService.test.tstest/unit/domain/services/LogicalIndexReader.test.tstest/unit/domain/services/MaterializedView.equivalence.test.tstest/unit/domain/services/MaterializedViewService.test.tstest/unit/domain/services/MaterializedViewService.verify.test.tstest/unit/domain/services/ProvenancePayload.test.tstest/unit/domain/services/SchemaCompat.test.tstest/unit/domain/services/StateDiff.test.tstest/unit/domain/services/StateSerializer.test.tstest/unit/domain/services/SyncProtocol.test.tstest/unit/domain/services/TrustPayloadParity.test.tstest/unit/domain/services/VisibleStateScope.test.tstest/unit/domain/services/WarpMessageCodec.v3.test.tstest/unit/domain/services/WormholeService.test.tstest/unit/domain/services/controllers/ComparisonController.test.tstest/unit/domain/services/controllers/ProvenanceController.test.tstest/unit/domain/services/noBufferGlobal.test.tstest/unit/domain/services/strand/ConflictAnalyzerService.test.tstest/unit/domain/trust/TrustAssessment.snapshot.test.tstest/unit/domain/trust/TrustCrypto.signVerify.test.tstest/unit/domain/trust/TrustEvaluator.test.tstest/unit/domain/types/ops/reducer-integration.test.tstest/unit/domain/warp/hydrateCheckpointIndex.regression.test.tstest/unit/scripts/public-api-advanced-guide-shape.test.tstest/unit/scripts/v18-v17-fixture-wet-run-harness.test.tstest/unit/scripts/v18-v17-golden-graph-fixtures.test.tstest/unit/scripts/visible-state-upgrade.test.ts
| } from '../../../../src/domain/services/JoinReducer.ts'; | ||
| import { decodeEdgePropKey } from '../../../../src/domain/services/KeyCodec.ts'; | ||
| const reduceV5 = (_reduceV5) as (...args: any[]) => any; | ||
| const reducePatches = (_reducePatches) as (...args: any[]) => any; |
There was a problem hiding this comment.
Remove as any casting.
Line 14 violates the TypeScript coding guidelines:
- "Do not use
anytype in TypeScript code anywhere, including in adapters" - "Do not use
as anycasting anywhere in TypeScript code, including in adapters"
The cast as (...args: any[]) => any defeats type safety and hides mismatches between the imported reducePatches signature and how it's called in tests. Either update the test calls to match the proper signature, or adjust the reducePatches API contract if the usage is correct.
🤖 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 `@test/unit/domain/services/JoinReducer.edgeProps.test.ts` at line 14, The test
currently force-casts _reducePatches to an unsafe signature using "as (...args:
any[]) => any"; remove that cast and make the test call sites match
reducePatches' real type or update the reducePatches API to accept the test
usage. Concretely, stop using the cast on reducePatches/_reducePatches, import
the actual typed reducePatches, and update the test arguments (or the function's
exported signature) so TypeScript types line up without using any or as any.
Source: Coding guidelines
| } from '../../../../src/domain/services/JoinReducer.ts'; | ||
| import { decodePropKey } from '../../../../src/domain/services/KeyCodec.ts'; | ||
| const reduceV5 = (_reduceV5) as (...args: any[]) => any; | ||
| const reducePatches = (_reducePatches) as (...args: any[]) => any; |
There was a problem hiding this comment.
Remove any type usage.
Line 19 uses any for both parameters and return type, which violates the coding guideline "Do not use any type in TypeScript code anywhere, including in adapters."
Consider importing the proper types from JoinReducer.ts or using typeof _reducePatches instead:
♻️ Suggested fix
-const reducePatches = (_reducePatches) as (...args: any[]) => any;
+const reducePatches = _reducePatches;If a type assertion is necessary due to test-specific constraints, use the actual function signature instead of any.
📝 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.
| const reducePatches = (_reducePatches) as (...args: any[]) => any; | |
| const reducePatches = _reducePatches; |
🤖 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 `@test/unit/domain/services/JoinReducer.test.ts` at line 19, The test uses an
unsafe any assertion for reducePatches; replace "(_reducePatches) as (...args:
any[]) => any" by using the real function type from the implementation (import
the type/signature from JoinReducer.ts) or assert using typeof _reducePatches
(e.g., const reducePatches = _reducePatches as typeof _reducePatches) so the
parameters and return types match the actual reducePatches signature; if a
specific signature is required for the test, copy the reducePatches function
type (parameters and return type) from JoinReducer.ts into the assertion instead
of using any.
Source: Coding guidelines
| import PatchError from '../../../../src/domain/errors/PatchError.ts'; | ||
|
|
||
| const reduceV5 = (_reduceV5) as (...args: any[]) => any; | ||
| const reducePatches = (_reducePatches) as (...args: any[]) => any; |
There was a problem hiding this comment.
Remove any type from cast.
The type cast uses any which violates the coding guideline "Do not use any type in TypeScript code anywhere, including in adapters." Use the function with its proper imported type instead of casting.
🔧 Suggested fix
import {
createEmptyState,
applyPatchOp,
- reducePatches as _reducePatches,
+ reducePatches,
} from '../../../../src/domain/services/JoinReducer.ts';
import { EventId } from '../../../../src/domain/utils/EventId.ts';
import { Dot } from '../../../../src/domain/crdt/Dot.ts';
import PatchError from '../../../../src/domain/errors/PatchError.ts';
-const reducePatches = (_reducePatches) as (...args: any[]) => any;
-As per coding guidelines: Do not use any type in TypeScript code anywhere, including in adapters.
📝 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.
| const reducePatches = (_reducePatches) as (...args: any[]) => any; | |
| import { | |
| createEmptyState, | |
| applyPatchOp, | |
| reducePatches, | |
| } from '../../../../src/domain/services/JoinReducer.ts'; | |
| import { EventId } from '../../../../src/domain/utils/EventId.ts'; | |
| import { Dot } from '../../../../src/domain/crdt/Dot.ts'; | |
| import PatchError from '../../../../src/domain/errors/PatchError.ts'; |
🤖 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 `@test/unit/domain/services/JoinReducer.validation.test.ts` at line 19, The
test currently casts _reducePatches using an any type ("const reducePatches =
(_reducePatches) as (...args: any[]) => any;"); replace that any-cast by using
the actual function type imported from the production module (or use typeof on
the real exported function) so the test uses the concrete type signature instead
of any — locate the test symbol _reducePatches and change the cast to the proper
imported type (or typeof reducePatchesExport) for reducePatches.
Source: Coding guidelines
|
|
||
| // ── Full rebuild ────────────────────────────────────────────── | ||
| const fullState = (reduceV5(patches) as any); | ||
| const fullState = (reducePatches(patches) as any); |
There was a problem hiding this comment.
Remove as any cast.
The coding guidelines prohibit as any casting anywhere in TypeScript code. The return type of reducePatches() should be properly typed instead of casting to any.
Recommended fix to remove the cast
If reducePatches() returns the correct type, the cast should be unnecessary:
- const fullState = (reducePatches(patches) as any);
+ const fullState = reducePatches(patches);If there's a type mismatch, consider fixing the return type of reducePatches() or declaring fullState with an explicit type annotation instead of using as any.
📝 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.
| const fullState = (reducePatches(patches) as any); | |
| const fullState = reducePatches(patches); |
🤖 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 `@test/unit/domain/services/MaterializedView.equivalence.test.ts` at line 230,
The test currently silences TypeScript by casting the result of
reducePatches(patches) to any when assigning to fullState; remove the `as any`
cast and instead ensure proper typing: update reducePatches() return type to the
correct shape or add an explicit type annotation for fullState that matches the
expected state interface used in this test (e.g., declare fullState:
ExpectedState = reducePatches(patches)). Locate the assignment to fullState and
replace the cast with a typed declaration or fix reducePatches' signature so the
cast is unnecessary.
Source: Coding guidelines
|
|
||
| // ── Full rebuild ────────────────────────────────────────────── | ||
| const fullState = (reduceV5(patches) as any); | ||
| const fullState = (reducePatches(patches) as any); |
There was a problem hiding this comment.
Remove as any cast.
The coding guidelines prohibit as any casting anywhere in TypeScript code.
Recommended fix to remove the cast
- const fullState = (reducePatches(patches) as any);
+ const fullState = reducePatches(patches);📝 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.
| const fullState = (reducePatches(patches) as any); | |
| const fullState = reducePatches(patches); |
🤖 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 `@test/unit/domain/services/MaterializedView.equivalence.test.ts` at line 274,
Remove the `as any` cast on the reducePatches result: update the code so
`fullState` has an explicit, correct type instead of using `as any` (for example
declare `const fullState: MaterializedViewState = reducePatches(patches)`), or
change `reducePatches`'s signature to return the proper typed state; locate the
symbol `reducePatches` and the variable `fullState` in the test and replace the
cast with an explicit, concrete type that matches the expected state produced
from `patches`.
Source: Coding guidelines
| ]; | ||
|
|
||
| const fullState = (reduceV5(patches) as any); | ||
| const fullState = (reducePatches(patches) as any); |
There was a problem hiding this comment.
Remove as any cast.
The coding guidelines prohibit as any casting anywhere in TypeScript code.
Recommended fix to remove the cast
- const fullState = (reducePatches(patches) as any);
+ const fullState = reducePatches(patches);📝 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.
| const fullState = (reducePatches(patches) as any); | |
| const fullState = reducePatches(patches); |
🤖 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 `@test/unit/domain/services/MaterializedView.equivalence.test.ts` at line 400,
Remove the unsafe "as any" cast on fullState; instead give fullState a concrete
typed declaration or make reducePatches return the correct type. Locate the
variable fullState and the call reducePatches(patches) and either annotate
fullState with the correct interface/type (e.g., const fullState:
ExpectedStateType = reducePatches(patches)) or update the reducePatches
signature/generics so it returns ExpectedStateType directly; ensure patches is
typed compatibly so no "as any" is necessary.
Source: Coding guidelines
| import { reducePatches as _reducePatches, encodeEdgeKey, encodePropKey } from '../../../../src/domain/services/JoinReducer.ts'; | ||
| const reducePatches = (_reducePatches) as (...args: any[]) => any; |
There was a problem hiding this comment.
Remove any type casting pattern across test files.
Three test files share the same anti-pattern: importing reducePatches then casting it to (...args: any[]) => any. This violates the guideline "Do not use any type in TypeScript code anywhere". The cast is unnecessary—simply import and use the function directly to preserve proper typing.
🤖 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 `@test/unit/domain/services/ProvenancePayload.test.ts` around lines 4 - 5, The
tests import reducePatches as _reducePatches and then cast it to (...args:
any[]) => any which introduces prohibited any usage; remove that cast and simply
use the imported function directly (replace the const reducePatches =
(_reducePatches) as (...args: any[]) => any pattern with directly using
reducePatches/_reducePatches), and do the same cleanup in the other test files
exhibiting this pattern so reducePatches (and related imports like
encodeEdgeKey, encodePropKey) retain their original TypeScript types.
Source: Coding guidelines
| reducePatches as _reducePatches, | ||
| } from '../../../../src/domain/services/JoinReducer.ts'; | ||
| const reduceV5: (...args: any[]) => any = _reduceV5; | ||
| const reducePatches: (...args: any[]) => any = _reducePatches; |
There was a problem hiding this comment.
Remove any type annotation.
The alias uses explicit any type annotation, which violates the coding guidelines that forbid any type usage anywhere in TypeScript code, including tests.
🔧 Suggested fix
-const reducePatches: (...args: any[]) => any = _reducePatches;
+const reducePatches = _reducePatches;Let TypeScript infer the correct type from the imported function.
As per coding guidelines: "Do not use any type in TypeScript code anywhere, including in adapters"
📝 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.
| const reducePatches: (...args: any[]) => any = _reducePatches; | |
| const reducePatches = _reducePatches; |
🤖 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 `@test/unit/domain/services/SyncProtocol.test.ts` at line 15, The alias
declaration for reducePatches currently forces an explicit any type (const
reducePatches: (...args: any[]) => any = _reducePatches;) — remove the explicit
any annotation and let TypeScript infer the type from the imported
_reducePatches (i.e., change the alias to simply const reducePatches =
_reducePatches) so the test obeys the no-any guideline while preserving the
original binding.
Source: Coding guidelines
|
@codex self-code review findings for PR #654.
No source edits were made during this review. |
|
@codex follow-up for the self-review findings.
Local/pre-push validation completed:
|
Release Preflight
If you tag this commit as |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
index.ts (1)
1-508:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftSplit
index.tsto satisfy the 500-LOC source-file cap.This file is 508 LOC, exceeding the binding limit for TS/JS source files. Please split exports/compat aliases into smaller concept-focused modules and re-export from here minimally.
As per coding guidelines: "
**/*.{ts,tsx,js,mjs,cjs}: Limit source files to 500 lines of code (LOC); test files to 800 LOC; bin/scripts to 300 LOC".🤖 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 `@index.ts` around lines 1 - 508, index.ts is 508 LOC and exceeds the 500-LOC limit; split large top-level exports and compatibility aliases into smaller concept-focused modules and keep this file as a minimal re-export surface. Create new modules (e.g., exports/compat.ts, exports/continuum.ts, exports/hologram.ts, exports/indexing.ts) that each export related symbols (for example move WarpApp, openWarpGraph, createNodeAdd/createEdgeAdd/createPropSet, and the V1 factories into a compat module; move Continuum* types and ContinuumArtifactJsonFileAdapter into a continuum module; move createWormhole/composeWormholes/serializeWormhole into a hologram module; move WarpStateIndexBuilder/buildWarpStateIndex and index builders into an indexing module), update index.ts to import only those module entrypoints and re-export a minimal set (including default WarpApp) so index.ts itself drops below 500 LOC while preserving all exported symbols and types.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@index.ts`:
- Around line 1-508: index.ts is 508 LOC and exceeds the 500-LOC limit; split
large top-level exports and compatibility aliases into smaller concept-focused
modules and keep this file as a minimal re-export surface. Create new modules
(e.g., exports/compat.ts, exports/continuum.ts, exports/hologram.ts,
exports/indexing.ts) that each export related symbols (for example move WarpApp,
openWarpGraph, createNodeAdd/createEdgeAdd/createPropSet, and the V1 factories
into a compat module; move Continuum* types and ContinuumArtifactJsonFileAdapter
into a continuum module; move createWormhole/composeWormholes/serializeWormhole
into a hologram module; move WarpStateIndexBuilder/buildWarpStateIndex and index
builders into an indexing module), update index.ts to import only those module
entrypoints and re-export a minimal set (including default WarpApp) so index.ts
itself drops below 500 LOC while preserving all exported symbols and types.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6bb6e2d3-c45e-4b1b-b330-089d754ebd2e
📒 Files selected for processing (47)
CHANGELOG.mdbin/cli/commands/verify-audit.tsdocs/ANTI_SLUDGE_POLICY.mdindex.tspackage.jsonscripts/check-anti-sludge.shscripts/source-version-name-policy.tssrc/domain/continuum/ContinuumGeneratedFamilyInventoryEntry.tssrc/domain/continuum/ContinuumGeneratedFamilyStatus.tssrc/domain/continuum/createCurrentContinuumGeneratedFamilyInventory.tssrc/domain/memory/createBoundedMemoryCapabilityReport.tssrc/domain/migrations/DryRunGraphModelMigrationPlanner.tssrc/domain/migrations/GraphModelMigrationFinalizationConfirmation.tssrc/domain/orset/trie/TrieBranch.tssrc/domain/orset/trie/TrieGeometry.tssrc/domain/orset/trie/TrieStorePort.tssrc/domain/services/GCPolicy.tssrc/domain/services/JoinReducer.tssrc/domain/services/LegacyAnchorDetector.tssrc/domain/services/VisibleStateScope.tssrc/domain/services/Worldline.tssrc/domain/services/index/WarpStateIndexBuilder.tssrc/domain/services/optic/CheckpointTailReadIdentityBuilder.tssrc/domain/services/state/CheckpointSerializer.tssrc/domain/services/state/StateSerializer.tssrc/domain/services/state/checkpointLoad.tssrc/domain/services/strand/conflictCandidateAnalysis.tssrc/domain/services/sync/SyncProtocol.tssrc/domain/services/sync/syncDelta.tssrc/domain/services/sync/syncPatchLoader.tssrc/domain/services/sync/syncRequestResponse.tssrc/domain/trust/verdict.tssrc/domain/types/ops/unions.tssrc/domain/utils/parseStrandBlob.tstest/unit/cli/optic.test.tstest/unit/cli/verify-audit.test.tstest/unit/domain/index.exports.test.tstest/unit/domain/migrations/DryRunGraphModelMigrationPlanner.test.tstest/unit/domain/migrations/GraphModelMigrationFinalizationSafety.test.tstest/unit/domain/migrations/GraphModelMigrationOperationLowering.test.tstest/unit/domain/types/conflict/ConflictResolution.test.tstest/unit/infrastructure/adapters/GraphModelMigrationFinalizationRequestJsonAdapter.test.tstest/unit/scripts/source-version-name-policy.test.tstest/unit/scripts/v18-graph-model-migration-command-cli.test.tstest/unit/scripts/v18-graph-model-migration-dry-run.test.tstest/unit/scripts/v18-migration-command.test.tstest/unit/scripts/v18-migration-finalizer.test.ts
✅ Files skipped from review due to trivial changes (16)
- src/domain/trust/verdict.ts
- src/domain/continuum/createCurrentContinuumGeneratedFamilyInventory.ts
- test/unit/domain/migrations/DryRunGraphModelMigrationPlanner.test.ts
- src/domain/orset/trie/TrieStorePort.ts
- src/domain/orset/trie/TrieBranch.ts
- src/domain/continuum/ContinuumGeneratedFamilyStatus.ts
- src/domain/services/sync/syncDelta.ts
- src/domain/utils/parseStrandBlob.ts
- src/domain/services/GCPolicy.ts
- src/domain/services/index/WarpStateIndexBuilder.ts
- src/domain/continuum/ContinuumGeneratedFamilyInventoryEntry.ts
- src/domain/services/state/CheckpointSerializer.ts
- src/domain/orset/trie/TrieGeometry.ts
- src/domain/services/LegacyAnchorDetector.ts
- src/domain/services/Worldline.ts
- src/domain/services/sync/SyncProtocol.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- src/domain/services/sync/syncPatchLoader.ts
- test/unit/domain/index.exports.test.ts
- src/domain/services/sync/syncRequestResponse.ts
- src/domain/services/state/checkpointLoad.ts
- src/domain/memory/createBoundedMemoryCapabilityReport.ts
- src/domain/types/ops/unions.ts
- src/domain/services/JoinReducer.ts
- src/domain/services/VisibleStateScope.ts
- CHANGELOG.md
- src/domain/services/state/StateSerializer.ts
Closes #655.
Summary
srcno longer carries release-versioned nouns such as V5, V2, V1, or V18 in active symbols or filenames.srcand intoscripts/v18.0.0/migrations/graph-model/as migration-path artifacts.Validation
npm run typechecknpm run lintgit diff --checknpm test -- --run test/unit/domain/services/JoinReducer.test.ts test/unit/domain/services/StateSerializer.test.ts test/unit/domain/services/JoinReducer.stateSession.test.tsnpm test -- --run test/unit/domain/migrations/V17GoldenGraphFixtureGenesisReading.test.ts test/unit/scripts/v18-v17-golden-graph-fixtures.test.ts test/unit/domain/continuum/GitWarpGraphModelContractConformance.test.ts test/unit/domain/continuum/GitWarpWarpTtdGeneratedFamilySmoke.test.tsnpm test -- --run test/unit/scripts/public-api-advanced-guide-shape.test.tsnpm test -- --run test/unit/scripts/release-policy-shape.test.ts test/unit/scripts/v18-release-story-shape.test.tsnpm testDocker-backed full stable unit suiteSource Smell Check
find src -iname '*v[0-9]*' -o -iname '*V[0-9]*'returns no paths.IPv4,IPv6, andfnv1aBytes.Summary by CodeRabbit
Changed
signed_evidence_v1tosigned_evidenceDeprecated
Chores