From 041f957a7f7f09c825e5f3ddcd57303139555151 Mon Sep 17 00:00:00 2001 From: jinliyl <6469360+jinliyl@users.noreply.github.com> Date: Mon, 1 Jun 2026 11:35:19 +0800 Subject: [PATCH] refactor(components) components and file I/O, fix method calls and validation (#268) * refactor(components): extract shared component state into mixin - Introduce ComponentMixin class with shared state for components and steps - Move identity, config, and vault path functionality to ComponentMixin - Update BaseComponent to inherit from ComponentMixin - Update BaseStep to inherit from ComponentMixin - Consolidate vault path helper methods in ComponentMixin - Remove duplicate vault path implementations from BaseComponent and BaseStep - Add ComponentMixin to components module exports * refactor(file_io): implement path locks cache eviction mechanism - Add _PATH_LOCKS_MAX constant set to 1024 for cache size limit - Implement cache eviction logic when locks exceed maximum capacity - Remove half of unlocked entries when cache limit is reached - Use list comprehension to identify unlocked locks for removal - Maintain existing path normalization and locking behavior fix(edit): correct method call from public to private fail method - Change self.fail to self._fail for internal error handling - Maintain consistent private method usage within class fix(mcp_client): change pop to get for optional command and args - Replace kwargs.pop with kwargs.get to avoid removing keys - Preserve original kwargs dictionary contents - Maintain default empty string and list values feat(reme): add client backend validation with error raising - Check if client_cls is None before instantiation - Raise ValueError with descriptive message for unknown backends - Provide clear error feedback for invalid backend configurations * fix(components): move directory creation to start method - Moved component_metadata_path.mkdir call from __init__ to _start in base_keyword_index - Moved component_metadata_path.mkdir call from __init__ to _start in local_file_graph - Moved component_metadata_path.mkdir call from __init__ to _start in local_file_store - Ensures directory creation happens after component initialization - Prevents potential issues with path creation during object construction * fix(steps): replace assertions with runtime errors for app_context validation - Replace assert statements with explicit RuntimeError exceptions when app_context is None - Add descriptive error messages for better debugging when resolving components - Replace assert in resolve_component method with proper exception handling - Replace assert in get_file_parser method with proper exception handling - Maintain same functionality while improving error reporting clarity * refactor(file_io): split file IO utilities into modular components - Move daily note helpers to separate _daily_index module - Extract path validation and resolution to new _path module - Remove unused code and imports from _file_io module - Update import statements across affected modules - Introduce WikilinkHandler utility for link parsing - Replace regex-based link extraction with WikilinkHandler - Add integration JSONL files to gitignore - Consolidate file locking mechanism in _file_io module * style(formatter): fix spacing issues in file IO and chunked file parser - Fixed whitespace around colon in slice notation in file_io.py - Corrected spacing around colon in slice notation in chunked_file_parser.py - Applied consistent formatting for array slicing operations - Improved code readability by standardizing space placement in ranges * refactor(steps): replace property-based component resolution with Ref descriptor - Introduce Ref descriptor class for lazy component dependency resolution - Replace _resolve method and individual properties with Ref descriptors - Add as_llm, as_llm_formatter, as_token_counter, file_store, and embedding Ref attributes - Remove legacy property methods and resolve logic from BaseStep - Add cache clearing mechanism for Ref values during step calls - Update UpdateCatalogStep to use Ref instead of property-based resolution --- .gitignore | 3 +- reme4/components/__init__.py | 3 +- reme4/components/base_component.py | 61 ++- reme4/components/client/mcp_client.py | 4 +- .../components/file_graph/local_file_graph.py | 2 +- .../file_parser/chunked_file_parser.py | 47 +- .../components/file_store/local_file_store.py | 2 +- .../keyword_index/base_keyword_index.py | 2 +- reme4/reme.py | 2 + reme4/steps/base_step.py | 153 ++++--- reme4/steps/file_io/_daily_index.py | 118 +++++ reme4/steps/file_io/_file_io.py | 415 ++---------------- reme4/steps/file_io/_path.py | 96 ++++ reme4/steps/file_io/daily_create.py | 3 +- reme4/steps/file_io/daily_list.py | 2 +- reme4/steps/file_io/daily_reindex.py | 2 +- reme4/steps/file_io/edit.py | 12 +- reme4/steps/file_io/read.py | 3 +- reme4/steps/file_io/read_image.py | 2 +- reme4/steps/file_io/write.py | 10 +- reme4/steps/index/update_catalog.py | 18 +- tests4/unit/test_chunked_file_parser.py | 41 +- 22 files changed, 433 insertions(+), 568 deletions(-) create mode 100644 reme4/steps/file_io/_daily_index.py create mode 100644 reme4/steps/file_io/_path.py diff --git a/.gitignore b/.gitignore index 04a536d5..9bb3ea32 100644 --- a/.gitignore +++ b/.gitignore @@ -46,4 +46,5 @@ meta_memory/* *.db memories/* .reme/* -vault \ No newline at end of file +vault +*/integration/*.jsonl \ No newline at end of file diff --git a/reme4/components/__init__.py b/reme4/components/__init__.py index 7239c7d6..8b7af531 100644 --- a/reme4/components/__init__.py +++ b/reme4/components/__init__.py @@ -14,7 +14,7 @@ from . import keyword_index from . import service from . import tokenizer from .application_context import ApplicationContext -from .base_component import BaseComponent +from .base_component import BaseComponent, ComponentMixin from .component_registry import ComponentRegistry, R from .prompt_handler import PromptHandler from .runtime_context import RuntimeContext @@ -22,6 +22,7 @@ from .runtime_context import RuntimeContext __all__ = [ "ApplicationContext", "BaseComponent", + "ComponentMixin", "ComponentRegistry", "R", "PromptHandler", diff --git a/reme4/components/base_component.py b/reme4/components/base_component.py index bc96c64b..85b5a5ec 100644 --- a/reme4/components/base_component.py +++ b/reme4/components/base_component.py @@ -14,6 +14,42 @@ if TYPE_CHECKING: T = TypeVar("T", bound="BaseComponent") +class ComponentMixin: + """Shared state for components and steps: identity, config, vault paths.""" + + component_type = ComponentEnum.BASE + + def __init__( + self, + name: str | None = None, + backend: str = "", + app_context: "ApplicationContext | None" = None, + **kwargs, + ) -> None: + self.name: str = name or self.__class__.__name__ + self.backend: str = backend + self.app_context: "ApplicationContext | None" = app_context + self.kwargs: dict = dict(kwargs) + + logger = get_logger() + self.logger = logger.bind(component=self.name) if hasattr(logger, "bind") else logger + + @property + def vault_path(self) -> Path: + """Absolute vault root directory (cwd when no app_context is attached).""" + if self.app_context is None: + return Path.cwd() + return Path(self.app_context.app_config.vault_dir).absolute() + + def to_vault_relative(self, path: str | Path) -> str: + """Convert `path` to a vault-relative string; return absolute path when outside.""" + abs_path = Path(path).absolute() + try: + return str(abs_path.relative_to(self.vault_path)) + except ValueError: + return str(abs_path) + + class Dependency: """Placeholder returned by ``BaseComponent.bind`` for an unresolved dependency. @@ -46,7 +82,7 @@ class Dependency: ) -class BaseComponent(ABC): +class BaseComponent(ComponentMixin, ABC): """Async lifecycle base class with bind-based dependency injection.""" component_type = ComponentEnum.BASE @@ -58,13 +94,7 @@ class BaseComponent(ABC): app_context: "ApplicationContext | None" = None, **kwargs, ) -> None: - self.name: str = name or self.__class__.__name__ - self.backend: str = backend - self.app_context: "ApplicationContext | None" = app_context - self.kwargs: dict = dict(kwargs) - - logger = get_logger() - self.logger = logger.bind(component=self.name) if hasattr(logger, "bind") else logger + super().__init__(name=name, backend=backend, app_context=app_context, **kwargs) self._is_started: bool = False self._lock: asyncio.Lock = asyncio.Lock() @@ -146,13 +176,6 @@ class BaseComponent(ABC): # ----- Vault path helpers -------------------------------------------- - @property - def vault_path(self) -> Path: - """Absolute vault root directory (cwd when no app_context is attached).""" - if self.app_context is None: - return Path.cwd() - return Path(self.app_context.app_config.vault_dir).absolute() - @property def vault_metadata_path(self) -> Path: """Vault metadata directory: ``/``.""" @@ -165,14 +188,6 @@ class BaseComponent(ABC): """Per-component metadata directory under the vault.""" return self.vault_metadata_path / self.component_type.value - def to_vault_relative(self, path: str | Path) -> str: - """Convert `path` to a vault-relative string; return absolute path when outside.""" - abs_path = Path(path).absolute() - try: - return str(abs_path.relative_to(self.vault_path)) - except ValueError: - return str(abs_path) - # ----- Lifecycle hooks (override in subclasses) ---------------------- async def _start(self) -> None: diff --git a/reme4/components/client/mcp_client.py b/reme4/components/client/mcp_client.py index 6cb390cb..0d6b5571 100644 --- a/reme4/components/client/mcp_client.py +++ b/reme4/components/client/mcp_client.py @@ -80,8 +80,8 @@ class MCPClient(BaseClient): cls = _TRANSPORT_MAP[self.transport] if self.transport == "stdio": - command = self.kwargs.pop("command", "") - args = self.kwargs.pop("args", []) + command = self.kwargs.get("command", "") + args = self.kwargs.get("args", []) return cls(command=command, args=args) path = "/sse" if self.transport == "sse" else "/mcp" diff --git a/reme4/components/file_graph/local_file_graph.py b/reme4/components/file_graph/local_file_graph.py index 1a67c8a6..6726de1e 100644 --- a/reme4/components/file_graph/local_file_graph.py +++ b/reme4/components/file_graph/local_file_graph.py @@ -17,12 +17,12 @@ class LocalFileGraph(BaseFileGraph): self._nodes: dict[str, FileNode] = {} self._inverse: dict[str, set[str]] = {} # real target → sources self._pending: dict[str, set[str]] = {} # virtual target → sources - self.component_metadata_path.mkdir(parents=True, exist_ok=True) self._graph_file: Path = self.component_metadata_path / f"{self.name}.jsonl" # -- Lifecycle --------------------------------------------------------- async def _start(self) -> None: + self.component_metadata_path.mkdir(parents=True, exist_ok=True) await super()._start() # base calls load() await self.rebuild_links() diff --git a/reme4/components/file_parser/chunked_file_parser.py b/reme4/components/file_parser/chunked_file_parser.py index 2d892157..1725d3e0 100644 --- a/reme4/components/file_parser/chunked_file_parser.py +++ b/reme4/components/file_parser/chunked_file_parser.py @@ -1,6 +1,5 @@ """File parser with byte-based overlapping chunking.""" -import re from bisect import bisect_right from pathlib import Path @@ -9,21 +8,8 @@ import yaml from .base_file_parser import BaseFileParser from ..component_registry import R -from ...schema import FileChunk, FileFrontMatter, FileLink, FileNode - -# Single-pass wikilink + optional dataview predicate. -# Covers: [[X]] / ![[X]] / [[X#h]] / [[X|alias]] / pred:: [[X]] / [pred:: [[X]]] -# - predicate group: optional leading '[' (dataview inline-bracket form), an identifier, -# then '::' — the whole prefix is non-capturing-optional so bare wikilinks still match. -# - optional '!' prefix matches the embed form (![[X]]). -# - target / anchor / alias all forbid '\n' so a wikilink cannot span lines. -# - alias '|...': consumed but not captured (we don't need display text). -_LINK_RE = re.compile( - r"(?:\[?\s*(?P[A-Za-z][\w-]*)\s*::\s*)?" - r"!?\[\[\s*(?P[^\[\]|#\n]+?)" - r"(?:#(?P[^\[\]|\n]+?))?" - r"\s*(?:\|[^\[\]\n]*?)?\s*]]", -) +from ...schema import FileChunk, FileFrontMatter, FileNode +from ...utils.wikilink_handler import WikilinkHandler @R.register("chunked") @@ -36,25 +22,6 @@ class ChunkedFileParser(BaseFileParser): self.chunk_byte_size = max(100, chunk_byte_size) self.overlap_byte_size = max(4, overlap_byte_size) - @staticmethod - def parse_links(content: str, source_path: str) -> list[FileLink]: - """Extract wikilinks with optional dataview predicate as outgoing FileLinks.""" - links: list[FileLink] = [] - for m in _LINK_RE.finditer(content): - target = m["target"].strip() - if not target: - continue - anchor = m["anchor"] - links.append( - FileLink( - source_path=source_path, - target_path=target, - target_anchor=anchor.strip() if anchor else None, - predicate=m["predicate"], - ), - ) - return links - @staticmethod def _parse_front_matter(text: str) -> tuple[FileFrontMatter, str]: """Parse YAML front matter delimited by ---, return (front_matter, remaining).""" @@ -86,7 +53,7 @@ class ChunkedFileParser(BaseFileParser): front_matter, content = self._parse_front_matter(text) if not content: return FileNode(path=rel_path, st_mtime=stat.st_mtime, front_matter=front_matter), [] - links = self.parse_links(content, rel_path) + links = WikilinkHandler.extract_links(content, rel_path) else: front_matter = FileFrontMatter() content = text @@ -109,12 +76,12 @@ class ChunkedFileParser(BaseFileParser): """Return [start, end) byte spans of every wikilink in content.""" spans: list[tuple[int, int]] = [] last_char, last_byte = 0, 0 - for m in _LINK_RE.finditer(content): - last_byte += len(content[last_char : m.start()].encode(self.encoding)) - match_bytes = len(m.group(0).encode(self.encoding)) + for wm in WikilinkHandler.iter_matches(content): + last_byte += len(content[last_char : wm.start].encode(self.encoding)) + match_bytes = len(content[wm.start : wm.end].encode(self.encoding)) spans.append((last_byte, last_byte + match_bytes)) last_byte += match_bytes - last_char = m.end() + last_char = wm.end return spans @staticmethod diff --git a/reme4/components/file_store/local_file_store.py b/reme4/components/file_store/local_file_store.py index 173ca97c..261dedf0 100644 --- a/reme4/components/file_store/local_file_store.py +++ b/reme4/components/file_store/local_file_store.py @@ -48,13 +48,13 @@ class LocalFileStore(BaseFileStore): self.encoding = encoding self.store_version = store_version - self.component_metadata_path.mkdir(parents=True, exist_ok=True) self.file_chunks: dict[str, FileChunk] = {} self.chunks_path = self.component_metadata_path / f"file_chunks_{self.name}_{self.store_version}.jsonl" # -- lifecycle ------------------------------------------------------------ async def _start(self) -> None: + self.component_metadata_path.mkdir(parents=True, exist_ok=True) await super()._start() if self.embedding_model is not None and not await self.embedding_model.health_check(): self.logger.warning(f"{self.name}: embedding unhealthy, vector disabled") diff --git a/reme4/components/keyword_index/base_keyword_index.py b/reme4/components/keyword_index/base_keyword_index.py index 466f4c66..bf3f3dc2 100644 --- a/reme4/components/keyword_index/base_keyword_index.py +++ b/reme4/components/keyword_index/base_keyword_index.py @@ -17,9 +17,9 @@ class BaseKeywordIndex(BaseComponent): from ..tokenizer import RegexTokenizer self.tokenizer = self.bind(tokenizer, BaseTokenizer, default_factory=RegexTokenizer) - self.component_metadata_path.mkdir(parents=True, exist_ok=True) async def _start(self) -> None: + self.component_metadata_path.mkdir(parents=True, exist_ok=True) await self.load() async def _close(self) -> None: diff --git a/reme4/reme.py b/reme4/reme.py index c8ad95a0..19598a90 100644 --- a/reme4/reme.py +++ b/reme4/reme.py @@ -18,6 +18,8 @@ async def call_server(action: str, **kwargs): """Call the appropriate server component.""" backend: str = kwargs.pop("backend", "http") client_cls = R.get(ComponentEnum.CLIENT, backend) + if client_cls is None: + raise ValueError(f"Unknown client backend: {backend!r}") async with client_cls() as client: async for chunk in client(action=action, **kwargs): print(chunk, end="", flush=True) diff --git a/reme4/steps/base_step.py b/reme4/steps/base_step.py index f5e1e868..65526cc4 100644 --- a/reme4/steps/base_step.py +++ b/reme4/steps/base_step.py @@ -11,6 +11,7 @@ from agentscope.model import ChatModelBase from agentscope.token import TokenCounterBase from agentscope.tool import Toolkit, ToolResponse +from ..components.base_component import ComponentMixin from ..components.embedding import BaseEmbeddingModel from ..components.file_parser import BaseFileParser from ..components.file_store import BaseFileStore @@ -18,7 +19,6 @@ from ..components.prompt_handler import PromptHandler from ..components.runtime_context import RuntimeContext from ..enumeration import ComponentEnum from ..schema import FileChunk, FileNode, Response -from ..utils import get_logger if TYPE_CHECKING: from ..components import ApplicationContext @@ -26,8 +26,81 @@ if TYPE_CHECKING: T = TypeVar("T") +_UNSET = object() -class BaseStep(ABC): + +class Ref: + """Descriptor that lazily resolves a component dependency for Steps. + + Replaces the ``@property`` + ``_resolve()`` boilerplate with a single + class-level declaration:: + + as_llm = Ref(ChatModelBase, ComponentEnum.AS_LLM, "model") + file_store = Ref(BaseFileStore, ComponentEnum.FILE_STORE) + + Resolution follows a 3-source fallback identical to the old ``_resolve``: + ``kwargs`` -> ``context`` -> ``app_context`` component registry. + The resolved value is cached on the instance for its lifetime + (steps are rebuilt per job call via ``_build_steps``). + """ + + __slots__ = ("base_cls", "comp_enum", "attr", "optional", "key", "_cache_attr") + + def __init__( + self, + base_cls: type, + comp_enum: ComponentEnum, + attr: str | None = None, + *, + optional: bool = False, + ) -> None: + self.base_cls = base_cls + self.comp_enum = comp_enum + self.attr = attr + self.optional = optional + self.key: str = "" + self._cache_attr: str = "" + + def __set_name__(self, owner: type, name: str) -> None: + self.key = name + self._cache_attr = f"_ref_{name}" + + def __get__(self, obj: "BaseStep | None", objtype: type | None = None): + if obj is None: + return self + cached = obj.__dict__.get(self._cache_attr, _UNSET) + if cached is not _UNSET: + return cached + value = self._resolve(obj) + obj.__dict__[self._cache_attr] = value + return value + + def __set__(self, obj: "BaseStep", value) -> None: + obj.__dict__[self._cache_attr] = value + + def __delete__(self, obj: "BaseStep") -> None: + obj.__dict__.pop(self._cache_attr, None) + + def _resolve(self, obj: "BaseStep"): + for source in (obj.kwargs, obj.context or {}): + value = source.get(self.key) + if isinstance(value, self.base_cls): + return value + + name = obj.kwargs.get(self.key, "default") + if obj.app_context is None: + if self.optional: + return None + raise RuntimeError(f"app_context is not set when resolving '{self.key}'") + comp = obj.app_context.components[self.comp_enum].get(name) + if comp is None: + if self.optional: + return None + raise KeyError(f"Component '{name}' not found in {self.comp_enum.value}") + return getattr(comp, self.attr) if self.attr else comp + + +class BaseStep(ComponentMixin, ABC): """Composable unit of an LLM workflow.""" component_type = ComponentEnum.STEP @@ -50,30 +123,32 @@ class BaseStep(ABC): output_mapping: dict[str, str] | None = None, **kwargs, ): - super().__init__() - self.name: str = name or self.__class__.__name__ - self.backend: str = backend - self.app_context: "ApplicationContext | None" = app_context + super().__init__(name=name, backend=backend, app_context=app_context, **kwargs) self.language: str = language self.input_mapping = input_mapping self.output_mapping = output_mapping - self.kwargs: dict = kwargs self.context: RuntimeContext | None = None - self.logger = get_logger() - if hasattr(self.logger, "bind"): - self.logger = self.logger.bind(component=self.name) - # Load class-level prompts first, then overlay caller-provided overrides. self.prompt = PromptHandler(language=self.language) self.prompt.load_prompt_by_class(self.__class__).load_prompt_dict(prompt_dict) + # ----- Component references (resolved lazily on first access) ---------- + + as_llm: ChatModelBase = Ref(ChatModelBase, ComponentEnum.AS_LLM, "model") + as_llm_formatter: FormatterBase = Ref(FormatterBase, ComponentEnum.AS_LLM_FORMATTER, "formatter") + as_token_counter: TokenCounterBase = Ref(TokenCounterBase, ComponentEnum.AS_TOKEN_COUNTER, "token_counter") + file_store: BaseFileStore = Ref(BaseFileStore, ComponentEnum.FILE_STORE) + embedding: BaseEmbeddingModel = Ref(BaseEmbeddingModel, ComponentEnum.EMBEDDING_MODEL) + @abstractmethod async def execute(self): """Run the step's logic against ``self.context``.""" async def __call__(self, context: RuntimeContext | None = None, **kwargs): - # Build runtime context, then apply key remapping around execute(). + # Clear cached Ref values so context-supplied overrides take effect. + for key in [k for k in self.__dict__ if k.startswith("_ref_")]: + del self.__dict__[key] self.context = RuntimeContext.from_context(context, **kwargs) assert self.context is not None if self.input_mapping: @@ -83,57 +158,6 @@ class BaseStep(ABC): self.context.apply_mapping(self.output_mapping) return result - @property - def vault_path(self) -> Path: - """Resolved vault root path from app context or cwd.""" - if self.app_context is None: - return Path.cwd() - return Path(self.app_context.app_config.vault_dir).absolute() - - def _resolve( - self, - key: str, - base_cls: type[T], - comp_enum: ComponentEnum, - attr: str | None = None, - ) -> T: - """Return a kwargs-supplied instance, or look one up by name in the app registry.""" - # 1. Step init kwargs, 2. Runtime context (run_job kwargs), 3. App registry by name. - for source in (self.kwargs, self.context or {}): - value = source.get(key) - if isinstance(value, base_cls): - return value - - name = self.kwargs.get(key, "default") - assert self.app_context is not None - comp = self.app_context.components[comp_enum][name] - return getattr(comp, attr) if attr else comp - - @property - def as_llm(self) -> ChatModelBase: - """Return the chat model component.""" - return self._resolve("as_llm", ChatModelBase, ComponentEnum.AS_LLM, "model") - - @property - def as_llm_formatter(self) -> FormatterBase: - """Return the LLM formatter component.""" - return self._resolve("as_llm_formatter", FormatterBase, ComponentEnum.AS_LLM_FORMATTER, "formatter") - - @property - def as_token_counter(self) -> TokenCounterBase: - """Return the token counter component.""" - return self._resolve("as_token_counter", TokenCounterBase, ComponentEnum.AS_TOKEN_COUNTER, "token_counter") - - @property - def file_store(self) -> BaseFileStore: - """Return the file store component.""" - return self._resolve("file_store", BaseFileStore, ComponentEnum.FILE_STORE) - - @property - def embedding(self) -> BaseEmbeddingModel: - """Return the embedding model component.""" - return self._resolve("embedding", BaseEmbeddingModel, ComponentEnum.EMBEDDING_MODEL) - async def parse_file(self, path: str | Path) -> tuple[FileNode, list[FileChunk]]: """Parse ``path`` with the parser whose ``supported_extensions`` claims its suffix. @@ -141,7 +165,8 @@ class BaseStep(ABC): ``default`` parser (stat-only) when no parser claims the suffix — that's how attachments / binaries / unknown types still produce a FileNode. """ - assert self.app_context is not None + if self.app_context is None: + raise RuntimeError("app_context is not set when resolving file parser") file_parser_dict: dict[str, BaseFileParser] = self.app_context.components[ComponentEnum.FILE_PARSER] suffix = Path(path).suffix.lstrip(".").lower() diff --git a/reme4/steps/file_io/_daily_index.py b/reme4/steps/file_io/_daily_index.py new file mode 100644 index 00000000..1f7e3e45 --- /dev/null +++ b/reme4/steps/file_io/_daily_index.py @@ -0,0 +1,118 @@ +"""Daily-note helpers: session_id validation + day-index rebuild.""" + +from pathlib import Path + +import frontmatter + +from ._path import validate_filename_component +from ...utils import get_logger + +logger = get_logger() + + +def validate_session_id(session_id: str) -> str | None: + """Validate a daily-note session_id. Thin wrapper over :func:`validate_filename_component`.""" + return validate_filename_component(session_id, kind="session_id") + + +_NOTES_OPEN = "" +_NOTES_CLOSE = "" + + +def _render_notes_block(notes: list[dict]) -> str: + """Render each note as ``- [[path]] key: val ...`` (one line per note).""" + if not notes: + return "(none)" + lines: list[str] = [] + for note in notes: + meta: dict = note["metadata"] + keys = [k for k in ("name", "description") if k in meta] + [k for k in meta if k not in ("name", "description")] + parts = [f"- [[{note['path']}]]"] + [ + f"{k}: {str(v).replace(chr(10), ' ')}" for k in keys if (v := meta[k]) not in (None, "") + ] + lines.append(" ".join(parts)) + return "\n".join(lines) + + +def _rebuild_body(body: str, notes_content: str) -> str: + """Replace or append the auto block, preserving surrounding content.""" + block = f"{_NOTES_OPEN}\n{notes_content}\n{_NOTES_CLOSE}" + if _NOTES_OPEN in body and _NOTES_CLOSE in body: + return body.split(_NOTES_OPEN, 1)[0] + block + body.split(_NOTES_CLOSE, 1)[1] + return f"{body.rstrip()}\n\n{block}\n" if body.strip() else f"{block}\n" + + +def scan_notes(vault_dir: Path, date: str, daily_dir: str) -> list[dict]: + """Walk ``//*.md`` and pull each note's frontmatter. + + Returns one dict per note:: + + {"session_id": str, "path": str, "metadata": dict} + """ + date_dir = vault_dir / daily_dir / date + if not date_dir.is_dir(): + return [] + out: list[dict] = [] + for md_path in sorted(p for p in date_dir.iterdir() if p.is_file() and p.suffix == ".md"): + session_id = md_path.stem + try: + post = frontmatter.loads(md_path.read_text(encoding="utf-8")) + except Exception: + continue + out.append( + { + "session_id": session_id, + "path": f"{daily_dir}/{date}/{session_id}.md", + "metadata": dict(post.metadata or {}), + }, + ) + return out + + +async def refresh_day_index(file_store, date: str, daily_dir: str) -> dict: + """Rebuild ``/.md`` from the current state of its notes. + + Returns ``{date, path, notes, created}``. + """ + vault_dir = Path(file_store.vault_path or ".").resolve() + index_rel = f"{daily_dir}/{date}.md" + index_abs = vault_dir / index_rel + notes = scan_notes(vault_dir, date, daily_dir) + + notes_payload = [{"path": n["path"], "session_id": n["session_id"], "metadata": n["metadata"]} for n in notes] + + if not notes and not index_abs.is_file(): + return { + "date": date, + "path": index_rel, + "notes": notes_payload, + "created": False, + } + + notes_block = _render_notes_block(notes) + + n = len(notes) + fm = {"name": date, "description": "No notes today." if n == 0 else f"{n} note(s) today."} + + if index_abs.is_file(): + post = frontmatter.loads(index_abs.read_text(encoding="utf-8")) + new_body = _rebuild_body(post.content, notes_block) + merged = dict(post.metadata or {}) + for key, value in fm.items(): + if not merged.get(key): + merged[key] = value + fm = merged + was_created = False + else: + index_abs.parent.mkdir(parents=True, exist_ok=True) + new_body = f"{_NOTES_OPEN}\n{notes_block}\n{_NOTES_CLOSE}\n" + was_created = True + out = frontmatter.Post(new_body, **fm) + index_abs.write_text(frontmatter.dumps(out), encoding="utf-8") + + return { + "date": date, + "path": index_rel, + "notes": notes_payload, + "created": was_created, + } diff --git a/reme4/steps/file_io/_file_io.py b/reme4/steps/file_io/_file_io.py index 46c91be5..fb53773c 100644 --- a/reme4/steps/file_io/_file_io.py +++ b/reme4/steps/file_io/_file_io.py @@ -1,23 +1,11 @@ -"""Shared filesystem helpers for CRUD steps. - -Two related concerns, both private to the ``crud`` package: - -1. **Generic file IO** — path gating, encoding-aware read/write, output - truncation (used by every CRUD step that touches the filesystem). -2. **Daily-note helpers** — session_id validation + ``daily/.md`` - index rebuild (used by the ``daily_*`` steps). The day index is a - derived rollup page auto-managed in marker-delimited sections; - user-edited manual sections are preserved verbatim across refreshes. -""" +"""Encoding-aware file IO, output truncation, and per-path write locks.""" import asyncio -import re from pathlib import Path from typing import Iterable import aiofiles import aiofiles.os -import frontmatter from ...constants import DEFAULT_MAX_BYTES, MAX_FILE_READ_BYTES, TRUNCATION_NOTICE_MARKER from ...utils import get_logger @@ -25,33 +13,32 @@ from ...utils import get_logger logger = get_logger() # --------------------------------------------------------------------------- -# Generic file IO +# In-process per-path write lock. +# --------------------------------------------------------------------------- +_PATH_LOCKS_MAX = 1024 +_PATH_LOCKS: dict[str, asyncio.Lock] = {} +_PATH_LOCKS_REGISTRY = asyncio.Lock() + + +async def get_path_lock(target: Path) -> asyncio.Lock: + """Return the asyncio.Lock for ``target``; created lazily on first request.""" + key = str(target) + async with _PATH_LOCKS_REGISTRY: + lock = _PATH_LOCKS.get(key) + if lock is None: + if len(_PATH_LOCKS) >= _PATH_LOCKS_MAX: + to_remove = [k for k, v in _PATH_LOCKS.items() if not v.locked()] + for k in to_remove[: len(_PATH_LOCKS) // 2]: + del _PATH_LOCKS[k] + lock = asyncio.Lock() + _PATH_LOCKS[key] = lock + return lock + + +# --------------------------------------------------------------------------- +# Encoding detection # --------------------------------------------------------------------------- -NON_MD_WARNING = ( - "non-markdown file detected; CRUD operations are recommended on markdown files. " - "Operating in compatibility mode may carry risks of errors." -) - -NON_IMAGE_WARNING = ( - "non-image file detected; CRUD image operations are recommended on standard image formats. " - "Operating in compatibility mode may carry risks of errors." -) - -# Image suffix → MIME mapping. SVG intentionally excluded (text format, base64 -# encoding has no benefit — caller should use read_step instead). -IMAGE_MIME_BY_EXT: dict[str, str] = { - ".png": "image/png", - ".jpg": "image/jpeg", - ".jpeg": "image/jpeg", - ".webp": "image/webp", - ".gif": "image/gif", - ".bmp": "image/bmp", - ".tiff": "image/tiff", - ".heic": "image/heic", -} - -# Modern text formats — assume UTF-8 by convention. _STANDARD_TEXT_EXTS = { ".md", ".py", @@ -69,159 +56,9 @@ _STANDARD_TEXT_EXTS = { ".txt", ".sh", } -# Legacy formats that may use ANSI/GBK on Chinese Windows systems. _NON_STANDARD_EXTS = {".csv", ".bat", ".cmd", ".reg"} -# --------------------------------------------------------------------------- -# In-process per-path write lock. -# -# Concurrent CRUD writes (write / edit) targeting the same path from the -# same process must be serialized so a read-modify-write cycle isn't -# interleaved by another coroutine. Different paths get different locks, -# so unrelated writes still run in parallel. -# -# NOTE: this is in-process only — multi-worker / multi-process deployments -# are NOT protected. That trade-off is acceptable for the current single- -# process reme server; cross-process protection would need flock or OCC. -# --------------------------------------------------------------------------- -_PATH_LOCKS: dict[str, asyncio.Lock] = {} -_PATH_LOCKS_REGISTRY = asyncio.Lock() - - -async def get_path_lock(target: Path) -> asyncio.Lock: - """Return the asyncio.Lock for ``target``; created lazily on first request. - - The lock is keyed by the string form of ``target`` — callers should pass - a path that has already been normalized by :func:`resolve_path` so two - equivalent paths share one lock. - """ - key = str(target) - async with _PATH_LOCKS_REGISTRY: - lock = _PATH_LOCKS.get(key) - if lock is None: - lock = asyncio.Lock() - _PATH_LOCKS[key] = lock - return lock - - -# Path helpers -# ------------ - -# Filename validation. Windows is the strictest mainstream filesystem, so we -# validate to its bar — paths that pass here also work on macOS and Linux, -# and survive sync to a Windows machine or zip-and-share workflows. - -_INVALID_CHARS = re.compile(r'[<>:"/\\|?*\x00-\x1f]') -_RESERVED_NAMES = { - "CON", - "PRN", - "AUX", - "NUL", - *(f"COM{i}" for i in range(1, 10)), - *(f"LPT{i}" for i in range(1, 10)), -} - - -def validate_filename_component(name: str, *, kind: str = "filename") -> str | None: - """Return an error message, or ``None`` when ``name`` is a safe filename component. - - A *component* is a single path segment — no ``/`` or ``\\`` allowed inside. - Used for both daily-note slugs and ``resolve_path`` per-component checks. - - Rules: - - - non-empty, no leading / trailing whitespace - - no reserved characters: ``< > : " / \\ | ? *`` or control chars (``\\x00-\\x1f``) - - no reserved device names: ``CON`` / ``PRN`` / ``AUX`` / ``NUL`` / - ``COM1-9`` / ``LPT1-9`` (Windows reserves these with or without an - extension — ``CON.txt`` is also forbidden) - - no trailing ``.`` (also rejects ``..``, which doubles as path-traversal protection - for callers that validate per component) - - ``kind`` is the human-readable label inserted into error messages - (e.g. ``"session_id"``, ``"path component"``). - """ - if not name: - return f"{kind} is required" - if name != name.strip(): - return f"{kind} cannot have leading or trailing whitespace: {name!r}" - if _INVALID_CHARS.search(name): - return f'{kind} contains invalid characters (one of < > : " / \\ | ? * or a control char): {name!r}' - if name.endswith("."): - return f"{kind} cannot end with '.': {name!r}" - if name.split(".", 1)[0].upper() in _RESERVED_NAMES: - return f"{kind} is a Windows-reserved device name: {name!r}" - return None - - -def resolve_path(vault_path: Path, raw: str) -> tuple[Path | None, str | None]: - """Resolve a `path=` argument against ``vault_path``. - - Rules: - - Relative paths are joined under ``vault_path``. - - Absolute paths are accepted and returned as-is; a warning is logged - recommending relative paths, but the read still proceeds. - - Each path component is validated against the same Windows-strict - filename rules used for daily-note slugs — see - :func:`validate_filename_component`. ``..`` is rejected by the - trailing-``.`` rule, which doubles as path-traversal protection. - Returns ``(abs_path, None)`` on success, or ``(None, error_message)`` on failure. - Filetype-specific gating (e.g. markdown-only / suffix auto-append) is - layered on top by callers — see ``reme/steps/file_io/_file_io.py::gate_md``. - """ - if not raw or not str(raw).strip(): - return None, "`path` is required" - s = str(raw).strip() - p = Path(s) - for part in p.parts: - if part == p.anchor: - continue - err = validate_filename_component(part, kind="path component") - if err: - return None, err - if p.is_absolute(): - logger.info("absolute path detected, recommending relative paths") - return p, None - return vault_path / p, None - - -def gate_md(target: Path) -> tuple[Path, bool]: - """Markdown gate with compatibility fallback. - - Returns ``(path, is_md)``: - - No suffix → auto-append `.md`, ``is_md=True``. - - `.md` suffix → ``is_md=True``. - - Any other suffix → ``is_md=False`` (caller handles degraded mode). - """ - if target.suffix == "": - return target.with_suffix(".md"), True - if target.suffix.lower() != ".md": - return target, False - return target, True - - -def gate_image(target: Path) -> tuple[Path, bool, str | None]: - """Image gate with compatibility fallback. Returns ``(path, is_image, mime)``. - - Behavior diverges from :func:`gate_md` in two ways: - - **No suffix is NOT auto-appended.** Image formats have no single - reasonable default; guessing would mislead. - - **No path mutation** — ``target`` is returned unchanged. - - Suffix routing: - - Known image suffix (see ``IMAGE_MIME_BY_EXT``) → ``(target, True, mime)`` - - Empty suffix or unknown suffix → ``(target, False, None)`` - Caller may still read the file and surface a ``NON_IMAGE_WARNING``. - """ - mime = IMAGE_MIME_BY_EXT.get(target.suffix.lower()) - return target, mime is not None, mime - - -# Encoding detection (private) -# ---------------------------- - - def _try_decode(data: bytes, encodings: Iterable[str]) -> tuple[str, str] | None: """Return ``(text, encoding)`` for the first encoding that decodes ``data`` cleanly.""" for enc in encodings: @@ -233,13 +70,7 @@ def _try_decode(data: bytes, encodings: Iterable[str]) -> tuple[str, str] | None def _decode_known_file(data: bytes, file_extension: str) -> tuple[str, str]: - """Decode file bytes using the extension as a hint. Returns ``(text, encoding)``. - - Strategy: - 1. BOM-based detection. - 2. Extension-driven defaults: - 3. Last resort → UTF-8 with ``errors='replace'`` so the function never raises. - """ + """Decode file bytes using the extension as a hint. Returns ``(text, encoding)``.""" if data.startswith(b"\xef\xbb\xbf"): return data.decode("utf-8-sig"), "utf-8-sig" if data.startswith((b"\xff\xfe", b"\xfe\xff")): @@ -254,7 +85,7 @@ def _decode_known_file(data: bytes, file_extension: str) -> tuple[str, str]: try: return data.decode("utf-8-sig"), "utf-8" except UnicodeDecodeError: - pass # fall through + pass if ext in _NON_STANDARD_EXTS: result = _try_decode(data, ("utf-8-sig", "gbk")) @@ -262,22 +93,18 @@ def _decode_known_file(data: bytes, file_extension: str) -> tuple[str, str]: text, enc = result return text, "utf-8" if enc == "utf-8-sig" else enc - # Unknown extension or earlier strategies failed. - return data.decode("utf-8", errors="replace"), "utf-8" +# --------------------------------------------------------------------------- # File read / write -# ----------------- +# --------------------------------------------------------------------------- async def read_file_safe(file_path, max_bytes: int = MAX_FILE_READ_BYTES) -> tuple[str, str]: """Read file in byte mode and decode using extension-aware strategy. - Returns ``(text, encoding)``. Callers that need to write the file back - in its original encoding can pass ``encoding`` straight to - :func:`write_file_safe`, avoiding a second read via - :func:`detect_file_encoding`. + Returns ``(text, encoding)``. """ stat = await aiofiles.os.stat(str(file_path)) read_size = min(stat.st_size, max_bytes) @@ -287,32 +114,18 @@ async def read_file_safe(file_path, max_bytes: int = MAX_FILE_READ_BYTES) -> tup async def detect_file_encoding(file_path, sniff_bytes: int = 8192) -> str: - """Detect the encoding of an existing file so writes can preserve it. - - Reads up to ``sniff_bytes`` from the head of the file (enough for BOM - detection and statistical analysis). Falls back to ``utf-8`` if the file - is unreadable. - """ + """Detect the encoding of an existing file so writes can preserve it.""" try: async with aiofiles.open(str(file_path), "rb") as f: data = await f.read(sniff_bytes) - except Exception: # pylint: disable=broad-except + except Exception: return "utf-8" _, enc = _decode_known_file(data, Path(file_path).suffix) return enc async def write_file_safe(file_path: Path, content: str | bytes, encoding: str = "utf-8") -> None: - """Write ``content`` to ``file_path`` in binary mode; creates parent dirs. - - ``str`` input is encoded with ``encoding`` (default UTF-8); callers wanting - to preserve a file's original encoding should pass the result of - :func:`detect_file_encoding`. If the requested ``encoding`` can't represent - some characters, falls back to UTF-8 to avoid data loss. - - ``bytes`` input is written verbatim — callers managing their own encoding - can pass raw bytes directly. - """ + """Write ``content`` to ``file_path`` in binary mode; creates parent dirs.""" file_path.parent.mkdir(parents=True, exist_ok=True) if isinstance(content, str): try: @@ -329,8 +142,9 @@ async def write_file_safe(file_path: Path, content: str | bytes, encoding: str = await f.write(payload) -# Output formatting -# ----------------- +# --------------------------------------------------------------------------- +# Output truncation +# --------------------------------------------------------------------------- def truncate_text_output( @@ -342,12 +156,7 @@ def truncate_text_output( file_path: str | None = None, encoding: str = "utf-8", ) -> str: - """Truncate text by bytes preserving line integrity; append a continuation notice. - - See qwenpaw `tools/utils.py` for the same semantics. Returns text unchanged when - it fits within max_bytes, when max_bytes <= 0, or when the last line itself - exceeds max_bytes (unhandled edge case). - """ + """Truncate text by bytes preserving line integrity; append a continuation notice.""" if not text or max_bytes <= 0: return text @@ -379,153 +188,3 @@ def truncate_text_output( except Exception: logger.warning("truncate_text_output failed, returning original text", exc_info=True) return text - - -# --------------------------------------------------------------------------- -# Daily-note helpers: session_id validation + day-index rebuild -# --------------------------------------------------------------------------- - -# Session ID validation -# --------------------- - - -def validate_session_id(session_id: str) -> str | None: - """Validate a daily-note session_id. Thin wrapper over :func:`validate_filename_component`.""" - return validate_filename_component(session_id, kind="session_id") - - -# Day-index rebuild -# ----------------- -# The day index is a derived artifact whose single job is daily-note -# consolidation — its source of truth lives in each note's -# frontmatter. The rebuild refreshes the auto-managed notes block -# while preserving any user content sitting outside the markers. -# -# Frontmatter shape — only the two reserved fields: -# name: -# description: -# -# The note inventory lives in the body's ```` -# block: each note becomes a single line with its full frontmatter -# inlined (``- [[path]] name: ... description: ... ``), -# letting an agent scan the day at a glance. Content outside the -# auto markers is preserved verbatim across refreshes. - -_NOTES_OPEN = "" -_NOTES_CLOSE = "" - - -def _render_notes_block(notes: list[dict]) -> str: - """Render each note as ``- [[path]] key: val ...`` (one line per note).""" - if not notes: - return "(none)" - lines: list[str] = [] - for note in notes: - meta: dict = note["metadata"] - keys = [k for k in ("name", "description") if k in meta] + [k for k in meta if k not in ("name", "description")] - parts = [f"- [[{note['path']}]]"] + [ - f"{k}: {str(v).replace(chr(10), ' ')}" for k in keys if (v := meta[k]) not in (None, "") - ] - lines.append(" ".join(parts)) - return "\n".join(lines) - - -def _rebuild_body(body: str, notes_content: str) -> str: - """Replace or append the auto block, preserving surrounding content.""" - block = f"{_NOTES_OPEN}\n{notes_content}\n{_NOTES_CLOSE}" - if _NOTES_OPEN in body and _NOTES_CLOSE in body: - return body.split(_NOTES_OPEN, 1)[0] + block + body.split(_NOTES_CLOSE, 1)[1] - return f"{body.rstrip()}\n\n{block}\n" if body.strip() else f"{block}\n" - - -# Public scan + rebuild -# --------------------- - - -def scan_notes(vault_dir: Path, date: str, daily_dir: str) -> list[dict]: - """Walk ``//*.md`` and pull each note's frontmatter. - - Returns one dict per note:: - - {"session_id": str, "path": str, "metadata": dict} - - ``metadata`` is the raw frontmatter dict (insertion-ordered); - consumers decide which keys to surface. Each ``.md`` directly - under the day folder is a note; the file's stem is the session_id. - """ - date_dir = vault_dir / daily_dir / date - if not date_dir.is_dir(): - return [] - out: list[dict] = [] - for md_path in sorted(p for p in date_dir.iterdir() if p.is_file() and p.suffix == ".md"): - session_id = md_path.stem - try: - post = frontmatter.loads(md_path.read_text(encoding="utf-8")) - except Exception: # pylint: disable=broad-except - continue - out.append( - { - "session_id": session_id, - "path": f"{daily_dir}/{date}/{session_id}.md", - "metadata": dict(post.metadata or {}), - }, - ) - return out - - -async def refresh_day_index(file_store, date: str, daily_dir: str) -> dict: - """Rebuild ``/.md`` from the current state of its notes. - - Behaviour: - * No ``//`` at all and no existing index file → no-op. - * Notes present → write the index file (create if missing, - otherwise refresh the auto block in place, preserving content - outside the markers, refresh frontmatter). - * Notes directory empty but index file exists → rebuild with - empty auto block (keeps the file in sync with reality). - - Returns ``{date, path, notes, created}``. Each row in ``notes`` is - ``{path, slug, metadata}`` with the raw frontmatter dict. - """ - vault_dir = Path(file_store.vault_path or ".").resolve() - index_rel = f"{daily_dir}/{date}.md" - index_abs = vault_dir / index_rel - notes = scan_notes(vault_dir, date, daily_dir) - - notes_payload = [{"path": n["path"], "session_id": n["session_id"], "metadata": n["metadata"]} for n in notes] - - if not notes and not index_abs.is_file(): - return { - "date": date, - "path": index_rel, - "notes": notes_payload, - "created": False, - } - - notes_block = _render_notes_block(notes) - - n = len(notes) - fm = {"name": date, "description": "No notes today." if n == 0 else f"{n} note(s) today."} - - if index_abs.is_file(): - post = frontmatter.loads(index_abs.read_text(encoding="utf-8")) - new_body = _rebuild_body(post.content, notes_block) - merged = dict(post.metadata or {}) - for key, value in fm.items(): - if not merged.get(key): - merged[key] = value - fm = merged - was_created = False - else: - index_abs.parent.mkdir(parents=True, exist_ok=True) - new_body = f"{_NOTES_OPEN}\n{notes_block}\n{_NOTES_CLOSE}\n" - was_created = True - out = frontmatter.Post(new_body, **fm) - index_abs.write_text(frontmatter.dumps(out), encoding="utf-8") - - return { - "date": date, - "path": index_rel, - "notes": notes_payload, - "created": was_created, - } diff --git a/reme4/steps/file_io/_path.py b/reme4/steps/file_io/_path.py new file mode 100644 index 00000000..259e63a4 --- /dev/null +++ b/reme4/steps/file_io/_path.py @@ -0,0 +1,96 @@ +"""Path resolution, validation, and filetype gating for CRUD steps.""" + +import re +from pathlib import Path + +from ...utils import get_logger + +logger = get_logger() + +NON_MD_WARNING = ( + "non-markdown file detected; CRUD operations are recommended on markdown files. " + "Operating in compatibility mode may carry risks of errors." +) + +NON_IMAGE_WARNING = ( + "non-image file detected; CRUD image operations are recommended on standard image formats. " + "Operating in compatibility mode may carry risks of errors." +) + +IMAGE_MIME_BY_EXT: dict[str, str] = { + ".png": "image/png", + ".jpg": "image/jpeg", + ".jpeg": "image/jpeg", + ".webp": "image/webp", + ".gif": "image/gif", + ".bmp": "image/bmp", + ".tiff": "image/tiff", + ".heic": "image/heic", +} + +_INVALID_CHARS = re.compile(r'[<>:"/\\|?*\x00-\x1f]') +_RESERVED_NAMES = { + "CON", + "PRN", + "AUX", + "NUL", + *(f"COM{i}" for i in range(1, 10)), + *(f"LPT{i}" for i in range(1, 10)), +} + + +def validate_filename_component(name: str, *, kind: str = "filename") -> str | None: + """Return an error message, or ``None`` when ``name`` is a safe filename component.""" + if not name: + return f"{kind} is required" + if name != name.strip(): + return f"{kind} cannot have leading or trailing whitespace: {name!r}" + if _INVALID_CHARS.search(name): + return f'{kind} contains invalid characters (one of < > : " / \\ | ? * or a control char): {name!r}' + if name.endswith("."): + return f"{kind} cannot end with '.': {name!r}" + if name.split(".", 1)[0].upper() in _RESERVED_NAMES: + return f"{kind} is a Windows-reserved device name: {name!r}" + return None + + +def resolve_path(vault_path: Path, raw: str) -> tuple[Path | None, str | None]: + """Resolve a `path=` argument against ``vault_path``. + + Returns ``(abs_path, None)`` on success, or ``(None, error_message)`` on failure. + """ + if not raw or not str(raw).strip(): + return None, "`path` is required" + s = str(raw).strip() + p = Path(s) + for part in p.parts: + if part == p.anchor: + continue + err = validate_filename_component(part, kind="path component") + if err: + return None, err + if p.is_absolute(): + logger.info("absolute path detected, recommending relative paths") + return p, None + return vault_path / p, None + + +def gate_md(target: Path) -> tuple[Path, bool]: + """Markdown gate with compatibility fallback. + + Returns ``(path, is_md)``: + - No suffix -> auto-append `.md`, ``is_md=True``. + - `.md` suffix -> ``is_md=True``. + - Any other suffix -> ``is_md=False`` (caller handles degraded mode). + """ + if target.suffix == "": + return target.with_suffix(".md"), True + if target.suffix.lower() != ".md": + return target, False + return target, True + + +def gate_image(target: Path) -> tuple[Path, bool, str | None]: + """Image gate with compatibility fallback. Returns ``(path, is_image, mime)``.""" + mime = IMAGE_MIME_BY_EXT.get(target.suffix.lower()) + return target, mime is not None, mime diff --git a/reme4/steps/file_io/daily_create.py b/reme4/steps/file_io/daily_create.py index bf429e9b..3e63cac5 100644 --- a/reme4/steps/file_io/daily_create.py +++ b/reme4/steps/file_io/daily_create.py @@ -29,7 +29,8 @@ from pathlib import Path import frontmatter -from ._file_io import refresh_day_index, validate_session_id, write_file_safe +from ._daily_index import refresh_day_index, validate_session_id +from ._file_io import write_file_safe from ..base_step import BaseStep from ...components import R diff --git a/reme4/steps/file_io/daily_list.py b/reme4/steps/file_io/daily_list.py index 084cc47b..9e9976de 100644 --- a/reme4/steps/file_io/daily_list.py +++ b/reme4/steps/file_io/daily_list.py @@ -15,7 +15,7 @@ to today. from datetime import date as _date from pathlib import Path -from ._file_io import scan_notes +from ._daily_index import scan_notes from ..base_step import BaseStep from ...components import R diff --git a/reme4/steps/file_io/daily_reindex.py b/reme4/steps/file_io/daily_reindex.py index d01b1e5b..8ef38953 100644 --- a/reme4/steps/file_io/daily_reindex.py +++ b/reme4/steps/file_io/daily_reindex.py @@ -21,7 +21,7 @@ Always idempotent and safe to re-run. from datetime import date as _date -from ._file_io import refresh_day_index +from ._daily_index import refresh_day_index from ..base_step import BaseStep from ...components import R diff --git a/reme4/steps/file_io/edit.py b/reme4/steps/file_io/edit.py index 34160e7d..8dc405b1 100644 --- a/reme4/steps/file_io/edit.py +++ b/reme4/steps/file_io/edit.py @@ -3,14 +3,8 @@ import frontmatter import yaml -from ._file_io import ( - NON_MD_WARNING, - gate_md, - get_path_lock, - read_file_safe, - resolve_path, - write_file_safe, -) +from ._file_io import get_path_lock, read_file_safe, write_file_safe +from ._path import NON_MD_WARNING, gate_md, resolve_path from ..base_step import BaseStep from ...components import R @@ -44,7 +38,7 @@ class EditStep(BaseStep): self._fail("`old` is required and must be non-empty") return None if new is None: - self.fail("`new` is required") + self._fail("`new` is required") return None old_str = str(old) new_str = str(new) diff --git a/reme4/steps/file_io/read.py b/reme4/steps/file_io/read.py index 1114e5f3..d153ad87 100644 --- a/reme4/steps/file_io/read.py +++ b/reme4/steps/file_io/read.py @@ -2,7 +2,8 @@ from pathlib import Path -from ._file_io import NON_MD_WARNING, gate_md, read_file_safe, resolve_path, truncate_text_output +from ._file_io import read_file_safe, truncate_text_output +from ._path import NON_MD_WARNING, gate_md, resolve_path from ..base_step import BaseStep from ...components import R from ...utils import expand_links, render_expansion_lines diff --git a/reme4/steps/file_io/read_image.py b/reme4/steps/file_io/read_image.py index d5a60f67..2227d416 100644 --- a/reme4/steps/file_io/read_image.py +++ b/reme4/steps/file_io/read_image.py @@ -5,7 +5,7 @@ import base64 import aiofiles import aiofiles.os -from ._file_io import NON_IMAGE_WARNING, gate_image, resolve_path +from ._path import NON_IMAGE_WARNING, gate_image, resolve_path from ..base_step import BaseStep from ...components import R from ...constants import DEFAULT_MAX_IMAGE_BYTES diff --git a/reme4/steps/file_io/write.py b/reme4/steps/file_io/write.py index 26aee115..005130a1 100644 --- a/reme4/steps/file_io/write.py +++ b/reme4/steps/file_io/write.py @@ -2,14 +2,8 @@ import frontmatter -from ._file_io import ( - NON_MD_WARNING, - detect_file_encoding, - gate_md, - get_path_lock, - resolve_path, - write_file_safe, -) +from ._file_io import detect_file_encoding, get_path_lock, write_file_safe +from ._path import NON_MD_WARNING, gate_md, resolve_path from ..base_step import BaseStep from ...components import R diff --git a/reme4/steps/index/update_catalog.py b/reme4/steps/index/update_catalog.py index c94d97ad..56936919 100644 --- a/reme4/steps/index/update_catalog.py +++ b/reme4/steps/index/update_catalog.py @@ -4,7 +4,7 @@ from pathlib import Path from watchfiles import Change -from ..base_step import BaseStep +from ..base_step import BaseStep, Ref from ...components import R from ...components.file_catalog import BaseFileCatalog from ...enumeration import ComponentEnum @@ -15,17 +15,7 @@ from ...schema import FileNode class UpdateCatalogStep(BaseStep): """Classify raw watcher changes and update the file_catalog.""" - @property - def file_catalog(self) -> BaseFileCatalog: - """Return the file catalog component.""" - return self._resolve("file_catalog", BaseFileCatalog, ComponentEnum.FILE_CATALOG) - - def _to_vault_relative(self, path: str | Path) -> str: - abs_path = Path(path).absolute() - try: - return str(abs_path.relative_to(self.vault_path)) - except ValueError: - return str(abs_path) + file_catalog: BaseFileCatalog = Ref(BaseFileCatalog, ComponentEnum.FILE_CATALOG) async def execute(self): assert self.context is not None @@ -58,7 +48,7 @@ class UpdateCatalogStep(BaseStep): self.logger.info(f"{action} file: {path}") try: stat = abs_path.stat() - nodes.append(FileNode(path=self._to_vault_relative(abs_path), st_mtime=stat.st_mtime)) + nodes.append(FileNode(path=self.to_vault_relative(abs_path), st_mtime=stat.st_mtime)) ok_paths.append(path) except Exception as e: self.logger.exception(f"Failed to stat {path}") @@ -78,7 +68,7 @@ class UpdateCatalogStep(BaseStep): if self.file_catalog is None: raise RuntimeError("file_catalog is not initialized!") self.logger.info(f"Detected {len(deleted)} deleted file(s)") - rel_deleted = [self._to_vault_relative(p) for p in deleted] + rel_deleted = [self.to_vault_relative(p) for p in deleted] try: await self.file_catalog.delete(rel_deleted) results.extend({"change": "deleted", "path": p, "success": True} for p in deleted) diff --git a/tests4/unit/test_chunked_file_parser.py b/tests4/unit/test_chunked_file_parser.py index ec3cc6a7..e9b488f9 100644 --- a/tests4/unit/test_chunked_file_parser.py +++ b/tests4/unit/test_chunked_file_parser.py @@ -5,6 +5,7 @@ import os import tempfile from reme4.components.file_parser import ChunkedFileParser +from reme4.utils.wikilink_handler import WikilinkHandler # Add parent path for import @@ -171,7 +172,7 @@ def test_file_chunk_properties(): def test_parse_links_bare(): """Bare wikilink: [[target]].""" - links = ChunkedFileParser.parse_links("see [[note]]", "src.md") + links = WikilinkHandler.extract_links("see [[note]]", "src.md") assert len(links) == 1 link = links[0] assert link.source_path == "src.md" @@ -183,7 +184,7 @@ def test_parse_links_bare(): def test_parse_links_with_anchor(): """Wikilink with anchor: [[target#anchor]].""" - links = ChunkedFileParser.parse_links("see [[note#section A]]", "src.md") + links = WikilinkHandler.extract_links("see [[note#section A]]", "src.md") assert len(links) == 1 assert links[0].target_path == "note" assert links[0].target_anchor == "section A" @@ -193,7 +194,7 @@ def test_parse_links_with_anchor(): def test_parse_links_alias_dropped(): """Alias after '|' is consumed but not captured as anchor.""" - links = ChunkedFileParser.parse_links("see [[note|display text]]", "src.md") + links = WikilinkHandler.extract_links("see [[note|display text]]", "src.md") assert len(links) == 1 assert links[0].target_path == "note" assert links[0].target_anchor is None @@ -202,7 +203,7 @@ def test_parse_links_alias_dropped(): def test_parse_links_anchor_and_alias(): """[[target#anchor|alias]] — anchor captured, alias dropped.""" - links = ChunkedFileParser.parse_links("see [[note#sec|disp]]", "src.md") + links = WikilinkHandler.extract_links("see [[note#sec|disp]]", "src.md") assert len(links) == 1 assert links[0].target_path == "note" assert links[0].target_anchor == "sec" @@ -211,7 +212,7 @@ def test_parse_links_anchor_and_alias(): def test_parse_links_predicate_simple(): """Dataview inline: predicate:: [[target]].""" - links = ChunkedFileParser.parse_links("author:: [[Alice]]", "src.md") + links = WikilinkHandler.extract_links("author:: [[Alice]]", "src.md") assert len(links) == 1 assert links[0].predicate == "author" assert links[0].target_path == "Alice" @@ -221,7 +222,7 @@ def test_parse_links_predicate_simple(): def test_parse_links_predicate_bracketed(): """Dataview inline-bracket: [predicate:: [[target]]].""" - links = ChunkedFileParser.parse_links("text [author:: [[Alice]]] more", "src.md") + links = WikilinkHandler.extract_links("text [author:: [[Alice]]] more", "src.md") assert len(links) == 1 assert links[0].predicate == "author" assert links[0].target_path == "Alice" @@ -230,7 +231,7 @@ def test_parse_links_predicate_bracketed(): def test_parse_links_predicate_bracketed_with_anchor(): """[predicate:: [[target_path#target_anchor]]] — combined form.""" - links = ChunkedFileParser.parse_links( + links = WikilinkHandler.extract_links( "[predicate:: [[target_path#target_anchor]]]", "src.md", ) @@ -244,17 +245,17 @@ def test_parse_links_predicate_bracketed_with_anchor(): def test_parse_links_predicate_sticks_to_first(): - """Predicate attaches only to the immediately following wikilink.""" - links = ChunkedFileParser.parse_links("pred:: [[a]] and bare [[b]]", "src.md") + """Line-level predicate covers all wikilinks in its value portion.""" + links = WikilinkHandler.extract_links("pred:: [[a]] and bare [[b]]", "src.md") assert len(links) == 2 assert links[0].predicate == "pred" and links[0].target_path == "a" - assert links[1].predicate is None and links[1].target_path == "b" + assert links[1].predicate == "pred" and links[1].target_path == "b" print("✓ test_parse_links_predicate_sticks_to_first passed") def test_parse_links_multiple_on_one_line(): """Multiple bare wikilinks on the same line are all captured.""" - links = ChunkedFileParser.parse_links("see [[x]] and [[y#h]]", "src.md") + links = WikilinkHandler.extract_links("see [[x]] and [[y#h]]", "src.md") assert [(link.target_path, link.target_anchor) for link in links] == [ ("x", None), ("y", "h"), @@ -264,19 +265,19 @@ def test_parse_links_multiple_on_one_line(): def test_parse_links_no_match(): """Strings without [[]] yield no links, even if '::' appears.""" - assert len(ChunkedFileParser.parse_links("no link here :: foo", "src.md")) == 0 - assert len(ChunkedFileParser.parse_links("plain text without brackets", "src.md")) == 0 - assert len(ChunkedFileParser.parse_links("", "src.md")) == 0 + assert len(WikilinkHandler.extract_links("no link here :: foo", "src.md")) == 0 + assert len(WikilinkHandler.extract_links("plain text without brackets", "src.md")) == 0 + assert len(WikilinkHandler.extract_links("", "src.md")) == 0 print("✓ test_parse_links_no_match passed") -def test_parse_links_predicate_with_dash_and_digits(): - """Predicate identifier accepts letters, digits, underscore, dash.""" - links = ChunkedFileParser.parse_links("see-also-2:: [[target]]", "src.md") +def test_parse_links_predicate_with_underscore_and_digits(): + """Predicate identifier accepts letters, digits, underscore (no dash per Dataview spec).""" + links = WikilinkHandler.extract_links("see_also2:: [[target]]", "src.md") assert len(links) == 1 - assert links[0].predicate == "see-also-2" + assert links[0].predicate == "see_also2" assert links[0].target_path == "target" - print("✓ test_parse_links_predicate_with_dash_and_digits passed") + print("✓ test_parse_links_predicate_with_underscore_and_digits passed") def test_parse_links_in_file(): @@ -465,7 +466,7 @@ if __name__ == "__main__": test_parse_links_predicate_sticks_to_first() test_parse_links_multiple_on_one_line() test_parse_links_no_match() - test_parse_links_predicate_with_dash_and_digits() + test_parse_links_predicate_with_underscore_and_digits() test_parse_links_in_file() test_parse_links_empty_when_no_content() test_chunk_does_not_split_wikilink_at_boundary()