Say why a name derived from non-ASCII key fields has none, and print it - #34
Open
dajiaohuang wants to merge 2 commits into
Open
dajiaohuang wants to merge 2 commits into
dajiaohuang wants to merge 2 commits into
Conversation
An abstract written entirely in CJK, Arabic or Cyrillic slugifies to the empty string, and the store rejected the write with "key fields produce an empty name" -- which names neither the cause nor the way out. The write path the prompts teach (abstract and type only, no explicit name) is the one that hits it, so the user is left with a failure they cannot act on. The rejection stays: a digest or transliterated name would make the file tree unreadable, and legible paths are the point of the store. What changes is the diagnostic, which now states the cause and the remedy. The same correction applies to a group value that slugifies to nothing.
errors.py promises that a rejected write says which field and why, and the JSON payload keeps that promise. The human renderer did not: it formatted every list entry with the recall line, so a validation error printed as " - - . . score=None" with the field and the reason silently dropped. Improving the wording of a diagnostic changes nothing on the output path most callers read. A field error now renders as "field: reason". The regression test takes its expected text from the structured payload, so it holds the two renderings together instead of pinning a message.
dajiaohuang
marked this pull request as ready for review
September 11, 2026 01:00
dajiaohuang
force-pushed
the
fix/non-ascii-derived-name-error
branch
from
September 11, 2026 10:30
1e1e383 to
dfe440d
Compare
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
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.
Refs #20.
What reproduces at 34d12a2
slugifydrops every character that is not ASCII alphanumeric, so input writtenentirely in CJK, Arabic, or Cyrillic folds to the empty string. Eight of the nine
factory types derive their file name from the abstract through
portable_segment(
profileis the exception: it keys on the system-supplieduser), so the writepath the prompts teach —
--typeand--abstract, no--name— rejects such awrite. A group value that slugs to nothing fails the same way.
What does not reproduce
The issue reports "a file named
.mdat the domain root", with non-ASCII abstractscolliding on one path and overwriting each other. That is not the behaviour at
34d12a2:
placement.resolveraises before any path is built, and no file iscreated. The probe in the linked evidence shows the rejection and an empty store.
So this is not a data-loss bug at HEAD — it is a write that fails with a message
the caller cannot act on.
The change
Two defects, one user-visible outcome: a rejected write the user can act on.
The wording. In core, the rejection stays. A digest or transliterated fallback
would put unreadable names in a tree whose whole point is being walkable with
lsand
grep, and a transliteration table is a dependency this repo does not take.What changes is the diagnostic, which now names the cause and the way out:
The output path.
errors.pypromises that a rejected write says which fieldand why, and the JSON payload keeps that promise — but the plain renderer did not.
_emitformatted every list entry with the recall line, so anyValidationErrorprinted as
with the field and the reason silently dropped. Wording a diagnostic better is
worth nothing on the output path most callers read, so a field error now renders as
field: reason:Verified end to end: the rejected write names the field and the way out in the
default output mode,
mem record --helpshows that field as--name, and the sameabstract with
--name deploy-rewrite-noteslands atfact/default/deploy-rewrite-notes.md.If the maintainers would rather that a non-ASCII abstract succeed, that is a
naming-policy decision — it needs an ASCII-safe fallback and a view on how legible
the resulting tree stays — and I would rather raise it than pick for you.
Tests
tests/unit/test_storage.pytest_a_derived_name_names_the_cause_and_the_way_out— the rejection carries the causetest_a_non_ascii_abstract_is_written_under_an_explicit_name— the promised way out workstest_a_group_that_slugs_to_nothing_names_the_cause— the sibling pathThe first and third fail on 34d12a2 with the old wording and pass after; the second
passes on both, which is the point — an explicit name was always the way through.
tests/system/test_cli.pytest_a_rejected_write_prints_the_field_and_the_reason— the plain output carriesthe same field and reason the structured payload does. It reads its expected text
out of that payload instead of pinning the message, and it fails on the previous
commit with the blank line shown above.
Validation
Local replica of
.github/workflows/ci.yml:ruff checkclean,ruff format --checkclean,mypyclean (66 source files),pytest392 passed, coverage 91.01%against the 85% gate.