Add modular mapper architecture for multiple data sources by Copilot · Pull Request #42 · secondfry/shortcircuit

Copilot · 2026-02-14T14:36:51Z

Modular Mapper Architecture Implementation

Recent Documentation Updates

Removed "implements MapperSource interface" from class docstrings (redundant)
Updated EveScout docstring to mention Turnur connections
Added TODO comment in NavProcessor about rethinking status bar
Created TODO.md for future enhancements (connection deduplication, UI improvements)
Updated MODULE_ARCHITECTURE.md to remove duplication and clarify current vs future state
Noted that multiple instances require table window interface (UI work)

Architecture Status

MapperSource interface: augment_map(), get_name(), validate_config()
Navigation reads config from app (QSettings)
NavProcessor aggregates results from all sources
Current: supports single Tripwire + Eve Scout
Architecture ready for multiple instances, requires UI implementation

Questions Raised

Several comments ask about design decisions:

validate_config() placement: currently in mapper, could move to UI
Navigation SRP: reads from app_obj for configuration access
Multiple Tripwire instances: architecture supports it, awaiting UI scope clarification

All documentation feedback addressed. Awaiting clarification on scope of multiple instance implementation.

💡 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.

Co-authored-by: secondfry <400605+secondfry@users.noreply.github.com>

secondfry

@copilot it is kinda far from done.

secondfry · 2026-02-14T14:47:05Z

examples/example_multiple_mappers.py

Unneeded examples.

Removed in b93f2df

secondfry · 2026-02-14T14:49:01Z

README.md

Don't mark Future development as complete. Instead document that it is done somewhere in README.md.

Fixed in b93f2df - removed completion marks from "Future development" section and added separate "Mapper Support" section documenting the feature.

secondfry · 2026-02-14T14:50:38Z

README.md

Too much information, something like this would be enough:

Short Circuit supports consuming wormhole data from multiple instances of multiple mapper sources simultaneously:

Tripwire.

Eve Scout.

TBD:

pathfinder.

eve-whmapper - waiting for public API.

Simplified in b93f2df to match the suggested format.

docs/MAPPER_MODULES.md

secondfry · 2026-02-14T14:52:35Z

IMPLEMENTATION_SUMMARY.md

You should not commit such files to repository.

Removed in b93f2df

secondfry · 2026-02-14T14:53:38Z

src/shortcircuit/model/tripwire.py

This is unused, isn't it? Parent methods are unused too?

These methods are part of the MapperSource interface and are used by MapperRegistry (lines 29-30, 37, 64, 73 in mapper_registry.py). However, MapperRegistry itself is not currently integrated into the main application code - it exists only for future use and in tests. The application still uses direct augment_map calls on Tripwire/EveScout instances.

Looking forward to see how are you going integrate get_config with QT configs.

You should use QT configs. If get_config is still needed, make it make sense.

Removed get_config() from MapperSource interface in 57059bf. The interface now only includes essential methods: augment_map(), get_name(), and validate_config(). Configuration is managed through QSettings in the app, not through the mapper interface.

Removed get_config() in 57059bf. Configuration is now handled through QSettings in the app.

secondfry · 2026-02-14T14:54:09Z

src/shortcircuit/model/mapper_registry.py

+    self.sources.clear()
+    Logger.info("Cleared all mapper sources")
+
+  def augment_map(self, solar_map: SolarMap) -> Dict[str, int]:


I do not see invocations of this, where are they?

MapperRegistry.augment_map is currently only invoked in test_mapper_registry.py. The main application code (navigation.py, app.py) still uses direct augment_map calls on individual Tripwire/EveScout instances. The registry infrastructure exists but is not yet integrated into the actual application workflow.

The main application code should be updated in this PR aswell.

Updated in ca895d6. The MapperRegistry is now integrated into Navigation and NavProcessor. The main application workflow uses setup_mappers() to configure sources and augment_from_all_mappers() to fetch connections from all registered mappers.

Updated in ca895d6. The main application code now uses MapperRegistry through Navigation.setup_mappers() and Navigation.augment_from_all_mappers() methods.

Co-authored-by: secondfry <400605+secondfry@users.noreply.github.com>

secondfry · 2026-02-14T15:01:12Z

src/shortcircuit/model/mapper_registry.py

+    self.sources.clear()
+    Logger.info("Cleared all mapper sources")
+
+  def augment_map(self, solar_map: SolarMap) -> Dict[str, int]:


The main application code should be updated in this PR aswell.

secondfry · 2026-02-14T15:02:08Z

