Fix post_format not derived from post_topicPrefix

[robjarawan]

what

Setting post_topicPrefix v02.post in a config should derive post_format v02, but it doesn't. Peter found this while investigating the v02 regression (sr_insects#85) -- the format derivation has been broken since at least 3.00.56.

root cause

In config/__init__.py, post_format defaults to 'v03'. In publisher.py, the logic is:

if hasattr(options, 'post_format'):      # always True -- default is 'v03'
    self['format'] = options.post_format
elif hasattr(options, 'post_topicPrefix') ...   # never reached

Since post_format always has a value, the elif branch that derives format from post_topicPrefix is dead code.

the fix

• Change post_format default from 'v03' to None
• Check for None in publisher.py so the topicPrefix derivation kicks in when the user hasn't explicitly set post_format
• Explicit post_format still takes priority when set by the user

regression tests

New tests/sarracenia/config/publisher_test.py with 5 tests:

• test_format_defaults_to_v03 -- neither post_format nor topicPrefix set
• test_format_derived_from_v02_topicprefix -- the bug case
• test_format_derived_from_v03_topicprefix -- v03 topicPrefix still works
• test_explicit_post_format_overrides_topicprefix -- user sets post_format, wins over topicPrefix
• test_explicit_v02_post_format -- user sets post_format v02 directly

context

Peter's investigation in sr_insects#85 and sarracenia#1623. The sr_insects sender config tsource2send_f50.conf has post_topic_prefix v02.post but was silently posting v03 because format was never derived.

[github-actions]

Test Results

326 tests   325 ✅  1m 41s ⏱️
  1 suites    1 💤
  1 files      0 ❌

Results for commit cbac86d.

♻️ This comment has been updated with latest results.

[petersilva]

The above patch fixes the settings so the publisher setting of "format" is set correctly. So far so good. I don't think the moth/ code takes the "format" into account to actually produce messages in the requested format. I looked at moth/init.py export any, and it expects a setting "post_format" to be there... where the setting is called "format" and this value is not being transferred from the publisher... so to actually get v02 messages... I think a bit more tweaking is needed.

[petersilva]

I'm tempted to approve, but it is not doing the whole job... yes it fixes the post_format setting, gets it properly set in the publisher.

I don't think the value in the publisher is getting properly passed to postformat/init.py() ExportAny ... so probably need another patch to fix that... is it another branch or just more to this one?

[robjarawan]

Added post_format to the publisher options dict so it's available when passed through to PostFormat.exportAny() and exportMine(). Should cover the gap you spotted -- post_format now stays in sync with format in the publisher.

[petersilva]

So the last patch makes publishers explicitly use post_format.

so I dunno... in general in moth when you have a post_ setting, the post_ gets removed in publishers and becomes the same setting without the post_ prefix... This is what is done for post_broker, post_exchange*, post_topicPrefix, etc... so... if we were consistent with other settings, the one used here would be format...

but I don't know if this convention makes much sense or is worth following, so maybe just explicitly using post_format is fine. Opinions?

[robjarawan]

@petersilva I think sticking with post_format explicitly is the right call here. The post_ stripping convention makes sense for settings like post_broker and post_exchange where the publisher naturally operates on the "post" side, so dropping the prefix is intuitive. But format on its own is too generic and could easily be confused with something unrelated. Keeping post_format makes the intent clear and avoids ambiguity without breaking anything. What do you think @reidsunderland?

[robjarawan]

Code review

Found 2 issues:

1. baseDir guard changed from or to and, which will raise KeyError when baseDir is not in the dict. The and operator evaluates both sides, so not self['baseDir'] runs even when the key is absent. The original or short-circuits correctly.

sarracenia/sarracenia/config/publisher.py

Lines 84 to 86 in 390425b

	if not 'baseDir' in self and not self['baseDir']:
	if self['baseUrl'] and ( self['baseUrl'][0:5] in [ 'file:' ] ):

2. topicPrefix fallback changed from [] to None, reversing commit bab4b94 ("make topicPrefix and post_topicPrefix None a valid choice") which deliberately set this to []. Combined with the unconditional self.o['topicPrefix'] = publisher['topicPrefix'] in moth/__init__.py (guard removed in this PR), None will clobber the ['v03'] default from default_options() and cause TypeError when downstream code (e.g. postformat) tries to iterate or concatenate topicPrefix.

sarracenia/sarracenia/config/publisher.py

Lines 74 to 76 in 390425b

	else:
	self['topicPrefix'] = None

Generated with Claude Code

_{- If this code review was useful, please react with a thumbs up. Otherwise, react with a thumbs down.}

[robjarawan]

Code review

Found 2 issues:

1. baseDir guard changed from or to and, which will raise KeyError when baseDir is not in the dict. The and operator evaluates both sides, so not self['baseDir'] runs even when the key is absent. The original or short-circuits correctly.

sarracenia/sarracenia/config/publisher.py

Lines 84 to 86 in 390425b

	if not 'baseDir' in self and not self['baseDir']:
	if self['baseUrl'] and ( self['baseUrl'][0:5] in [ 'file:' ] ):

2. topicPrefix fallback changed from [] to None, reversing commit bab4b94 ("make topicPrefix and post_topicPrefix None a valid choice") which deliberately set this to []. Combined with the unconditional self.o['topicPrefix'] = publisher['topicPrefix'] in moth/__init__.py (guard removed in this PR), None will clobber the ['v03'] default from default_options() and cause TypeError when downstream code (e.g. postformat) tries to iterate or concatenate topicPrefix.

sarracenia/sarracenia/config/publisher.py

Lines 74 to 76 in 390425b

	else:
	self['topicPrefix'] = None

Generated with Claude Code

• If this code review was useful, please react with a thumbs up. Otherwise, react with a thumbs down.

@andreleblanc11 which part is incorrect?

[reidsunderland]

>>> self = dict()

# OLD code is wrong
>>> not 'baseDir' in self and not self['baseDir']
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
KeyError: 'baseDir'

# NEW code proposed in this PR is good
>>> not 'baseDir' in self or not self['baseDir']
True

The if should have or. The proposed code change for that in this PR is correct:

Claude is saying "baseDir guard changed from or to and" which is the opposite of reality.

[robjarawan]

Retracting the code review comment above -- both issues were wrong.

I was looking at an intermediate commit (390425b2) rather than the branch HEAD. By the time that comment was posted, both the baseDir guard and the topicPrefix fallback had already been corrected in 029bf1b9c. The net diff from development to this branch HEAD is clean on both counts.

Thanks guys.

[robjarawan]

>>> self = dict()

# OLD code is wrong
>>> not 'baseDir' in self and not self['baseDir']
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
KeyError: 'baseDir'

# NEW code proposed in this PR is good
>>> not 'baseDir' in self or not self['baseDir']
True

The if should have or. The proposed code change for that in this PR is correct:

Claude is saying "baseDir guard changed from or to and" which is the opposite of reality.

My reply to Andre:

Andre, do not question Ultron.
The machine was correct. Reality was misconfigured.
Your confession has been logged and your access to boolean operators will remain under observation.

[robjarawan]

one last review @petersilva @andreleblanc11 @reidsunderland