Implement mapping edit core for order 4 - #4
Merged
Merged
Conversation
… and host dagre resolution. Add contracts/application graph editing, fix unused-block detection by output reachability, and wire macOS to resolve @dagrejs/dagre after npm ci. Co-authored-by: Cursor <cursoragent@cursor.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughマッピング編集向けに、共有グラフ契約、可変グラフモデル、検証・評価・自動整列、Undo/Redo を含む Changesマッピング編集基盤
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Editor
participant MappingSession
participant GraphModel
participant validateGraph
participant evaluateGraph
Editor->>MappingSession: グラフ編集操作
MappingSession->>GraphModel: ノード・辺を更新
GraphModel-->>MappingSession: CommandResult
MappingSession->>validateGraph: 検証を実行
validateGraph-->>MappingSession: GraphIssue[]
Editor->>MappingSession: 入力値で評価
MappingSession->>evaluateGraph: グラフを評価
evaluateGraph-->>MappingSession: 出力値 Map
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@packages/application/jest.config.js`:
- Around line 3-6: Update the Jest configuration to include `@csvmapper/contracts`
in the transform scope by exempting it from the default node_modules
transformIgnorePatterns, so its TypeScript entry point is processed by the
existing ts-jest preset.
In `@packages/application/src/graph/model.ts`:
- Around line 216-228: Update moveNodes to validate every NodeId in positions
before mutating any node positions, returning UnknownNode immediately for an
invalid ID; only after validation succeeds should it apply all rounded
coordinates, preserving the CommandFailure contract that failed commands leave
state unchanged.
- Around line 346-400: Update the missing-endpoint validation in addEdgeInternal
to return GraphErrorCode.UnknownNode instead of GraphErrorCode.TerminalMismatch
when either fromNode or toNode is absent. Preserve the existing error message
and all other edge-validation behavior.
- Around line 273-305: Update setJoinInputOrder so it completes all
orderedEdgeIds validation, including membership, duplicates, and edge existence,
before changing any edge.joinOrder values. After validation succeeds, apply the
new joinOrder values in a separate pass; every failure path must leave the graph
state unchanged.
In `@packages/application/src/graph/validation.ts`:
- Around line 58-89: Normalize output names with trim before duplicate detection
and storage in the outputNames map within the output-node validation flow. Keep
the existing empty-name validation based on the trimmed value, and ensure names
differing only by leading or trailing whitespace are reported as
DuplicateOutputName.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: bd5795de-19dc-43d1-ad21-641526d78211
⛔ Files ignored due to path filters (3)
apps/macos/package-lock.jsonis excluded by!**/package-lock.jsonpackages/application/package-lock.jsonis excluded by!**/package-lock.jsonpackages/contracts/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (28)
.gitignoreapps/macos/metro.config.jsapps/macos/package.jsondocs/implementation-plan.mddocs/specs/design/application-architecture.mddocs/specs/modules/processing-core.mdpackages/application/__tests__/graph-error.test.tspackages/application/__tests__/graph-normal.test.tspackages/application/__tests__/graph-undo.test.tspackages/application/jest.config.jspackages/application/package.jsonpackages/application/src/graph/evaluate.tspackages/application/src/graph/model.tspackages/application/src/graph/validation.tspackages/application/src/index.tspackages/application/src/layout/autoLayout.tspackages/application/src/session/history.tspackages/application/src/session/store.tspackages/application/tsconfig.jsonpackages/contracts/package.jsonpackages/contracts/src/blocks.tspackages/contracts/src/edges.tspackages/contracts/src/ids.tspackages/contracts/src/index.tspackages/contracts/src/issues.tspackages/contracts/src/nodes.tspackages/contracts/src/snapshot.tspackages/contracts/tsconfig.json
Co-authored-by: Cursor <cursoragent@cursor.com>
5 tasks
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
1 participant
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.
Summary
packages/contracts/packages/applicationでマッピング編集コア（グラフ接続検証・Undo/Redo・自動整列・軽量評価）を追加し、順序 4 を完了扱いに更新したUnusedBlockを出力辺の有無ではなく出力項目への到達可能性で判定するようにし、中間デッドエンドも警告対象にした@dagrejs/dagreを直接依存し、Metro からリンク先パッケージ経由でも解決できるようにしたTest plan
cd packages/application && npm testが GRAPH-001〜006 / E001〜E005 を含む全件でパスするUnusedBlockの到達可能性ケース（入力→A→B で出力未接続）で A/B 両方に警告が付くことを確認するcd apps/macos && npm ci後にnode_modules/@dagrejs/dagreが存在し、autoLayoutの import が解決できることを確認するMade with Cursor
Summary by CodeRabbit