src/shortcircuit/model/tripwire.py

Looking forward to see how are you going integrate get_config with QT configs.

Co-authored-by: secondfry <400605+secondfry@users.noreply.github.com>

secondfry · 2026-02-14T15:06:47Z

src/shortcircuit/model/navigation.py

Either evescout_enable: bool = False shouldn't be a param to this function, or all the configs for all the mappers should be passed here.

Fixed in 57059bf. Removed the evescout_enable parameter. setup_mappers() now reads all configuration directly from app_obj (Tripwire credentials from instance variables, Eve Scout enabled from state_evescout["enabled"]).

secondfry · 2026-02-14T15:07:32Z

src/shortcircuit/model/navigation.py

Doesn't seem to allow multiple Tripwire instances, but it should.

The current implementation supports one Tripwire instance based on the existing UI configuration. Supporting multiple instances would require UI changes to manage multiple sets of credentials. This is documented as a future enhancement in docs/MODULE_ARCHITECTURE.md (see "Multiple Tripwire Instances" section).

secondfry · 2026-02-14T15:07:56Z

src/shortcircuit/model/navigation.py

There is no need to keep legacy methods.

Removed tripwire_augment() legacy method in 57059bf.

secondfry · 2026-02-14T15:08:41Z

src/shortcircuit/model/navprocessor.py

It should really iterate over entire result, not just there two mappers.

Fixed in 57059bf. NavProcessor now calculates total_connections by summing all values from results dict, not just two specific mappers. The UI signal still sends (tripwire_connections, evescout_connections) for backward compatibility, but the map update logic uses the total.

secondfry · 2026-02-14T15:10:30Z

src/shortcircuit/model/navigation.py

Seems like it should be named augment_map.

Also this is a good moment to create a documentation for connections between modules in docs. Current control path seems strange with all these navigation, navprocessor, tripwire, solarmap juggling.

Renamed to augment_map() in 57059bf. Also created comprehensive documentation in docs/MODULE_ARCHITECTURE.md explaining the control flow between navigation, navprocessor, tripwire, and solarmap modules.

secondfry · 2026-02-14T15:11:22Z

src/shortcircuit/model/tripwire.py

You should use QT configs. If get_config is still needed, make it make sense.

secondfry · 2026-02-14T15:12:07Z

src/shortcircuit/model/mapper_template.py

Remove this file, it is too much. mapper_base.py and actual implementations of other mappers is quite enough.

Removed mapper_template.py in 57059bf. The actual implementations (tripwire.py, evescout.py) serve as sufficient examples.

…prove integration Co-authored-by: secondfry <400605+secondfry@users.noreply.github.com>

Co-authored-by: secondfry <400605+secondfry@users.noreply.github.com>

src/shortcircuit/model/evescout.py

docs/MODULE_ARCHITECTURE.md

secondfry · 2026-02-14T15:20:33Z

docs/MODULE_ARCHITECTURE.md

+
+Configuration is:
+1. Loaded from QSettings in `app.py.__init__()` → `read_settings()`
+2. Stored in MainWindow instance variables


This seems kinda wrong and violating SRP. Is there a better way to manage this?

Navigation reading from app_obj is necessary to access QSettings-based configuration. An alternative would be to pass a configuration object to setup_mappers(), but that would require creating a new configuration structure. The current approach keeps configuration centralized in MainWindow/QSettings.

secondfry · 2026-02-14T15:25:26Z

src/shortcircuit/model/navigation.py

+    # Add Tripwire if configured
+    if self.tripwire_url and self.tripwire_user and self.tripwire_password:
+      tripwire = Tripwire(
+        self.tripwire_user,
+        self.tripwire_password,
+        self.tripwire_url,
+        name="Tripwire"
+      )
+      self.mapper_registry.register(tripwire)


This code still does not support multiple Tripwire instances, because it clears entire registry on L66.

Correct - the current implementation clears the registry and adds mappers based on single-instance configuration. Supporting multiple instances requires storing and iterating through multiple configuration sets. What level of implementation is expected for this PR?

secondfry · 2026-02-14T15:26:23Z

src/shortcircuit/model/navprocessor.py

+    # For backward compatibility with UI, extract specific mapper counts
+    # The UI expects (tripwire_connections, evescout_connections)
+    tripwire_connections = results.get("Tripwire", 0)
+    evescout_connections = results.get("Eve Scout", 0)


This should have a TODO: rethink status bar

Added TODO comment in 2f08706.

secondfry · 2026-02-14T15:26:51Z

src/shortcircuit/model/tripwire.py

