-
Notifications
You must be signed in to change notification settings - Fork 46
Add topological sort for safe output messages with temporary ID dependencies #13066
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Create safe_output_topological_sort.cjs with sorting algorithm - Implement dependency graph analysis for temporary IDs - Add comprehensive test coverage (35 tests) - Integrate into unified handler manager - Enable single-pass resolution of temporary IDs Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- All 35 topological sort tests passing - All unified handler tests passing - JavaScript formatting validated - TypeScript type checking passed - Build successful Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- Define TEMPORARY_ID_PATTERN as module-level constant - Use constant instead of inline regex - Improves maintainability and consistency - All tests still passing Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot keep temporary id helper in temporary_id helpe file, and only keep graph /topological sort in new file Add extensive testing. |
- Simplify in-degree calculation using deps.size - Add assertions to verify only first occurrence is used for duplicates - All tests passing - Code more readable and maintainable Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- Move extractTemporaryIdReferences to temporary_id.cjs - Move getCreatedTemporaryId to temporary_id.cjs - Keep only graph/topological sort logic in safe_output_topological_sort.cjs - Move tests to appropriate test files - temporary_id.test.cjs now has 62 tests (was 50) - safe_output_topological_sort.test.cjs now has 23 tests (was 35) - All 3215 JavaScript tests passing Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done! Moved temporary ID helpers ( |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
💀 Blimey! Changeset Generator was cancelled and walked the plank! No treasure today, matey! ☠️ |
Agent Container Tool Check
Result: 10/12 tools available Missing Tools:
All core development tools (bash, git, jq, yq, curl, gh, node, python3, go) are available. Java and .NET runtimes are not installed in this container.
|
|
PR titles: Add Serena MCP tool usage statistical analysis with request/response size metrics | Merge main branch and recompile workflows
|
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
- Use iterative DFS with explicit stack instead of recursion - Implement mark-and-sweep with WHITE/GRAY/BLACK coloring - Track parent pointers to reconstruct cycle path - No recursion - uses todo stack for traversal - All 23 tests passing - All 3215 JavaScript tests passing Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot merge main and recompile |
…al-sort-function # Conflicts: # actions/setup/js/safe_output_unified_handler_manager.cjs
- Merged origin/main (resolved conflicts in safe_output_unified_handler_manager.cjs) - Recompiled all 147 workflows successfully - Added 11 extensive new tests for topological sort - Now 34 tests total (was 23) covering: - Large graphs with many dependencies (61 messages) - Deeply nested linear dependencies - Multiple disconnected chains - Diamond dependency patterns - Multiple temp ID references in single message - Empty/single message edge cases - Object reference preservation - 3-node cycles - Mixed message types (issues, PRs, discussions, comments) - All 3230 JavaScript tests passing Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done! Merged origin/main (resolved conflicts in safe_output_unified_handler_manager.cjs) and recompiled all 147 workflows successfully. Added 11 extensive new tests - now 34 total (was 23) covering large graphs (61 messages), deeply nested chains, disconnected components, diamond patterns, multi-reference messages, edge cases, cycles, and mixed types. All 3,230 JavaScript tests passing. (commits 63d7184, fefb142) |
|
🎉 Yo ho ho! Changeset Generator found the treasure and completed successfully! ⚓💰 |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
Agent Container Tool Check ✅
Result: 10/12 tools available Notes:
|
Smoke Test ResultsRecent PRs:
Test Results:
Overall Status: PASS
|
|
PRs: Fix: Use workflow ID instead of workflow name for comment search
|
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
Topological Sort for Safe Output Tool Calls ✅
Refactored per code review feedback
Recent Changes:
Changes from previous feedback:
temporary_id.cjsImplementation
Updated:
temporary_id.cjsextractTemporaryIdReferences()- Finds all temp IDs referenced in a messagegetCreatedTemporaryId()- Gets the temp ID a message createstemporary_id.test.cjs(added 12 new tests)File:
safe_output_topological_sort.cjsbuildDependencyGraph()- Builds a dependency graph from messagesdetectCycle()- Iterative cycle detection using mark-and-sweep with explicit stacktopologicalSort()- Sorts using Kahn's algorithmsortSafeOutputMessages()- Main function to sort messagessafe_output_topological_sort.test.cjs(added 11 extensive tests)Updated:
safe_output_unified_handler_manager.cjsHow it works:
Test Coverage:
✅ 96 total tests (62 in temporary_id.test.cjs, 34 in safe_output_topological_sort.test.cjs)
✅ All 3,230 JavaScript tests passing
✅ Extensive test coverage including:
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.
Changeset
temporary_id.cjsmodule and move the related tests.Changeset