Skip to content
This repository was archived by the owner on Sep 23, 2026. It is now read-only.

Extract reflection engine to devonian; depend on it instead - #58

Merged
michielbdejong merged 2 commits into
mainfrom
claude/youthful-mendel-3ujtxo
Sep 21, 2026
Merged

michielbdejong merged 2 commits into
mainfrom
claude/youthful-mendel-3ujtxo

Conversation

@michielbdejong

Copy link
Copy Markdown
Contributor

Summary

Reflector's generic bidirectional reflection engine (ReflectionEngine, markers, id-map, state ledger) has been extracted into a new devonian package and is now a dependency. Reflector continues to provide the same public API (ReflectionService) but now delegates the core reflection logic to devonian while retaining only the reflector-specific machinery: building ReflectionSides from reflector's document/overlay/OAuth stack, and the background loop/on-demand trigger/status reporting.

This resolves localthought/atomic-plugins#6 by enabling other plugins to reuse the same generic reflection engine without duplicating code.

Key Changes

  • src/reflect/engine.ts (deleted) — ReflectionEngine now comes from devonian/reflect; the generic reflect-loop logic is no longer reflector-specific.

  • src/reflect/sides.ts (new) — buildReflectionSide() encapsulates the reflector-specific half: builds a devonian ReflectionSide from reflector's document/overlay/resource-model/OAuth machinery. Extracted from private methods that lived in the old ReflectionEngine (bind(), commentClient(), setState()).

  • src/reflect/marker.ts — Now a thin wrapper around devonian/reflect's namespaced marker codec, pinned to the reflector namespace so existing <!-- reflector:origin ... --> markers in production records continue to parse identically.

  • src/reflect/id-map.ts & src/reflect/kv-store.ts — Re-exported from devonian/reflect (these were already fully generic).

  • src/reflect/service.ts — ReflectionService retains its exact public shape (fromConfig/start/stop/reflectNow/status) but now:

    • Builds two ReflectionSides via buildReflectionSide().
    • Constructs devonian's ReflectionEngine with namespace: 'reflector'.
    • Delegates background-loop/on-demand-trigger/status behavior to devonian's ReflectionRunner.
  • package.json — Added devonian@^0.6.0 as a dependency.

  • __tests__/unit/reflect/engine.test.ts — Rewritten as an integration test through reflector's real document/overlay/resource-discovery pipeline driving devonian's engine. Unit coverage of the engine itself now lives in devonian's own test suite.

Implementation Details

  • The marker namespace is explicitly pinned to reflector in src/reflect/marker.ts to ensure backward compatibility with already-embedded markers in production records.
  • ReflectionService continues to expose the same interface to src/server/app.ts and src/main.ts; no changes needed in callers.
  • The test suite now validates that reflector's GitHub OpenAPI document + overlay correctly produces a ReflectionSide that devonian's engine can drive, rather than testing the engine in isolation (which is now devonian's responsibility).

https://claude.ai/code/session_01B2mbssSNDiu8GpKjaCFRn1

Per the decision recorded in ontola/atomic-plugins#6: reflector's
bidirectional GitHub-issue reflection engine (origin markers, id-map, echo
suppression, state reconciliation, comment reflection) has been generalized
and extracted to devonian/reflect (localthought/devonian@0.6.0, not yet
published). Reflector now depends on it instead of keeping its own copy.

- src/reflect/sides.ts (new) builds devonian's ReflectionSide from
  reflector's own document/overlay/resource-model/OAuth machinery — the
  reflector-specific half devonian's generic engine doesn't know about.
- src/reflect/engine.ts is gone; ReflectionEngine now comes from
  devonian/reflect.
- marker.ts/id-map.ts/kv-store.ts are now thin wrappers/re-exports.
  marker.ts pins the `reflector` namespace so markers already embedded in
  production records keep parsing unchanged.
- service.ts keeps its exact public API (fromConfig/start/stop/reflectNow/
  status) but delegates the loop/status machinery to devonian's
  ReflectionRunner.

__tests__/unit/reflect/engine.test.ts is rewritten as an integration test
through reflector's real document/overlay pipeline driving devonian's real
engine, per this project's stated preference for real-engine-plus-fake-HTTP
tests over mocking internals; marker/id-map/service tests needed no changes.

Verified (against a locally-linked devonian build, not committed) with
tsc --noEmit, eslint, `pnpm test` (88 passing), and `pnpm test:harness` (9
passing). pnpm-lock.yaml is intentionally left as-is: devonian@0.6.0 isn't
published yet, so `pnpm install` on this branch will correctly report the
lockfile out of sync with package.json until that publish happens — this
commit's dependency bump is meant to land after devonian's does.

Claude-Session: https://claude.ai/code/session_01B2mbssSNDiu8GpKjaCFRn1
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
devonian@0.6.0's tarball shipped with a stale build/ missing src/reflect
(fixed upstream in devonian's 0.6.1, which adds a prepublishOnly build step
so this can't recur). Re-resolves the devonian@^0.6.0 dependency added in
the previous commit against the real, now-published 0.6.1.

Verified: tsc --noEmit, eslint, prettier --check, `pnpm test` (88 passing),
`pnpm test:harness` (9 passing), and `pnpm build` all green against the
actual npm-resolved package (no local link).

Claude-Session: https://claude.ai/code/session_01B2mbssSNDiu8GpKjaCFRn1
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@michielbdejong
michielbdejong merged commit cc41678 into main Sep 21, 2026
1 check passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Handoff: Reflector/devonian architecture for retiring reflector-rs (supersedes ontola/atomic-server#1599 discussion)

2 participants