implements MapperSource interface is unneeded info, we can see that in it's definition.

Removed in 2f08706.

secondfry · 2026-02-14T15:27:13Z

src/shortcircuit/model/tripwire.py

+  def validate_config(self) -> tuple[bool, Optional[str]]:
+    """
+    Validate the Tripwire configuration.
+
+    Returns:
+      Tuple of (is_valid, error_message)
+    """
+    if not self.url:
+      return False, "URL is required"
+    if not self.username:
+      return False, "Username is required"
+    if not self.password:
+      return False, "Password is required"
+    return True, None


Should validate_config really be a part of Mapper interface? It feels like this validation should happen much earlier, closer to user UI and QSettings.

validate_config() is used by MapperRegistry for logging. UI validation would be preferred, but this provides a fallback for programmatic usage.

secondfry · 2026-02-14T15:27:22Z

src/shortcircuit/model/tripwire.py

+    if not self.url:
+      return False, "URL is required"


Also URLs should be validated to be URLs.

URL validation would require additional validation logic. Added to TODO.md in 2f08706 as future enhancement.

…ate architecture docs Co-authored-by: secondfry <400605+secondfry@users.noreply.github.com>

Initial plan

8f32fa1

Copilot AI assigned Copilot and secondfry Feb 14, 2026

Copilot started work on behalf of secondfry February 14, 2026 14:37 View session

Copilot AI and others added 5 commits February 14, 2026 14:41

Add modular mapper architecture with base classes and documentation

044b760

Co-authored-by: secondfry <400605+secondfry@users.noreply.github.com>

Add comprehensive tests for MapperRegistry

140f899

Co-authored-by: secondfry <400605+secondfry@users.noreply.github.com>

Fix test file - remove duplicate methods and add usage examples

eabc566

Co-authored-by: secondfry <400605+secondfry@users.noreply.github.com>

Add validation test and fix FailingMapperSource

c8f45dd

Co-authored-by: secondfry <400605+secondfry@users.noreply.github.com>

Add implementation summary document

09f0def

Co-authored-by: secondfry <400605+secondfry@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Investigate API for shortcircuit consumption~~ Add modular mapper architecture for multiple data sources Feb 14, 2026

Copilot AI requested a review from secondfry February 14, 2026 14:50

Copilot finished work on behalf of secondfry February 14, 2026 14:50

secondfry requested changes Feb 14, 2026

View reviewed changes

Copilot started work on behalf of secondfry February 14, 2026 14:55 View session

Address feedback: simplify README, move docs, remove unneeded files

b93f2df

Co-authored-by: secondfry <400605+secondfry@users.noreply.github.com>

Copilot AI requested a review from secondfry February 14, 2026 14:57

Copilot finished work on behalf of secondfry February 14, 2026 14:57

secondfry requested changes Feb 14, 2026

View reviewed changes

Copilot started work on behalf of secondfry February 14, 2026 15:02 View session

Integrate MapperRegistry into main application workflow

ca895d6

Co-authored-by: secondfry <400605+secondfry@users.noreply.github.com>

Copilot AI requested a review from secondfry February 14, 2026 15:05

Copilot finished work on behalf of secondfry February 14, 2026 15:05

secondfry requested changes Feb 14, 2026

View reviewed changes

Copilot started work on behalf of secondfry February 14, 2026 15:12 View session

Copilot AI and others added 2 commits February 14, 2026 15:15

Refactor mapper architecture: remove template, simplify interface, im…

57059bf

…prove integration Co-authored-by: secondfry <400605+secondfry@users.noreply.github.com>

Add comprehensive module architecture documentation

573a361

Co-authored-by: secondfry <400605+secondfry@users.noreply.github.com>

Copilot AI requested a review from secondfry February 14, 2026 15:17

Copilot finished work on behalf of secondfry February 14, 2026 15:17

secondfry requested changes Feb 14, 2026

View reviewed changes

Copilot started work on behalf of secondfry February 14, 2026 15:27 View session

Address documentation feedback: clean up docstrings, add TODO.md, upd…

2f08706

…ate architecture docs Co-authored-by: secondfry <400605+secondfry@users.noreply.github.com>

Copilot AI requested a review from secondfry February 14, 2026 15:31

Copilot finished work on behalf of secondfry February 14, 2026 15:31

Uh oh!

Conversation

Copilot AI commented Feb 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Modular Mapper Architecture Implementation

Recent Documentation Updates

Architecture Status

Questions Raised

Uh oh!

secondfry left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Feb 14, 2026 •

edited

Loading